Skip to content

fix(registry): fail closed on supply paths with canonical shared vectors - #7513

Closed
bokelley wants to merge 13 commits into
mainfrom
fix/supply-path-verifier-golden-vectors
Closed

bokelley wants to merge 13 commits into
mainfrom
fix/supply-path-verifier-golden-vectors

Conversation

@bokelley

@bokelley bokelley commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Coordinator hold: Draft, auto-merge off. Hold, allocation correction. Current head: f4049f8c497fbc62bb985eb8fbc5d9a94c984f66; rebased onto main 70a91fe9f7eab38910e0877f56cf0e3332a21a78. Fresh exact-head CI and human/CODEOWNERS approval remain required. Preserve the requested bmilekic review; do not mark Ready, enable auto-merge, or merge until the coordinator hold is cleared.

The supply-path verifier could report a verified path with absent or malformed collection constraints, dangling carriage references, or grants narrowed by unsupported qualifiers. Domain-level queries could combine unrelated collection legs.

This change fails closed across the five-leg verdict, publishes canonical shared golden vectors with semantics_version: "1", and preserves authorization bulk-grant semantics. It verifies complete property binding and independently evaluates collection-specific IAB evidence with bounded fetching and exact-origin redirects.

The registry persists authority-scoped revocation holds and authoritative-location pins through the existing SDK PostgreSQL state store. Migration 597 binds successful-fetch provenance atomically to each cached manifest, keeping failed-attempt URLs and truncated display URLs out of authorization decisions. Existing cache rows require a successful refresh before establishing authority. Cross-origin shared documents require explicit publisher attribution. Sources expose observation times and full resolved URLs. Malformed provenance, mixed pointers, and chained targets fail closed without discarding held denials. First pins require a successful, valid manifest write.

Dependency: merge this upstream PR before adcp-client#2908, completing adcp-client#2783. The client consumes the canonical vector bytes, rejects older semantics, and must pin the actual merged upstream SHA before its own merge. The prior SDK dependency adcp-client#2911 has merged as 8ba12c2ace85a88533ce1d56efd35badea51a97c; its repair does not replace this head's fresh Generate TypeScript SDK check.

Migration allocation

The final tree contains only 597_supply_path_manifest_provenance.sql, with unchanged SQL blob f6629447ccfecb28834380e9efe4863099454555. The old supply-path 591 filename is absent. The obsolete intermediate two-file commit and its canceled CI are not qualification evidence.

Version Protected owner Filename
588 #7433 588_agent_compliance_runner_version.sql
589 main 589_compliance_run_publication.sql
590 main 590_compliance_run_provenance.sql
591 #7457 591_compliance_refresh_authenticated_credential.sql
592 #7463; inherited by #7499 592_email_mutations.sql
593 #7464; inherited by #7460 593_admin_credential_bind_operations.sql
594 unpublished Slack reservation 594_slack_identity_binding_operations.sql
595 #7501 595_normalized_email_invariant.sql
596 unpublished grant lifecycle reservation 596_organization_credential_grant_lifecycle.sql
597 this PR 597_supply_path_manifest_provenance.sql

At 2026-09-14 18:38:35 UTC, main and the then-open 50 PR migration directories were audited, together with the coordinator's unpublished 594/596 reservations. That snapshot found no other published PR claiming 597. The subsequent independent audit counted 51 open PRs; the newcomer had no migration. Neither snapshot inventories undisclosed worktrees, and unpublished 594/596 reservations are coordinator-supplied evidence rather than live GitHub refs. The earlier claim that no open PR owned 592–596 was incorrect; the table above is the corrected allocation.

597 has no SQL/application dependency on the #6827 chain. Preserve that chain's internal 591 → 592 → 593 → 594 → 595 → 596 order. Its files are not imported or modified in this PR. See allocation and replay evidence for exact reserved hashes and ownership.

Validation and review

  • Local full server suite: 9,561 passed / 31 skipped across 620 files. Run at local-only fc41b86e, which is not GitHub-retrievable and is not immutable exact-head CI evidence. All server runtime/test files are identical through this head except one removed EOF blank line. Main's subsequent feat(compliance): frequency-cap proposal-refinement scenario #7522 adds only a compliance scenario and its changeset.
  • Current-main root suite: 1,063 passed. Build, schema/compliance/protocol-bundle checks, commitlint, changeset scope/status, and diff checks pass. Canonical corpus remains 74 verdict + 11 helper vectors.
  • Focused live PostgreSQL migration and loader regressions: 28 passed; migration naming/duplicate/version checks: 3 passed.
  • Clean apply and no-op replay pass. Combined exact frozen 588–597 inputs apply 579 ordinary migrations, retaining the existing external evaluator exclusions for 584/585. Replay preserves every ledger row, timestamp, and raw schema hash.
  • Applying 597 first and the pending reserved chain later preserves its ledger entry and a successful manifest/provenance pair. The only schema difference from ascending application is 588's physical column position after main's already-applied 589/590; all other schema dump bytes match.
  • Negative controls reject duplicate 591 before applying SQL and reject an incorrect modern 591 ledger filename without mutating the ledger/schema. No production/shared database repair was performed. Reserved SQL was used only in isolated validation, not published here.
  • Four Claude expert reviews plus Codex reviews found no blocking code findings. Independent exact-head audit confirms f4049f8c / tree 44f9c3427847da8844eb1d91ca6d5214746aa3fb, preserved behavior with evidence-file additions and EOF cleanup, unchanged protected SQL blobs, and applicability of the recorded local migration evidence. This does not replace human/CODEOWNERS approval.

Exact-head GitHub CI is terminal: 61 successful checks, 2 failures, 1 intentionally skipped (Ladon for Draft). All storyboards, both generated SDKs, IPR, migration smoke/duplicate checks, and build/static checks passed. TypeScript SDK generation consumed the f4049f8c protocol bundle against client main ecf1c745. The new provenance migration regression also executed and passed 2/2 tests in fresh integration shard 2/4. The only substantive failure is private-ledger integration shard 3/4; its aggregate is the second red check. Content/migration audit: PASS. Merge readiness: BLOCKED. The push used an explicit lease against 70345d00df6baabf8828709965a599da861fecbe; Draft, auto-merge off, and the bmilekic review request were verified afterward. The local advisory storyboard matrix was skipped; the fresh exact-head GitHub matrix is authoritative. Older head 70345d00 failed integration shard 2/4 with two 30-second external-agent smoke timeouts at external-smoke-check.test.ts:35,45 (504 passed / 2 failed). The migration correction does not fix that unchanged live harness; #7511 owns its separate repair and remains held. No code from that PR is copied here, and old CI is not counted as final qualification.

Current-head failure classification: job 104103521658 failed fixed-trace-component-smoke-private-ledger-migration.test.ts:488 (637 passed / 1 failed / 7 skipped). PostgreSQL backend 211 logged a lock timeout at 18:43:28.900Z on the complete-plan recovery SELECT ... FOR UPDATE. The test holds the plan row across two fixed 75 ms sleeps against the unchanged production 250 ms lock budget; scheduling overhead can correctly produce persistence_uncertain. The test, implementation, and migrations 582/583 are byte-identical to main. This is the race already repaired by held #7514, not #7511's external smoke harness. No rerun or repair copy was performed. Consume #7514 only after its current-main qualification and human-approved landing, then rerun exact-head qualification. A probabilistic green retry would not incorporate that repair.

aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Sep 14, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — fail-closed supply-path verifier rewrite, clean review.

Reviewer confirmed the load-bearing fix (host grants now require an affirmative collections !== undefined selector; verified_owner_sold requires all three legs with full property coverage and no unsupported constraints), bounded fail-closed IAB fetching (5s / 256KB / 0-redirect / text-plain + rate limiter), DoS cost bounds, single-level bulk recursion, and schema↔docs↔OpenAPI coherence for semantics_version / resolved_collection_id. Changeset present, minor, protocol-scoped; additive wire fields; 64-vector golden corpus and unit tests included.

No Critical/High/Medium findings.

Gate checks:

  • Author team gates: no no-auto-approve match.
  • gated_paths: false → row 2 does not fire.
  • high_risk: true, but only server/src/routes/registry-api.ts (modified) with no medium-or-higher finding on it → rows 3 and 5 do not fire.
  • No data-loss/schema/infra medium findings (row 4 n/a).
  • No prior decision (row 6 n/a).
  • Zero findings → rows 1, 8 n/a.

Falls through to row 9 → approve.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Escalate to human review

Escalate — gated-path approval gate not satisfied.

This PR modifies protocol schema source (static/schemas/source/adagents.json and static/schemas/source/core/collection.json), which places it under the deterministic hard approval gate (gated_paths: true). The current review_decision is REVIEW_REQUIRED, not APPROVED, so per decision rule row 2 the outcome must be escalate rather than approve — despite the reviewer surfacing no critical/high/medium findings.

Escalation reasons

  • static/schemas/source/adagents.json (modified) — under hard approval gate; requires human/CODEOWNERS approval (review_decision is REVIEW_REQUIRED).
  • static/schemas/source/core/collection.json (modified) — under hard approval gate; requires human/CODEOWNERS approval.

Note: the reviewer confirmed fail-closed verifier semantics, same-origin/SSRF redirect policy, revocation persistence, provenance migration 591, and schema↔docs/changeset coherence with no blocking or Medium findings. The two non-blocking follow-ups (a benign publisher-migration pin surfacing as a 500, and the full precommit suite not yet confirmed green on final source) are informational. Once a human/CODEOWNERS approval lands (review_decision: APPROVED), this clean diff would fall through to approve.

Why human review

  • static/schemas/source/adagents.json (modified) is under the hard approval gate and review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required
  • static/schemas/source/core/collection.json (modified) is under the hard approval gate and review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/adagents.json (modified) matches static/schemas/source/**; static/schemas/source/core/collection.json (modified) matches static/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

@aao-secretariat aao-secretariat Bot added the ladon/needs-human-review Ladon has escalated this PR for human review. label Sep 14, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Escalate to human review

Escalate — gated schema paths require human/CODEOWNERS approval.

This PR modifies published protocol source under static/schemas/source/** (adagents.json and core/collection.json), which is under the repo's hard, deterministic approval gate. gated_paths is true and review_decision is REVIEW_REQUIRED (not APPROVED), so decision-table row 2 forces escalation — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

The reviewer delta pass surfaced no Critical/High/Medium findings (fail-closed hardening of the supply-path authority surface with matching normative docs and new tests), so there are no blocking code findings. This escalation is purely the schema-source approval gate.

Escalation reasons

  • static/schemas/source/adagents.json (modified) — under the hard approval gate; review_decision is REVIEW_REQUIRED, human/CODEOWNERS approval required.
  • static/schemas/source/core/collection.json (modified) — under the hard approval gate; review_decision is REVIEW_REQUIRED, human/CODEOWNERS approval required.

Why human review

  • static/schemas/source/adagents.json (modified) is under the hard approval gate and review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required
  • static/schemas/source/core/collection.json (modified) is under the hard approval gate and review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/adagents.json (modified) matches static/schemas/source/**; static/schemas/source/core/collection.json (modified) matches static/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Escalate to human review

Escalate — hard approval gate not yet satisfied.

This PR modifies protocol schema source (static/schemas/source/adagents.json, static/schemas/source/core/collection.json), which is under the repo's hard, non-overridable approval gate (gated_paths: true). The current review_decision is REVIEW_REQUIRED, not APPROVED, so row 2 of the decision table fires: Ladon cannot auto-approve regardless of how clean the diff is until a human/CODEOWNERS approval is recorded.

Blocking findings: none. The reviewer reported a clean pass on the delta — the change swaps .hash for parsed.href.includes('#') at three supply-path trust-boundary sites (adagents-manager.ts:607, supply-path-authority-state.ts:458, supply-path-snapshot.ts:661) to close the bare-# empty-fragment bypass, each with a matching test. Fail-closed, strict superset of the prior check, no false positives on %23-encoded paths. No critical/high/medium findings.

Medium findings: none.

Escalation reasons:

  • static/schemas/source/adagents.json (modified) is under the hard approval gate; review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required.
  • static/schemas/source/core/collection.json (modified) is under the hard approval gate; review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required.

This matches the prior decision (escalate) — the gate persists purely because the required human/CODEOWNERS approval has not yet been recorded. Once review_decision becomes APPROVED, this clean diff can fall through to approve.

Why human review

  • static/schemas/source/adagents.json (modified) is under the hard approval gate and review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required
  • static/schemas/source/core/collection.json (modified) is under the hard approval gate and review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/adagents.json (modified) matches static/schemas/source/**; static/schemas/source/core/collection.json (modified) matches static/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

Copy link
Copy Markdown
Contributor Author

Coordinator migration-collision hold: auto-merge disabled and PR moved to Draft. This branch publishes migration 591_supply_path_manifest_provenance.sql, but #7457 already reserved/published migration 591 as 591_compliance_refresh_authenticated_credential.sql. Merging either order would create a duplicate migration number/startup-deploy collision. Please renumber this PR to 597 or higher only after a fresh main/open-PR allocation audit; preserve 591-596 ownership/order, rerun migration-ledger/replay CI and obtain exact-head independent + human review before Ready.

@bokelley

Copy link
Copy Markdown
Contributor Author

Renumbered: 591_supply_path_manifest_provenance.sql → 597_supply_path_manifest_provenance.sql (two commits on head). Audit: main tops at 590; #7457 holds 591; no open PR claims 592–596; 597 is the next uncontested slot.

Remaining before Ready:

  • migration-ledger/replay CI must pass on the new head
  • exact-head independent + human/CODEOWNERS review

PR stays Draft until those gates clear.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Coordinator correction to the renumber note: 597 is currently uncontested, but the statement that no open PR claims 592–596 is false. #7463 owns 592, #7464 owns 593 (#7460 inherits the same 593), unpublished Slack lifecycle reserves 594, #7501 owns 595, and unpublished grant lifecycle reserves 596. Preserve all of those allocations. The transient 7956c26 head also briefly contained both 591 and 597; latest 70345d0 appears to remove 591, but remains under independent audit and is still based behind current main 1467e46. Keep Draft/auto-off; do not claim exact-head readiness until the latest tree, rebase, CI, body and full allocation references are corrected.

Copy link
Copy Markdown
Contributor Author

Closing as superseded by merged #7544, which deliberately combines the work from #7513 and #7514. This Draft is now stale, conflicting, and duplicates code already on main; it must not be merged. Downstream adcp-client#2908 should consume the canonical merged source at dc69e0b2264b4c563f2175a48c515b7108dbb4f6, refresh its vectors, and requalify on its exact rebased head.

@bokelley bokelley closed this Sep 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ladon/needs-human-review Ladon has escalated this PR for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant