feat(tool-capability): admit external Claude plugins fail-closed - #560
Conversation
Add a local Tool / Capability port for issue #545 so marketplace metadata, mutable refs, Anthropic review, and plugin instructions cannot become runtime authority. Exact commit/path/digest identity, pinned AppGuardrail and quarantine receipts, product/role scope, expiry/rollback, catalog-drift refusal, and idempotent activation/invocation receipts fail closed. Product-runtime Claude plugin wrappers are rejected. Keep a local ACL/test double until context-graph-contracts publishes an immutable shared contract.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (32)
📝 WalkthroughWalkthrough외부 Claude 커뮤니티 플러그인을 위한 로컬 fail-closed admission 포트를 추가했습니다. 정확한 source, catalog, Policy / Approval, AppGuardrail 및 quarantine 증거를 검증합니다. 활성화와 호출에서 authority drift, runtime window, replay 충돌을 재검증합니다. Changes외부 확장 admission
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant ExternalExtensionAdmission
participant PolicyAuthority
participant CatalogAndReceipts
participant WebCrypto
Caller->>ExternalExtensionAdmission: admitExternalExtension
ExternalExtensionAdmission->>PolicyAuthority: resolvePolicyApproval
ExternalExtensionAdmission->>CatalogAndReceipts: resolveCatalog and resolveScanReceipt
ExternalExtensionAdmission-->>Caller: AdmittedExternalExtension
Caller->>ExternalExtensionAdmission: activateExternalExtension
ExternalExtensionAdmission->>PolicyAuthority: revalidate policy
ExternalExtensionAdmission->>CatalogAndReceipts: revalidate owner evidence
ExternalExtensionAdmission-->>Caller: ExternalExtensionActivation
Caller->>ExternalExtensionAdmission: invokeExternalExtension
ExternalExtensionAdmission->>WebCrypto: digest invocation envelope
ExternalExtensionAdmission->>PolicyAuthority: revalidate live authority
ExternalExtensionAdmission->>CatalogAndReceipts: revalidate catalog and receipts
ExternalExtensionAdmission-->>Caller: invocation receipt or replay
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
seonghobae
added
enhancement
ChatGPT Codex Connector
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
유효 finding: admission은 external_extension_id, upstream_repository, upstream_commit_sha, upstream_path, artifact_sha256, marketplace_entry_sha256를 모두 pin하지만 invocation-time live catalog drift 검증은 현재 artifact digest와 commit SHA 두 필드만 비교합니다. 따라서 repository/path/marketplace digest 또는 catalog entry identity가 바뀌어도 invocation이 통과할 수 있어 PR의 “catalog drift cannot silently update an admitted extension” 계약을 완전히 만족하지 않습니다. Test-only exact 58f8bbadd6a4a3863d642883e40f4753f6dc291f에 네 누락 identity drift 회귀를 추가했습니다. Hosted RED가 실제 test execution까지 materialize된 뒤 full pinned-catalog identity를 최소 범위로 revalidate하고 exact-head GREEN을 다시 받기 전에는 merge authority가 없습니다.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Fresh Tool / Capability boundary review found one additional authority defect independent of the current catalog-drift RED. Recording it without mutating the test-only head so the hosted RED generation can finish first.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
A second runtime-authenticity defect remains after the activation finding: the exported AdmittedExternalExtension is also a structurally constructible interface. Recording this separately so the current hostile activation RED generation is not rewritten while queued.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Fresh Tool / Capability + Policy / Approval boundary review found one remaining authority gap. admitExternalExtension() authenticates six-field source identity and AppGuardrail/quarantine scan receipts, but every product grant is still taken from the untrusted ExternalExtensionDescriptor: approval_status, allowed_product_repositories, allowed_execution_roles, the validity window and egress/isolation grant are not bound to an independently trusted Noema Policy / Approval issuance. With an otherwise valid catalog entry and scan receipts, a caller can self-assert approval_status: "active" and broaden the allowed product/role set; the module-private WeakSet then proves only that this self-asserted descriptor passed admission, and activateExternalExtension() treats those fields as approval authority. That violates ADR-0015's invariant that one product cannot use another product's approval and that upstream/scanner evidence is not CWL approval authority. Add a hostile public admission→activation regression using unchanged source/catalog/scan pins but a self-broadened product grant, then bind the grant fields to a separately pinned Noema Policy / Approval authority/receipt. Do not overload Anthropic catalog metadata or AppGuardrail/quarantine receipts as product-approval truth.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Fresh exact-head review found one additional publication-time authority race after the existing owner-evidence repair. The finding is isolated to the async replay-digest boundary; no gate or foreign-owner boundary change is proposed.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Fresh review finding on exact a37304cca6cc14d18ea5018cf9f20b70ef3668da: ExternalExtensionInvocationRequest.instruction and observed_content are untrusted text, but invokeBoundary() validates only type/non-empty/policy phrases and imposes no UTF-8 byte ceiling. The public boundary then snapshots both strings and digestExternalExtensionInvocationEnvelope() serializes and TextEncoder-encodes the entire envelope before Web Crypto SHA-256. Because this API is exported and can be called independently of the already-bounded /exchange HTTP body path, an oversized invocation can force avoidable Worker heap/CPU work before rejection. This is directly inconsistent with the repository threat model's 128 MB isolate / oversized-input DoS boundary.
Add a hostile test first that uses multibyte UTF-8 text to exceed the chosen bounded invocation-text contract while staying below it in JavaScript character count, and prove rejection occurs before the digest provider is reached. Also keep an exact-boundary positive case so the limit is byte-based rather than UTF-16-code-unit based. Then add the smallest fail-closed byte bound at the Tool / Capability boundary; do not solve this only at one HTTP adapter, and do not weaken the existing Policy / Approval, owner-evidence, replay, or publication-time checks.
|
Current-head hosted RCA advanced. Exact |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
* test(docs): expose post-547 commercial authority drift * test(docs): require current active commercial lanes * docs: refresh commercial gap authority after #547 * test: advance patch-validator candidate authority * test: reject superseded patch-validator candidate * docs: refresh patch-validator exact authority * test: require current #535 commercial authority * docs: repair current commercial authority after hosted RED * test(docs): require live post-558 commercial authority Require the documentation lane to reflect protected #558 integration, the newly converged #535 exact head, and the newly observed #556 stacked head before production baseline text is repaired. * docs(gap): repair post-558 live commercial authority Bring #559's owned baseline in line with protected #558 integration, exact #535 convergence, and the newly observed #556 stack while preserving canonical owner boundaries and release-evidence discipline. * test(docs): require latest observed #556 authority Advance the documentation contract to the live #556 successor after its hosted release-test RED, while rejecting the superseded observation. Production baseline text follows in the causal repair commit. * docs(gap): record live #556 successor and hosted RED Update #559's sole documentation authority to the latest #556 exact head, preserve the observed hosted release-test failure as historical evidence, and keep the downstream stack non-authorizing until #535 reaches protected main. * test(docs): require current #535 endpoint-repair authority * docs: track current #535 gateway endpoint repair * test(docs): require #535 coverage-repair authority * docs: track #535 coverage-gate repair * test(docs): require current central control-plane snapshot * docs: refresh central control-plane snapshot * test(docs): retire stale commercial authority assertions * docs: refresh protected and claim-evidence authority * test(docs): track protected #535 integration * test(docs): track current claim-evidence head * test(docs): bind post-535 protected authority * test(docs): refresh current candidate contract * test(docs): align claim-binding authority wording * docs: refresh claim-evidence candidate authority * test(docs): match hosted run authority casing * docs: refresh live Noema commercial authority * test(docs): track current Noema feature authority * test(docs): bind commercial gap to live feature heads * test(docs): require settled external-extension authority * docs: converge commercial gap to current plugin admission * test(docs): track active policy-approval RED * docs: bind commercial gap to policy-approval RED * docs: restore durable commercial gap owners * docs: align external-extension authority after restack * test: bind documentation authority to restacked #560 * test(d(docs): require complete gap authority schema * docs: bind gap status to architecture authorities * test(docs): reject stale tool-capability candidate * docs: refresh current tool-capability evidence * docs: refresh active extension authority * docs: bind extension event chronology evidence * docs: refresh extension chronology authority * docs: bind gap baseline to hostile admission repair * test(docs): bind live candidate to admission repair * test(docs): preserve candidate ADR authority wording * docs: refresh external-extension runtime-time authority * test(docs): bind current runtime-time candidate authority * test(docs): refresh live external-extension candidate authority * docs: bind gap authority to invocation replay repair * docs: converge on public replay authority repair * docs: converge on activation revocation repair * docs: converge on admission-bound invocation authority * docs: bind commercial gap to exact admission provenance * test(docs): require exact-admission candidate authority * test(docs): bind candidate contract to exact admission * docs: track crypto-provider RED authority * test(docs): bind external-extension authority to Web Crypto repair * docs: converge external-extension Web Crypto authority * test(docs): follow current external-extension authority * test(docs): assert public receipt binding authority * test(docs): match activation revocation authority wording * docs: converge baseline after #560 protected integration * test: bind documentation authority to #560 integration * test: treat external-extension admission as protected history * test: extend protected integration history through #560 * test: move external-extension gap to lifecycle successor
1 participant
Why
Issue #545 is a customer-facing Tool / Capability gap: wholesale marketplace installation would turn third-party prompts, hooks, MCP servers, and network access into implicit runtime authority. Anthropic catalog/review metadata is discovery evidence, not CWL approval. Noema owns external-extension admission, product/role/time Policy / Approval, activation and invocation authority. AppGuardrail and quarantine-sandbox-runtime remain scanner/analysis/isolation owners, EgressWeave remains outbound owner, Keyverse remains identity/secret-handle owner, and provider/model routing remains
contextual-orchestrator.Retained RED → causal repair evidence
The branch preserves hosted hostile-case REDs before minimum causal fixes for immutable catalog identity, forged admission/activation/receipt authority, live scan/policy revocation, Policy / Approval scope issuance, activation chronology/runtime expiry, exact-admission provenance, replay equality/data retention, cryptographic-provider ownership, hostile accessor stability, independently owned scanner/quarantine evidence, invocation-time owner-evidence TOCTOU, publication-time authority drift across the asynchronous Web Crypto boundary, UTF-8 input bounding, and retained core edge-case coverage.
Recent authority evidence:
b52f5f10b6e757be45e1cab2a2dc1e8c38eb482cproduced hosted RED for owner-evidence profile TOCTOU. Productionef46c8703957d9a06759717d9d391ccefe2c0bf2moved the second owner-evidence read before the narrower core and passed one immutable validated snapshot;e4cd6d023e7f9a58bbce6e5c0ef5039bfc294889restored exact-head application coverage.fe2e8d4d786f534182ba3875cba498dec7e77e68produced hosted application RED34284153988, job102255685361: exact checkout/live-base/lockfile/install/typecheck passed, thentest/external-extension-publication-time-authority.test.tsfailed with 1 failed / 617 passed files and 4 failed / 4,229 passed tests because Policy / Approval revocation, catalog drift, AppGuardrail profile drift, and runtime expiry across Web Crypto still published accepted receipts. Minimum production repairc3786306bd42485c3db78dfb1b4ca5bd0ef8b586revalidates those live authorities immediately after the async digest boundary;a37304cca6cc14d18ea5018cf9f20b70ef3668dacarries the review/test cleanup.a37304c...found that the exported invocation seam accepted unboundedinstruction/observed_content, then canonicalized and UTF-8 encoded both before SHA-256. Test-only3fdd0f8c9b8a1972f2a483fe2800d128c8c60d24materialized the hosted application RED on CI34287093432: the 8,193-byte multibyte hostile cases reached the boundary without the required synchronous rejection. Production737f8700a66f01d1f52bf42bd48db5be39abf6bcadds the smallest Tool / Capability boundary repair: both fields are bounded to 8,192 UTF-8 bytes before replay-digest/Web Crypto work, with exact-boundary positive cases preserved.737f8700...then produced a distinct hosted coverage-only RED in application CI34288116426, job102268290127. Exact checkout, live-base verification, deterministic lock/install and typecheck passed, and all 618 files / 4,235 tests passed, but the mandatory 100% gate reported only the retained internal coreobserved_contentnon-string rejection (external-extension-admission-core.ts:978-979) as uncovered. Current test-only5aab7c098f3478069127f34e398326415ec599a4adds one direct core hostile case requiringobserved_content must be a string; production behavior, Policy / Approval, foreign-owner boundaries and thresholds are unchanged.Earlier retained hosted RED→repair generations for runtime-time authority, activation/invocation provenance, replay semantics, Web Crypto ownership, Policy / Approval accessor TOCTOU, activation-request TOCTOU, future-event chronology, stale fixtures, explicit owner-profile separation, and repository-minted policy fallback remain in branch history and review discussion.
Ownership and current authority
Current exact is test-only
5aab7c098f3478069127f34e398326415ec599a4, an ordinary non-force descendant of GitHub-verified protectedmain@36e5cf957ee20a8bb3e19ff50fea6c97771d2ba1. ADR 0015 remainsProposed. Protected.github/reviewer source is untouched, anddocs/product-technical-gap-baseline.mdremains solely owned by #559.context-graph-contracts#27remains the future shared external-capability contract owner and currently has no immutable release.appguardrail#1099remains scanner/provenance owner. AppGuardrail/quarantine evidence is not Noema approval; EgressWeave remains outbound owner; Keyverse remains identity/secret-handle owner;contextual-orchestratorremains provider/model-routing owner.Current exact gate generation is application CI
34289599257SUCCESS, reviewer-ci34289599291SUCCESS, required Security34289599289SUCCESS, and patch-validator-image34289599248IN_PROGRESS. The image job is assigned and currently building the exact-head patch-validator image; it is not passing evidence yet. Predecessor GREEN does not transfer.Two current review threads cover owner-evidence snapshot mediation and publication-time revalidation across the Web Crypto boundary. Their production repairs are present in this exact head, but they remain unresolved deliberately until this unchanged head reaches all four terminal-success gates and a fresh source/review/ancestry sweep confirms no new valid finding.
Keep Draft until the unchanged exact head has all four applicable gates terminal SUCCESS, those addressed threads are resolved only after fresh verification, review authority is clean, and ancestry still contains the live protected main. Source integration does not claim live plugin installation, immutable shared-contract consumption, released AppGuardrail/quarantine policy artifacts, measured pilot completion, immutable Noema release, rollback rehearsal, deployment, durable lifecycle persistence (#561), or buyer completion.
Summary by CodeRabbit
새로운 기능
문서
테스트