Cockpit C1 — Repair Job Authority Envelope & Merge Barrier - #14
Draft
LogicDuke wants to merge 8 commits into
Draft
Cockpit C1 — Repair Job Authority Envelope & Merge Barrier#14LogicDuke wants to merge 8 commits into
LogicDuke wants to merge 8 commits into
Conversation
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
C1-A01 (P2). `resolveJobOperation` resolved operation names through `OPERATION_LOOKUP.get(value)`. `Map.prototype.get` is looked up at call time, so a hostile replacement installed after module initialization could map any requested name onto a repair-authorizable one. Reproduced from the parent baseline: with `Map.prototype.get` returning `source.edit`, a valid repair-job envelope resolved `merge` to `source.edit` and produced ALLOW_ONCE / WITHIN_JOB_ENVELOPE with an execution permit issued. The same corruption applied to `auto_merge.enable` and to unmodeled names such as `shell.exec`. Remove the Map lookup entirely. Resolution is now an exact membership test against the existing frozen vocabularies via `containsValue`, which touches no prototype method, and the value returned on a hit is the caller's own string rather than one produced by a container. The resolver can therefore return only the exact requested name when it is modeled, or UNKNOWN_JOB_OPERATION. No runtime mechanism can substitute one operation name for another. Adds focused adversarial regression coverage under poisoned `Map.prototype.get`, restoring the captured descriptor in a finally block: merge stays merge, auto_merge.enable stays auto_merge.enable, shell.exec stays unknown, source.edit stays source.edit, merge cannot reach ALLOW_ONCE, unknown cannot reach ALLOW_ONCE, and a legitimate source.edit still authorizes byte-identically to its unpoisoned baseline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
C1-A01 — Harden operation resolution against prototype poisoning
C1-A02 (P2): `OperatorMergeAuthorization` / `operatorMergeAuthorizes` comments and the C1 architecture document claimed stronger guarantees than the implementation proved. The predicate proves structural binding only: readable required fields, a literal `singleUse === true` marker, and exact repository, pull-request, and current-HEAD SHA equality. It does not prove operator origin, human identity, authentication, trusted minting, signature or possession, uniqueness, one-time consumption, or replay prevention. A plain caller-written object literal passes, and the same record passes repeatedly because C1 has no consumed-capability store. Correct the claims without changing executable authorization semantics. A `true` result is now documented as a necessary binding check, not sufficient proof that a merge is operator-authorized; the future trusted operator boundary / merge broker remains responsible for authenticated operator origin, trusted minting provenance, and one-time consumption. Two focused tests pin the limitation so the documentation cannot drift from the implementation. Ordinary repair-job merge authority is unchanged: still OPERATOR_REQUIRED, mayExecuteOnce=false, permit=null. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
External review of PR #17 (Codex and CodeRabbit, one root cause) found that the repaired C1-A02 text still described `MergeTarget.currentHeadSha` as if C1 observed an authoritative live repository HEAD. It does not. `operatorMergeAuthorizes` performs no repository read, no GitHub API call, no adapter call, and no network access. It compares `authorization.headSha` against the caller-supplied `target.currentHeadSha` and nothing else, so the binding is only ever as fresh and as authoritative as the target handed to it. Correct the claims without changing executable authorization semantics: - `MergeTarget` fields are documented as caller-supplied input; the "repository's HEAD now, supplied by a trusted adapter" wording is gone. - The predicate's guarantee is stated against the supplied target, with target authoritativeness and freshness listed as not proved. - "a new HEAD requires a new operator decision" is removed. C1 requires only a newly matching candidate record; it cannot tell a fresh human decision from the same untrusted caller assembling another literal. - The architecture document gains an explicit list of what the future trusted Merge Broker must do, including obtaining the authoritative pull-request HEAD immediately before merge and consuming the capability atomically. One test title repeated the same false repository-observation claim and is corrected; assertions are unchanged. Ordinary repair-job merge authority remains OPERATOR_REQUIRED, mayExecuteOnce=false, permit=null. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…laims C1-A02 — Clarify operator merge authority guarantees
`readList` obtained each authorization-list entry with an ordinary indexed read, which walks the prototype chain. At a sparse hole that resolved whatever a custom array prototype — or `Array.prototype` itself — carried at that numeric key, so a value the operator never supplied could enter the trusted `RepairJobAuthorization` snapshot as an authorized path or command class and reach `ALLOW_ONCE` with an `ExecutionPermit` bound to the fabricated operand. Entries are now obtained through `readOwnElement`, which gates the read behind the module's already-captured `Object.hasOwn` and reports absence with a module-private sentinel rather than collapsing it into `undefined`, so the list refuses a missing element itself instead of relying on the element reader. A sparse hole rejects the whole list: never skipped, defaulted, or filled from the prototype chain. Dense own lists are unaffected. The guarantee is documented at the strength the code proves. It holds for any array whose own-property introspection is truthful; a Proxy defines the observable result of both the own check and the read, so one that misreports ownership can still pass an inherited value through. That widens nothing — such a caller can supply the same value as a dense own element — and the comment and architecture text now say so rather than claiming an atomic observation. The sentinel is a bare object literal, so it adds no call into a mutable global and keeps the module's captured-intrinsic discipline. Merge stays OPERATOR_REQUIRED, auto-merge stays DENY, and C1-A01 and C1-A02 are untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
C1-A03 — Reject inherited repair list elements
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
C1 establishes the pure TypeScript repair-job authority boundary for the future AgentBridge Cockpit.
It models:
C1 performs no filesystem, Git, GitHub, subprocess, network, persistence, workflow-state, or merge execution.
The historical V1 read-only boundary remains preserved. Future write authority is explicitly job-scoped and is not granted by this PR itself.
Status
DRAFT — BLOCKED BY CURRENT INDEPENDENT-AUDIT FINDINGS.
This PR is the protected C1 parent integration branch.
Known CURRENT findings at this baseline:
C1-A01 — P2
resolveJobOperationuses uncapturedMap.prototype.get; hostile prototype mutation can corrupt operation resolution and potentially turn forbidden/unknown operations into an allowed modeled operation.File:
src/domain/job-operation.tsC1-A02 — P2
OperatorMergeAuthorization/operatorMergeAuthorizesdocumentation overstates what is proven. Current C1 performs structural binding checks but does not authenticate operator identity/origin and does not enforce one-time consumption.Files:
src/domain/execution-permit.ts,docs/architecture/C1-repair-job-authority.mdC1-A03 — P3
readListuses ordinary indexed reads; a sparse authorization list plus a prototype-planted numeric property can fabricate authorization scope.File:
src/domain/repair-job.tsThese findings MUST NOT be repaired directly on this protected parent branch.
Each finding will enter an isolated repair branch/worktree and stacked validation PR targeting this branch.
Quarantine rule
Finding
→ verify against CURRENT C1 HEAD
→ isolated repair branch/worktree
→ bounded repair
→ stacked validation PR
→ independent review
→ complete validation
→ operator merge decision
→ re-audit parent HEAD
No implementing agent is its own sole validator.
Merge boundary
MERGE IS OPERATOR-ONLY.
No AI agent is authorized to merge this PR or any stacked repair PR.
No auto-merge.
Baseline verification
Re-run against this exact baseline commit immediately before it was created:
npm run typechecknpm run lintnpm testnpm run buildnpm auditgit diff --check/git diff --cached --checkPassing tests are evidence, not proof; the independent audit findings above remain blocking.
Baseline scope
Exactly ten files, 4614 insertions, 0 deletions. No dependency,
package.json,package-lock.json, CI,tsconfig,eslint, orvitestchange. No PR #9 or PR #10 content.src/domain/index.ts(additive re-exports only)src/domain/repair-job.tssrc/domain/job-operation.tssrc/domain/execution-permit.tssrc/domain/job-authorization.tstests/domain/repair-job-fixtures.tstests/domain/job-authorization.test.tstests/domain/job-authorization-invariants.test.tstests/domain/execution-permit.test.tsdocs/architecture/C1-repair-job-authority.md🤖 Generated with Claude Code