Skip to content

Commit 7fb2fbc

Browse files
JSv4claude
andcommitted
feat(packs): let an operator decline to execute in-pack provider code
Closes the three items in #2263. 1. AUTHORITY_PACK_LOAD_PROVIDERS (default True, so nothing changes for existing installs). Loading an in-pack provider IMPORTS it into the web and worker processes; extraction refuses path traversal and setuid bits but cannot refuse code. When off, the modules are NOT imported and the skip is logged once with a count and the module names, so 'turned off' and 'no packs installed' stay distinguishable. Safe to decline because the pack contract (authority-packs SOURCE_PROVIDERS.md, P5) requires a pack to serve its text with providers/ deleted. 2. get_authority_source_provider(class_name) — the supported seam for an in-pack provider that DELEGATES to a core one rather than re-scraping. Accepts a bare class name or a dotted path, refuses an ambiguous leaf rather than picking, and returns None (never raises) so a pack declines instead of crashing registry build. 3. install_authority_pack now prints the provider modules a pack ships BEFORE any DB writes — by listing files and reading the optional providers: block, never by importing them, since reporting a pack's code by executing it would defeat the point. test_authority_pack_provider_trust.py probes with a module that writes a sentinel AT IMPORT, so 'was it executed?' is answerable independently of whether its class registered. Verified the two gate tests fail with the gate removed and pass with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 388d681 commit 7fb2fbc

4 files changed

Lines changed: 354 additions & 0 deletions

File tree

config/settings/base.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1784,6 +1784,22 @@
17841784
# path. Same restart requirement as AUTHORITY_PACK_PATHS.
17851785
AUTHORITY_PACK_ROOTS = env.list("AUTHORITY_PACK_ROOTS", default=[])
17861786

1787+
# Whether to LOAD (import) provider modules shipped inside authority packs
1788+
# (``<pack>/providers/*.py``, ``<pack>/discovery_providers/*.py``).
1789+
#
1790+
# Installing such a pack executes its Python in the web and worker processes.
1791+
# That is a materially larger blast radius than ``source_hosts``, where
1792+
# "installing the pack is the trust decision" holds because the consequence is
1793+
# bounded to which hosts may be fetched. Extraction already refuses path
1794+
# traversal and setuid bits (``tar.extract(..., filter="data")``); it cannot
1795+
# refuse code.
1796+
#
1797+
# Default True preserves existing behaviour. An operator installing packs they
1798+
# did not author sets this False and loses only the ability to RE-FETCH the
1799+
# pack's text: the authority-packs contract (SOURCE_PROVIDERS.md, clause P5)
1800+
# requires a pack to install and serve its sections with ``providers/`` deleted.
1801+
AUTHORITY_PACK_LOAD_PROVIDERS = env.bool("AUTHORITY_PACK_LOAD_PROVIDERS", default=True)
1802+
17871803
# Where `manage.py install_authority_pack` materialises packs fetched from the
17881804
# pack registry repo. The directory is an implicit pack bundle root (scanned by
17891805
# authority_pack_dirs() exactly like an AUTHORITY_PACK_ROOTS entry), so a

opencontractserver/corpuses/management/commands/install_authority_pack.py

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,63 @@ def materialise_pack(staged_pack: Path, pack: str, stdout=None) -> Path:
140140
return dest
141141

142142

143+
def _report_pack_providers(pack_dir, stdout, style) -> None:
144+
"""Print the provider modules a pack ships, if any. Never imports them.
145+
146+
Installing a pack that ships ``providers/`` imports its Python into the web
147+
and worker processes, so the operator should be able to see that surface
148+
before the install writes anything. Reads the filesystem and, when present,
149+
the OPTIONAL ``providers:`` declaration in pack.yaml — never the modules.
150+
"""
151+
from pathlib import Path as _Path
152+
153+
pack_dir = _Path(pack_dir)
154+
shipped: list[tuple[str, str]] = []
155+
for subdir in ("providers", "discovery_providers"):
156+
component_dir = pack_dir / subdir
157+
if not component_dir.is_dir():
158+
continue
159+
for py in sorted(component_dir.glob("*.py")):
160+
if not py.name.startswith("_"):
161+
shipped.append((subdir, py.name))
162+
if not shipped:
163+
return
164+
165+
stdout.write(
166+
style.WARNING(
167+
f"This pack ships {len(shipped)} provider module(s). Loading the pack "
168+
"IMPORTS them into the web and worker processes:"
169+
)
170+
)
171+
for subdir, name in shipped:
172+
stdout.write(f" {subdir}/{name}")
173+
174+
# The declaration is optional and descriptive; show it when a pack has one
175+
# so the claimed prefixes are visible without reading Python.
176+
try:
177+
import yaml
178+
179+
manifest = yaml.safe_load((pack_dir / "pack.yaml").read_text()) or {}
180+
for entry in manifest.get("providers") or []:
181+
stdout.write(
182+
f" declares: {entry.get('class')} "
183+
f"prefixes={entry.get('supported_prefixes')} "
184+
f"delegates_to={entry.get('delegates_to') or '-'}"
185+
)
186+
except Exception: # noqa: BLE001 - a missing/!parsing manifest is not fatal here
187+
pass
188+
189+
from django.conf import settings
190+
191+
if not getattr(settings, "AUTHORITY_PACK_LOAD_PROVIDERS", True):
192+
stdout.write(
193+
style.SUCCESS(
194+
" AUTHORITY_PACK_LOAD_PROVIDERS is off — these will NOT be "
195+
"imported. The pack's text still installs and serves."
196+
)
197+
)
198+
199+
143200
class Command(BaseCommand):
144201
help = (
145202
"Fetch an authority pack from the pack registry repo into "
@@ -282,6 +339,11 @@ def handle(self, *args, **options):
282339
"--creator is required to install or preflight (or use --fetch-only)"
283340
)
284341

342+
# Say what code this pack will run, BEFORE any DB writes, and without
343+
# importing it — reporting a pack's providers by executing them would
344+
# defeat the point. Static file listing only.
345+
_report_pack_providers(dest, self.stdout, self.style)
346+
285347
call_command(
286348
"load_authority_pack",
287349
path=str(dest),

opencontractserver/pipeline/registry.py

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,27 @@ def authority_pack_dirs() -> list[Path]:
228228
return unique
229229

230230

231+
def pack_provider_modules(subdir_name: str) -> list[Path]:
232+
"""Provider modules a pack SHIPS, found without importing any of them.
233+
234+
Used for two things that must not execute pack code: telling an operator
235+
what a pack will run before they install it, and naming what was skipped
236+
when loading is switched off. Mirrors the filtering
237+
``_discover_pack_component_classes`` applies (``*.py``, no leading
238+
underscore) so the count is the count that matters.
239+
"""
240+
modules: list[Path] = []
241+
for pack_dir in authority_pack_dirs():
242+
component_dir = pack_dir / subdir_name
243+
if not component_dir.is_dir():
244+
continue
245+
modules.extend(
246+
py for py in sorted(component_dir.glob("*.py"))
247+
if not py.name.startswith("_")
248+
)
249+
return modules
250+
251+
231252
class ComponentType(str, Enum):
232253
"""Types of pipeline components."""
233254

@@ -472,6 +493,26 @@ def _discover_pack_component_classes(
472493
classes that merely happen to be in scope are ignored via the
473494
``__module__`` check.
474495
"""
496+
# Loading an in-pack provider IMPORTS it into this process. That is the
497+
# trust decision AUTHORITY_PACK_LOAD_PROVIDERS exists to let an operator
498+
# decline; see the setting's comment in config/settings/base.py.
499+
# Reported rather than silent, and reported with a count, so "I turned
500+
# it off" and "this install has none" are distinguishable.
501+
from django.conf import settings as _settings
502+
503+
if not getattr(_settings, "AUTHORITY_PACK_LOAD_PROVIDERS", True):
504+
skipped = pack_provider_modules(subdir_name)
505+
if skipped:
506+
logger.info(
507+
"AUTHORITY_PACK_LOAD_PROVIDERS is off: skipped %d in-pack "
508+
"%s module(s) without importing them (%s). The packs still "
509+
"serve their text; only re-fetch is unavailable.",
510+
len(skipped),
511+
subdir_name,
512+
", ".join(sorted(p.parent.parent.name + "/" + p.name for p in skipped)),
513+
)
514+
return []
515+
475516
seen: set[type] = set()
476517
found: list[type] = []
477518
pack_dirs = authority_pack_dirs()
@@ -1020,6 +1061,66 @@ def get_all_authority_source_providers_cached() -> (
10201061
return get_registry().authority_source_providers
10211062

10221063

1064+
def get_authority_source_provider(class_name: str) -> Optional[Any]:
1065+
"""Return an INSTANCE of a registered authority source provider, by class name.
1066+
1067+
The supported seam for an in-pack provider that DELEGATES to a core one.
1068+
1069+
Most packs should not ship a scraper: core already covers the Code of
1070+
Federal Regulations, the U.S. Code and the Federal Register, and those
1071+
providers fail to fire for a pack only because of key SHAPE — the CFR
1072+
provider accepts ``cfr-{digits}:`` while a pack's sections are keyed
1073+
``itar:``, ``ear:``, ``aeca:``. A pack already declares that translation in
1074+
its own equivalence rows, so the in-pack provider is a key translator plus a
1075+
delegation (authority-packs ``SOURCE_PROVIDERS.md``).
1076+
1077+
Doing that by importing the core class directly works, but couples every
1078+
such pack to an import path: a refactor of the core providers would break
1079+
them, and registry discovery isolates import failures by design, so the
1080+
breakage would be a logged warning and a provider that silently stopped
1081+
existing. Depending on this function instead makes the class name the
1082+
contract.
1083+
1084+
Returns ``None`` when no provider of that class name is registered — a pack
1085+
MUST handle that by declining (``can_handle`` -> False) rather than raising,
1086+
so an install missing a core provider degrades to "cannot re-fetch" instead
1087+
of breaking registry build.
1088+
"""
1089+
registry = get_registry()
1090+
definition = registry.get_by_class_name(class_name) or registry.get_by_name(class_name)
1091+
if definition is None:
1092+
# ``class_name`` is stored as a full dotted path; a pack author writes
1093+
# the leaf. Accept either, and refuse an ambiguous leaf rather than
1094+
# picking — two providers with the same class name in different
1095+
# packages is exactly when silently choosing is worst.
1096+
matches = [
1097+
d
1098+
for d in get_all_authority_source_providers_cached()
1099+
if d.class_name.rsplit(".", 1)[-1] == class_name
1100+
]
1101+
if len(matches) > 1:
1102+
logger.warning(
1103+
"Authority source provider name %r is ambiguous (%s); "
1104+
"delegate using the full dotted path.",
1105+
class_name,
1106+
", ".join(sorted(d.class_name for d in matches)),
1107+
)
1108+
return None
1109+
if not matches:
1110+
return None
1111+
definition = matches[0]
1112+
component = getattr(definition, "component_class", None)
1113+
if component is None:
1114+
return None
1115+
try:
1116+
return component()
1117+
except Exception as exc: # pragma: no cover - defensive; a provider ctor should be trivial
1118+
logger.warning(
1119+
"Could not instantiate authority source provider %r: %s", class_name, exc
1120+
)
1121+
return None
1122+
1123+
10231124
def get_all_authority_discovery_providers_cached() -> (
10241125
tuple[PipelineComponentDefinition, ...]
10251126
):
Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
"""Declining to execute a pack's code, and the supported way to reuse a core provider.
2+
3+
Installing an authority pack that ships ``<pack>/providers/*.py`` IMPORTS that
4+
Python into the web and worker processes. Extraction already refuses path
5+
traversal and setuid bits (``tar.extract(..., filter="data")``); it cannot
6+
refuse code. That is a materially larger blast radius than ``source_hosts``,
7+
where "installing the pack is the trust decision" holds because the consequence
8+
is bounded to which hosts may be fetched.
9+
10+
``AUTHORITY_PACK_LOAD_PROVIDERS`` lets an operator decline. It is safe to
11+
decline because the pack contract (authority-packs ``SOURCE_PROVIDERS.md``,
12+
clause P5) requires a pack to install and serve its sections with ``providers/``
13+
deleted — so turning it off costs re-fetch and nothing else.
14+
15+
The tests below use a provider module that writes a sentinel **at import time**.
16+
That is exactly what a pack MUST NOT do (clause P4), which is what makes it a
17+
good probe here: if the sentinel appears, the module was imported, and no
18+
assertion about registration can be fooled by a provider that merely failed to
19+
register.
20+
"""
21+
22+
from __future__ import annotations
23+
24+
import tempfile
25+
from pathlib import Path
26+
27+
from django.test import SimpleTestCase, override_settings
28+
29+
from opencontractserver.pipeline.registry import (
30+
get_all_authority_source_providers_cached,
31+
get_authority_source_provider,
32+
pack_provider_modules,
33+
reset_registry,
34+
)
35+
36+
_REGISTRY_LOGGER = "opencontractserver.pipeline.registry"
37+
38+
# Writes a file when imported, so "was this module executed?" is answerable
39+
# independently of whether its class ended up registered.
40+
_PROVIDER_SRC = '''
41+
from pathlib import Path
42+
43+
from opencontractserver.pipeline.base.base_authority_source_provider import (
44+
AuthorityRequest,
45+
BaseAuthoritySourceProvider,
46+
)
47+
48+
Path(__file__).with_name("IMPORTED.sentinel").write_text("yes")
49+
50+
51+
class TrustProbeSourceProvider(BaseAuthoritySourceProvider):
52+
title = "Trust Probe"
53+
description = "Fixture provider for AUTHORITY_PACK_LOAD_PROVIDERS tests."
54+
author = "test"
55+
supported_prefixes = ("trustprobe",)
56+
57+
def can_handle(self, canonical_key: str) -> bool:
58+
return canonical_key.split(":", 1)[0] == "trustprobe"
59+
60+
def _locate_impl(self, canonical_key, **kwargs):
61+
return AuthorityRequest(url="https://example.invalid/x")
62+
63+
def _fetch_impl(self, request, **kwargs):
64+
raise NotImplementedError
65+
'''
66+
67+
68+
class _PackFixture(SimpleTestCase):
69+
"""Builds a throwaway pack dir containing one provider module."""
70+
71+
def setUp(self) -> None:
72+
self._tmp = tempfile.TemporaryDirectory()
73+
self.addCleanup(self._tmp.cleanup)
74+
self.pack_dir = Path(self._tmp.name) / "trustprobe_pack"
75+
providers = self.pack_dir / "providers"
76+
providers.mkdir(parents=True)
77+
(providers / "trust_probe_provider.py").write_text(_PROVIDER_SRC)
78+
self.sentinel = providers / "IMPORTED.sentinel"
79+
reset_registry()
80+
self.addCleanup(reset_registry)
81+
82+
def _class_names(self) -> set[str]:
83+
# ``class_name`` is the full dotted path; compare on the leaf so the
84+
# assertions read as the class a pack author actually writes.
85+
return {
86+
d.class_name.rsplit(".", 1)[-1]
87+
for d in get_all_authority_source_providers_cached()
88+
}
89+
90+
91+
class LoadProvidersSettingTests(_PackFixture):
92+
def test_providers_load_by_default(self) -> None:
93+
"""The default must not change: existing installs keep working."""
94+
with override_settings(
95+
AUTHORITY_PACK_PATHS=[str(self.pack_dir)],
96+
AUTHORITY_PACK_LOAD_PROVIDERS=True,
97+
):
98+
names = self._class_names()
99+
100+
self.assertIn("TrustProbeSourceProvider", names)
101+
self.assertTrue(
102+
self.sentinel.exists(), "fixture never imported; the test proves nothing"
103+
)
104+
105+
def test_providers_are_not_imported_when_disabled(self) -> None:
106+
"""Not merely unregistered — NOT EXECUTED."""
107+
with override_settings(
108+
AUTHORITY_PACK_PATHS=[str(self.pack_dir)],
109+
AUTHORITY_PACK_LOAD_PROVIDERS=False,
110+
):
111+
names = self._class_names()
112+
113+
self.assertNotIn("TrustProbeSourceProvider", names)
114+
self.assertFalse(
115+
self.sentinel.exists(),
116+
"the pack's module was IMPORTED despite AUTHORITY_PACK_LOAD_PROVIDERS "
117+
"being off — the setting gates registration but not execution, which "
118+
"is the only thing it was for",
119+
)
120+
121+
def test_skipping_is_reported_with_a_count(self) -> None:
122+
"""Silence would make 'turned off' and 'no packs installed' identical."""
123+
with override_settings(
124+
AUTHORITY_PACK_PATHS=[str(self.pack_dir)],
125+
AUTHORITY_PACK_LOAD_PROVIDERS=False,
126+
):
127+
with self.assertLogs(_REGISTRY_LOGGER, level="INFO") as captured:
128+
self._class_names()
129+
130+
messages = "\n".join(captured.output)
131+
self.assertIn("AUTHORITY_PACK_LOAD_PROVIDERS is off", messages)
132+
self.assertIn("trust_probe_provider.py", messages)
133+
134+
def test_core_providers_survive_the_setting(self) -> None:
135+
"""Only IN-PACK loading is declined; core providers are unaffected."""
136+
with override_settings(
137+
AUTHORITY_PACK_PATHS=[str(self.pack_dir)],
138+
AUTHORITY_PACK_LOAD_PROVIDERS=False,
139+
):
140+
names = self._class_names()
141+
142+
self.assertIn("CFRAuthoritySourceProvider", names)
143+
144+
145+
class PackProviderModulesTests(_PackFixture):
146+
def test_lists_shipped_modules_without_importing_them(self) -> None:
147+
"""The listing that --check and the skip log both depend on."""
148+
with override_settings(AUTHORITY_PACK_PATHS=[str(self.pack_dir)]):
149+
found = pack_provider_modules("providers")
150+
151+
self.assertEqual([p.name for p in found], ["trust_probe_provider.py"])
152+
self.assertFalse(
153+
self.sentinel.exists(), "listing the modules executed one of them"
154+
)
155+
156+
157+
class DelegationSeamTests(SimpleTestCase):
158+
"""``get_authority_source_provider`` is the seam in-pack providers delegate through."""
159+
160+
def setUp(self) -> None:
161+
reset_registry()
162+
self.addCleanup(reset_registry)
163+
164+
def test_returns_a_usable_core_provider_instance(self) -> None:
165+
provider = get_authority_source_provider("CFRAuthoritySourceProvider")
166+
167+
self.assertIsNotNone(provider)
168+
# The thing a delegating pack provider actually needs: routing plus a
169+
# pure locate it can hand a translated key to.
170+
self.assertTrue(provider.can_handle("cfr-22:120.4"))
171+
self.assertFalse(provider.can_handle("itar:120.4"))
172+
173+
def test_unknown_provider_returns_none_rather_than_raising(self) -> None:
174+
"""A pack must be able to decline, not crash registry build."""
175+
self.assertIsNone(get_authority_source_provider("NoSuchProviderClass"))

0 commit comments

Comments
 (0)