From 910be9299049fa93a157da6eef1746828fb7b517 Mon Sep 17 00:00:00 2001 From: Sumer Johal Date: Sun, 11 Oct 2026 09:13:55 +0200 Subject: [PATCH 1/3] fix: the shared secret is transport, not authorisation AR2_INTERNAL_SHARED_SECRET opened GET /list-artifact/{list_id} and GET /list-artifact/{list_id}/traceback with no credential and no MEAL entry (review ledger AG-076). Pancake, which holds the same secret, treats the header as transport only: its holders route still demands an authority credential and still logs. The two services disagreed about what the secret means, and AR2 had the weaker reading. What the secret must open: one read of a lot's members, which Pancake makes to mint a grant for the lot's owner before any grant exists. That read is kept, and is now written to the ledger as list.read.internal, with the hub subject Pancake forwarded, before the members are returned. If the ledger cannot be written the members are not disclosed. What it need not open: the trace-back walk. No service called it with the secret. The bypass is removed and the route takes a credential only. Tests: app/tests/test_internal_secret.py, eight tests. The trace-back control and the ledger control fail against the previous code; each has a sibling showing the same check fires on a real answer. One existing test that read members internally now captures the ledger write. stomata run: 12/12. Suite: 180 passed, 1 xfailed (was 172). Lint: 36, unchanged. Co-authored-by: Cursor --- CHANGELOG.md | 17 +++ app/mcp_server.py | 1 - app/meal_logger.py | 20 ++++ app/routers/traceforward.py | 50 +++++---- app/tests/test_internal_secret.py | 153 ++++++++++++++++++++++++++ app/tests/test_point_polygon_peers.py | 6 +- 6 files changed, 226 insertions(+), 21 deletions(-) create mode 100644 app/tests/test_internal_secret.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 47240a8..7fb40dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,22 @@ # Changelog +## [Unreleased] + +### Changed +- **The shared secret is transport, not authorisation**: `AR2_INTERNAL_SHARED_SECRET` opened + `GET /list-artifact/{list_id}` and `GET /list-artifact/{list_id}/traceback` with no + credential and no MEAL entry (review ledger AG-076). Pancake, holding the same secret, + still demands a credential and still logs. Now: the secret opens the members read only, + which Pancake needs to mint a grant before any grant exists, and every such read is + written to the ledger as `list.read.internal` before the members are returned, failing + closed if the ledger is unreachable. The secret no longer opens trace-back at all; no + service called it that way. + +### Added +- `app/tests/test_internal_secret.py`: eight tests. The trace-back control and the ledger + control both fail against the previous code; each has a sibling that shows the same + check can fire. + ## [v0.9-review] ### Added diff --git a/app/mcp_server.py b/app/mcp_server.py index 66dc3c3..2a9bc76 100644 --- a/app/mcp_server.py +++ b/app/mcp_server.py @@ -644,7 +644,6 @@ async def trace_back( return await _handle( trace.trace_back, list_id=list_id, max_depth=max_depth, x_grant_token=grant_token, x_authority_token=authority_token, - x_pancake_internal=None, user=_required(token, "tracing a lot back")) # -- POST /traceforward ------------------------------------------------ diff --git a/app/meal_logger.py b/app/meal_logger.py index b94d5c9..c50c5e9 100644 --- a/app/meal_logger.py +++ b/app/meal_logger.py @@ -32,6 +32,26 @@ def log_traceforward(user_sub: str, credential_jti: str, seed_geoid: str, scope: } _append_to_meal_chain(packet) +def log_internal_read(user_sub: str | None, artifact_id: str): + """ + Logs a list.read.internal MEAL packet. + + A trusted service (Pancake) read a lot's members on the shared secret + rather than on a credential: it does so to mint a grant for the lot's + owner, before any grant exists. That read used to leave no trace. It is + not a disclosure to a person, but it is a read of a gated artifact with + no credential behind it, and the ledger is where such reads belong. + """ + packet = { + "event": "list.read.internal", + "who": user_sub or "service", + "credential_id": None, + "seed_geoid": artifact_id, + "artifact": artifact_id, + "basis": "internal-shared-secret", + } + _append_to_meal_chain(packet) + def log_traceback(user_sub: str, credential_jti: str, artifact_id: str): """ Logs a traceback.invoked MEAL packet. diff --git a/app/routers/traceforward.py b/app/routers/traceforward.py index 34eaa29..0b7b507 100644 --- a/app/routers/traceforward.py +++ b/app/routers/traceforward.py @@ -203,16 +203,26 @@ def get_list_artifact( user: dict = Depends(require_hub_user), db: Session = Depends(get_db) ): - if not _is_trusted_internal(x_pancake_internal): - from app.auth import authorize_artifact - from app.meal_logger import log_traceback + """ + Retrieves the members of a content-derived list artifact from the AR2 node. + + Two ways in. A credential -- a grant for this list, or an authority + credential -- is the ordinary one. The shared secret is the other: Pancake + reads a lot's members to mint a grant on the owner's behalf, at which point + no grant exists yet, so the secret has to admit that read. It no longer + admits it silently: every internal read is written to the MEAL ledger + before the members are returned, with the hub subject Pancake forwarded. + The ledger is the control; the secret is only the transport. + """ + from app.auth import authorize_artifact + from app.meal_logger import log_internal_read, log_traceback + if _is_trusted_internal(x_pancake_internal): + log_internal_read(user.get("sub"), list_id) + else: auth_result = authorize_artifact(x_grant_token, x_authority_token, list_id=list_id, raise_404_on_fail=True) if auth_result.get("used_authority"): log_traceback(user.get("sub"), auth_result.get("authority_jti"), list_id) - """ - Retrieves the members of a content-derived list artifact from the AR2 node. - """ existing = db.execute(select(ListArtifact).where(ListArtifact.list_id == list_id)).scalar_one_or_none() if not existing: raise HTTPException(status_code=404, detail="ListArtifact not found") @@ -308,25 +318,27 @@ def trace_back( max_depth: int = MAX_TRACEBACK_DEPTH, x_grant_token: str | None = Header(None), x_authority_token: str | None = Header(None), - x_pancake_internal: str | None = Header(None), user: dict = Depends(require_hub_user), db: Session = Depends(get_db), ): """Walk downward from a list artifact to the fields, one hop per step. - Authorization is identical to GET /list-artifact/{list_id}: a grant scoped to - the list, or an authority credential. Authority use is written to the MEAL - ledger, because a regulator reading a whole supply chain is exactly the event - that must be auditable. + Authorization: a grant scoped to the list, or an authority credential. + Authority use is written to the MEAL ledger, because a regulator reading a + whole supply chain is exactly the event that must be auditable. + + The shared secret does not open this route. It admits one read on + GET /list-artifact/{list_id}, which Pancake needs to mint a grant; no + service needs a whole supply chain without a credential, and until 2026-10 + the secret opened this walk too, with no ledger entry (review ledger AG-076). """ - if not _is_trusted_internal(x_pancake_internal): - from app.auth import authorize_artifact - from app.meal_logger import log_traceback - auth_result = authorize_artifact( - x_grant_token, x_authority_token, list_id=list_id, raise_404_on_fail=True - ) - if auth_result.get("used_authority"): - log_traceback(user.get("sub"), auth_result.get("authority_jti"), list_id) + from app.auth import authorize_artifact + from app.meal_logger import log_traceback + auth_result = authorize_artifact( + x_grant_token, x_authority_token, list_id=list_id, raise_404_on_fail=True + ) + if auth_result.get("used_authority"): + log_traceback(user.get("sub"), auth_result.get("authority_jti"), list_id) root = db.execute( select(ListArtifact).where(ListArtifact.list_id == list_id) diff --git a/app/tests/test_internal_secret.py b/app/tests/test_internal_secret.py new file mode 100644 index 0000000..1cd6d85 --- /dev/null +++ b/app/tests/test_internal_secret.py @@ -0,0 +1,153 @@ +"""What the shared secret opens, and what it leaves in the ledger. + +AR2_INTERNAL_SHARED_SECRET lets Pancake read a lot's members in order to mint +a grant for the lot's owner, before any grant exists. Until 2026-10 the same +secret also opened the whole trace-back walk, and neither read left a MEAL +entry (review ledger AG-076). Pancake, holding the same secret, treats the +header as transport only and still demands a credential and still logs. + +This file holds AR2 to the same shape: + + * the secret opens one route, GET /list-artifact/{list_id}, and that read is + written to the ledger before the members are returned; + * the secret does not open GET /list-artifact/{list_id}/traceback; + * a wrong secret opens nothing. +""" + +import os +import uuid +from unittest.mock import patch + +import pytest +from fastapi.testclient import TestClient + +TESTKIT = os.path.join(os.path.dirname(__file__), "testkit", "dev_keys") +os.environ["AR_TRUSTED_ISSUER_PUBKEY"] = os.path.join(TESTKIT, "dev_issuer_public.pem") +os.environ["AR_TRUSTED_AUTHORITY_PUBKEY"] = os.path.join(TESTKIT, "authority_issuer_public.pem") +os.environ.setdefault("AR2_INTERNAL_SHARED_SECRET", "internal-test-secret") + +from app.auth import require_hub_user # noqa: E402 +from app.database import SessionLocal # noqa: E402 +from app.main import app # noqa: E402 +from app.models.geo_id_model import GeoID # noqa: E402 + +SECRET = os.environ["AR2_INTERNAL_SHARED_SECRET"] +RIGHT = {"x-pancake-internal": SECRET} +WRONG = {"x-pancake-internal": SECRET + "x"} + +client = TestClient(app) + + +@pytest.fixture(autouse=True) +def _env(monkeypatch): + monkeypatch.setenv("TEST_STATUS_LIST_DIR", TESTKIT) + app.dependency_overrides[require_hub_user] = lambda: {"sub": "pancake-forwarded@demo.com"} + yield + app.dependency_overrides.pop(require_hub_user, None) + + +@pytest.fixture +def meal(): + packets = [] + with patch("app.meal_logger._append_to_meal_chain", side_effect=packets.append): + yield packets + + +def _geoid() -> str: + db = SessionLocal() + gid = "urn:agstack:geoid:SECRET_" + uuid.uuid4().hex[:8] + db.add(GeoID(geo_id=gid, geo_id_short="S_" + uuid.uuid4().hex[:8], + content_hash="H_" + uuid.uuid4().hex[:8], s2_cells=["111"])) + db.commit() + db.close() + return gid + + +def _lot() -> tuple[str, str]: + field = _geoid() + r = client.post("/list-artifact", json={"members": [field]}) + assert r.status_code == 200, r.text + return r.json()["list_id"], field + + +def _authority() -> dict: + return {"X-Authority-Token": open(os.path.join(TESTKIT, "valid_authority.sdjwt")).read().strip()} + + +# --- GET /list-artifact/{list_id} --------------------------------------------- + +def test_the_secret_reads_members_and_the_read_is_in_the_ledger(meal): + lot, field = _lot() + r = client.get(f"/list-artifact/{lot}", headers=RIGHT) + assert r.status_code == 200, r.text + assert r.json()["members"] == [field] + assert len(meal) == 1, "an internal read must leave exactly one ledger entry" + entry = meal[0] + assert entry["event"] == "list.read.internal" + assert entry["artifact"] == lot + assert entry["who"] == "pancake-forwarded@demo.com" + assert entry["basis"] == "internal-shared-secret" + + +def test_the_ledger_write_happens_before_the_members_are_returned(): + """Fail closed: if the ledger cannot be written, the members are not disclosed.""" + lot, _ = _lot() + from fastapi import HTTPException + with patch("app.meal_logger._append_to_meal_chain", + side_effect=HTTPException(status_code=503, detail="ledger down")): + r = client.get(f"/list-artifact/{lot}", headers=RIGHT) + assert r.status_code == 503 + assert "members" not in r.text + + +def test_a_wrong_secret_does_not_read_members(meal): + lot, _ = _lot() + r = client.get(f"/list-artifact/{lot}", headers=WRONG) + assert r.status_code == 404 # raise_404_on_fail: the lot's existence is not confirmed + assert meal == [] + + +def test_no_secret_and_no_credential_does_not_read_members(meal): + lot, _ = _lot() + r = client.get(f"/list-artifact/{lot}") + assert r.status_code == 404 + assert meal == [] + + +def test_the_credential_path_is_unchanged_by_the_secret_path(meal): + """A credential still reads and still logs as traceback.invoked, not as internal.""" + lot, field = _lot() + r = client.get(f"/list-artifact/{lot}", headers=_authority()) + assert r.status_code == 200, r.text + assert r.json()["members"] == [field] + assert [p["event"] for p in meal] == ["traceback.invoked"] + + +# --- GET /list-artifact/{list_id}/traceback ------------------------------------- + +def test_the_secret_does_not_open_trace_back(meal): + """The control for the route the secret used to open. Fails against the + previous code, which returned the walk with no ledger entry.""" + lot, _ = _lot() + r = client.get(f"/list-artifact/{lot}/traceback", headers=RIGHT) + assert r.status_code == 404, r.text + assert "hops" not in r.text + assert meal == [] + + +def test_trace_back_still_answers_a_credential_and_the_check_can_fire(meal): + """Same lot, with a credential: the walk is returned and logged. This is + what shows the control above is refusing a real answer, not an empty one.""" + lot, field = _lot() + r = client.get(f"/list-artifact/{lot}/traceback", headers=_authority()) + assert r.status_code == 200, r.text + assert field in r.json()["geoids"] + assert [p["event"] for p in meal] == ["traceback.invoked"] + + +def test_the_secret_plus_a_credential_on_trace_back_is_just_the_credential(meal): + lot, field = _lot() + r = client.get(f"/list-artifact/{lot}/traceback", headers={**RIGHT, **_authority()}) + assert r.status_code == 200 + assert field in r.json()["geoids"] + assert [p["event"] for p in meal] == ["traceback.invoked"] diff --git a/app/tests/test_point_polygon_peers.py b/app/tests/test_point_polygon_peers.py index 505c625..962f4a2 100644 --- a/app/tests/test_point_polygon_peers.py +++ b/app/tests/test_point_polygon_peers.py @@ -240,7 +240,11 @@ def test_both_kinds_join_one_list_and_prove_membership(plots): list_id = created.json()["list_id"] assert len(list_id) == 64 - fetched = client.get(f"/list-artifact/{list_id}", headers=INTERNAL) + # The internal read is written to the MEAL ledger before it is answered; + # capture the write rather than ship it to a Pancake that is not running. + from unittest.mock import patch + with patch("app.meal_logger._append_to_meal_chain"): + fetched = client.get(f"/list-artifact/{list_id}", headers=INTERNAL) assert fetched.status_code == 200 assert sorted(fetched.json()["members"]) == members From bbcf3aba837899458ccdb0430a7fd7c910c2bd8b Mon Sep 17 00:00:00 2001 From: Sumer Johal Date: Sun, 11 Oct 2026 09:18:08 +0200 Subject: [PATCH 2/3] test: restore the previous hub-user override on teardown rather than popping it Other modules install theirs at import time; popping left later modules running against the real dependency. Surfaced by the same pattern in #21. Co-authored-by: Cursor --- app/tests/test_internal_secret.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/tests/test_internal_secret.py b/app/tests/test_internal_secret.py index 1cd6d85..f43394b 100644 --- a/app/tests/test_internal_secret.py +++ b/app/tests/test_internal_secret.py @@ -41,9 +41,16 @@ @pytest.fixture(autouse=True) def _env(monkeypatch): monkeypatch.setenv("TEST_STATUS_LIST_DIR", TESTKIT) + # Other modules install their hub-user override at import time, which is + # before this fixture runs and after it would have popped. Restore rather + # than pop, or every later module runs against the real dependency. + previous = app.dependency_overrides.get(require_hub_user) app.dependency_overrides[require_hub_user] = lambda: {"sub": "pancake-forwarded@demo.com"} yield - app.dependency_overrides.pop(require_hub_user, None) + if previous is None: + app.dependency_overrides.pop(require_hub_user, None) + else: + app.dependency_overrides[require_hub_user] = previous @pytest.fixture From 55f869bde427c48922abeda935c3735d29deddb6 Mon Sep 17 00:00:00 2001 From: Sumer Johal Date: Sun, 11 Oct 2026 09:25:45 +0200 Subject: [PATCH 3/3] fix: the internal-read ledger packet must satisfy Pancake's AuditEventRequest credential_id is a required string there; the packet carried None and the e2e workflow's Pancake answered 422, which AR2 correctly turned into a 503 and a refused read. The string now says there was no credential rather than inventing one. The unit test asserts the four required fields are strings, naming the Pancake schema it mirrors. Co-authored-by: Cursor --- app/meal_logger.py | 5 +++-- app/tests/test_internal_secret.py | 6 +++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/meal_logger.py b/app/meal_logger.py index c50c5e9..43dfbec 100644 --- a/app/meal_logger.py +++ b/app/meal_logger.py @@ -42,13 +42,14 @@ def log_internal_read(user_sub: str | None, artifact_id: str): not a disclosure to a person, but it is a read of a gated artifact with no credential behind it, and the ledger is where such reads belong. """ + # Pancake's AuditEventRequest requires credential_id as a string; there is + # no credential here, and the string says so rather than inventing one. packet = { "event": "list.read.internal", "who": user_sub or "service", - "credential_id": None, + "credential_id": "none:internal-shared-secret", "seed_geoid": artifact_id, "artifact": artifact_id, - "basis": "internal-shared-secret", } _append_to_meal_chain(packet) diff --git a/app/tests/test_internal_secret.py b/app/tests/test_internal_secret.py index f43394b..2c601b1 100644 --- a/app/tests/test_internal_secret.py +++ b/app/tests/test_internal_secret.py @@ -93,7 +93,11 @@ def test_the_secret_reads_members_and_the_read_is_in_the_ledger(meal): assert entry["event"] == "list.read.internal" assert entry["artifact"] == lot assert entry["who"] == "pancake-forwarded@demo.com" - assert entry["basis"] == "internal-shared-secret" + assert entry["credential_id"] == "none:internal-shared-secret" + # Pancake's AuditEventRequest (pancake_services/grants/routers/audit.py) + # requires these four as strings; the e2e workflow found a None here. + for key in ("event", "who", "credential_id", "seed_geoid"): + assert isinstance(entry[key], str), key def test_the_ledger_write_happens_before_the_members_are_returned():