From 9287467860a84ac2554b5d3f3d0e188f9fb8c4da Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Sat, 15 Aug 2026 22:23:23 +0200 Subject: [PATCH 01/15] feat: add cockpit C1 authority boundary baseline Co-Authored-By: Claude Opus 5 (1M context) --- docs/architecture/C1-repair-job-authority.md | 501 ++++++++++++ src/domain/execution-permit.ts | 355 +++++++++ src/domain/index.ts | 56 ++ src/domain/job-authorization.ts | 531 +++++++++++++ src/domain/job-operation.ts | 444 +++++++++++ src/domain/repair-job.ts | 725 +++++++++++++++++ tests/domain/execution-permit.test.ts | 409 ++++++++++ .../job-authorization-invariants.test.ts | 737 ++++++++++++++++++ tests/domain/job-authorization.test.ts | 636 +++++++++++++++ tests/domain/repair-job-fixtures.ts | 220 ++++++ 10 files changed, 4614 insertions(+) create mode 100644 docs/architecture/C1-repair-job-authority.md create mode 100644 src/domain/execution-permit.ts create mode 100644 src/domain/job-authorization.ts create mode 100644 src/domain/job-operation.ts create mode 100644 src/domain/repair-job.ts create mode 100644 tests/domain/execution-permit.test.ts create mode 100644 tests/domain/job-authorization-invariants.test.ts create mode 100644 tests/domain/job-authorization.test.ts create mode 100644 tests/domain/repair-job-fixtures.ts diff --git a/docs/architecture/C1-repair-job-authority.md b/docs/architecture/C1-repair-job-authority.md new file mode 100644 index 0000000..19bd966 --- /dev/null +++ b/docs/architecture/C1-repair-job-authority.md @@ -0,0 +1,501 @@ +# Repair Job Authority Envelope and Merge Barrier (Cockpit C1) + +Status: V1. Superseded only by an explicit architecture decision. + +Numbered outside the `001`–`006` series on purpose: C1 is the first Cockpit +bootstrap record, not the next step of the V1 pipeline, and it must not collide +with the numbering the Autoflow and transport PRs are using. + +## Purpose + +C1 establishes what a bounded autonomous repair job **may be authorized to do**, +before AgentBridge has repository-write or agent-execution capability. + + trusted job configuration + exact operation operands -> authorization decision + +It answers exactly one question: + +> May *this* bounded repair job perform *this exact operation*, with *these +> exact operands*, once? + +**C1 performs no operation.** No filesystem, git, worktree, commit, push, GitHub +API, pull-request creation, reviewer trigger, merge, subprocess, `child_process`, +shell, PowerShell, Bash, process transport, network, database, SQLite, Evidence +Store, REST API, HTTP server, dashboard, frontend, queue, poll, retry, timer, +scheduler, Promise, or async of any kind. No clock is read and no identifier is +generated. `authorizeJobOperation` is a pure function of its two arguments. + +The security boundary is deliberately established **first**, so that a later +execution layer has to be written through it rather than around it. + +## Relationship to the frozen V1 architecture + +The V1 statement stands unchanged: *managed repositories remain read-only.* C1 +does not rewrite V1 as write-capable, and nothing in this PR grants any write. + +What C1 adds is an **explicit future authority model**. A later Cockpit +execution layer **may** receive narrowly scoped repair authority, and if it ever +does, it may only receive it through this boundary. Outside a valid repair-job +authorization envelope, the existing read-only boundary is exactly as it was. + +C1 imports nothing from PR 002–PR 006 except for a documented bound-parity test, +and changes none of them: + +| Layer | Owns | C1 | +| --- | --- | --- | +| PR 002 | action taxonomy, deterministic classification, unknown fails closed | unchanged, un-imported | +| PR 003 | `ActionRequest`, `PolicyGate`, agent requests are not authority, human `ApprovalRecord` is separate | unchanged, un-imported | +| PR 004 | commit-bound evidence, `CURRENT`/`STALE`/`INVALID` freshness | unchanged, un-imported | +| PR 005 | provider-neutral review ingestion, findings are evidence not authority | unchanged, un-imported | +| PR 006 | provider-neutral `AgentInvocation`, provider identity inert, claims untrusted | unchanged, un-imported | + +C1 answers a different question from each of them. It does not reclassify +actions, does not judge freshness, does not ingest findings, and does not record +invocations. + +**C1 contains no workflow state machine.** No Autoflow implementation, no +states, no transitions, no `READY_FOR_MERGE`. The proposed Autoflow state +machine lives in its own PR and this branch neither reads from nor depends on +it. + +## The core invariant + +> A repair job is a bounded capability envelope. Authority is derived from +> trusted job configuration and exact operation operands, and from nothing else. + +Agent identity, provider name, rationale, prose, metadata, claimed success, and +privileged-sounding labels cannot increase authority. The strongest form of that +guarantee is structural rather than defensive: **there is no field typed to +accept them.** `RepairJobAuthorization` has no metadata bag. `JobOperationRequest` +has no `agentId`, `providerId`, `rationale`, `metadata`, `role`, `approval`, +`priority`, `confidence`, or `override`. The normalizer reads no such key, so a +value planted at runtime has nowhere to land and nothing to influence. + +Everything fails closed. + +## Trust boundary + +| Input | Trust | Contributes | +| --- | --- | --- | +| `RepairJobAuthorization` | **Trusted configuration** | the entire authority envelope | +| `JobOperationRequest` | **Untrusted** | which operation, and which exact operands | + +The request's binding fields (`jobId`, `repositoryId`, `parentPullRequestId`, +`parentHeadSha`) are *claims*, never authority. Their only purpose is to make a +request that names the wrong repository, pull request, job, or commit +**refusable** instead of silently re-targeted at whatever the job happens to +say. Each is compared by exact string equality against the trusted snapshot, and +an absent claim fails exactly like a wrong one. + +The job envelope itself is trusted but still read defensively, following PR 004's +treatment of `EvidenceTarget`: trusted does not mean dereferenceable without +care. + +## The authorization envelope + +`RepairJobAuthorization` binds one repair job to: + +| Field | Binds | +| --- | --- | +| `jobId` | the job's own identity | +| `policyVersion` | which policy authorized the envelope; part of permit identity | +| `repositoryId` | the one repository this job may ever touch | +| `parentPullRequestId` | the protected parent feature pull request | +| `protectedParentRef` | the parent integration ref, which no operation may write | +| `parentHeadSha` | the exact commit the job is bound to | +| `findingSource` | where the finding came from; a provider-neutral label, inert | +| `findingId` | the finding being repaired | +| `findingHeadSha` | the commit the finding was verified against | +| `repairBranch` | the isolated repair branch | +| `repairWorktreeId` | the isolated repair worktree | +| `authorizedPaths` | exact repository-relative paths, not a prefix or glob | +| `authorizedCommandClasses` | verification classes, never command strings | +| `repairAgentId` | who repairs; audit only, never authority | +| `independentValidatorId` | who must independently validate | + +Every field is required. Trusted configuration is **all-or-nothing**: there is +no partially configured job, and no field that degrades silently. A job with any +invalid field authorizes nothing, and `findInvalidRepairJobFields` reports the +offending names in declaration order. + +Two structural invariants are enforced as configuration validity rather than as +a runtime check that could be forgotten: + +- `repairBranch !== protectedParentRef`. A job whose repair branch *is* the + protected parent is not a quarantined repair; it is a direct write to + protected history wearing a repair job's name. +- `independentValidatorId !== repairAgentId`. A repair agent that is its own + validator defeats the quarantine the whole pipeline exists to enforce. + +One relationship is enforced at authorization time, because it is about +freshness rather than shape: `findingHeadSha` must equal `parentHeadSha`, or +every operation is denied `FINDING_SHA_STALE`. A repair derived from a finding +about some other commit is a repair of something that may no longer be there. +PR 004 remains the owner of `CURRENT` versus `STALE` for evidence; this is the +narrower structural check that the job's own two SHAs agree, which C1 can decide +without importing that kernel or producing a second answer to PR 004's question. + +## Operations are structured, not named + +A generic action name is not sufficient for Cockpit write authority. There is no +`repository.write`, no `git.run`, and no `shell.exec`: an operation whose +authority cannot be checked against an exact operand has no place in the model. + +| Operation | Required operands | Authorized when | +| --- | --- | --- | +| `source.read` | worktree, path | worktree is the repair worktree and path is in scope | +| `source.edit` | worktree, path | worktree is the repair worktree and path is in scope | +| `verification.run` | worktree, command class | class is modeled *and* configured for this job | +| `repair.commit` | worktree, ref | ref is exactly the repair branch | +| `repair.push` | ref, non-force | ref is exactly the repair branch and the push is not forced | +| `repair.change_request` | source ref, target ref | repair branch → protected parent ref | + +`repair.change_request` is the **only** operation that may name the protected +parent ref, and only as a change-request *target*. Opening a change request +against a ref does not mutate it: the parent stays untouched until an operator +merges. Every write-shaped operation additionally denies +`PROTECTED_REF_MUTATION` specifically when the parent ref is named, rather than +falling through to the generic "not the repair branch" refusal, so the audit +record distinguishes a mistake from an escape attempt. + +## Decision model + +| Decision | Meaning | +| --- | --- | +| `ALLOW_ONCE` | this exact normalized operation, under this exact job binding, may be executed **once**, under the accompanying `ExecutionPermit` | +| `DENY` | refused; nothing at this layer converts it into an allow | +| `OPERATOR_REQUIRED` | outside every autonomous envelope; only a human operator, through a separate type, could ever authorize it | + +`ALLOW_ONCE` is not a standing permission and does not generalise to a similar +operation, a later HEAD, or another job. `OPERATOR_REQUIRED` is **not** "escalate +and retry": the evaluator never returns a permit alongside it, and there is no +argument through which an approval could arrive to change it. + +The vocabulary deliberately does not reuse PR 003's `ALLOW`/`ESCALATE`/`DENY` or +its `AUTONOMOUS` outcome. Those answer "what is this action?" for a read-only V1; +this answers "may this bounded job perform this exact operation once?", which is +a different question over different operands. Reusing the words would invite a +later reader to treat the two as interchangeable. + +`ApprovalRecord` is **not** reused to represent automatic Cockpit permission. It +is human decision data about a PR 003 `ActionRequest`; making it double as a +machine authorization would turn every existing approval into a candidate +capability. The automatic machine authorization is `ExecutionPermit`, a separate +type with separate identity. + +Every refusal carries a stable, machine-readable reason: +`MERGE_IS_OPERATOR_ONLY`, `OPERATION_FORBIDDEN`, `OPERATION_UNKNOWN`, +`OPERATION_UNREADABLE`, `JOB_ENVELOPE_INVALID`, `JOB_MISMATCH`, +`REPOSITORY_MISMATCH`, `PARENT_PULL_REQUEST_MISMATCH`, `PARENT_HEAD_MISMATCH`, +`FINDING_SHA_STALE`, `OPERAND_MISSING`, `PATH_MALFORMED`, `PATH_NOT_AUTHORIZED`, +`WORKTREE_NOT_AUTHORIZED`, `COMMAND_CLASS_NOT_AUTHORIZED`, +`PROTECTED_REF_MUTATION`, `REF_NOT_REPAIR_BRANCH`, +`CHANGE_REQUEST_TARGET_INVALID`, `FORCE_PUSH_FORBIDDEN`. + +## The merge barrier + +**Merge is operator-only. This is a permanent AgentBridge Cockpit invariant +unless an explicit later architecture decision changes it.** + +There must be no path where an agent request, plus a repair job, plus provider +identity, plus metadata, plus a human `ApprovalRecord`, produces an autonomous +merge permission. C1 makes that structural rather than conventional, on five +independent levers, any one of which would be sufficient: + +1. **Type level.** `merge` is not a member of `RepairAuthorizableOperation`. + `ExecutionPermit.operation` is typed to that union, so **a merge permit does + not type-check.** A test asserts the compile error with `@ts-expect-error`. +2. **Single allow site.** `ALLOW_ONCE` is produced at exactly one `return` in + the codebase, reachable only after two type guards have narrowed the + operation to `RepairAuthorizableOperation`. +3. **First check.** The merge check is the first decision made, above the job + envelope validation and every binding and operand check, so no envelope, + binding, or operand state can precede or condition it. +4. **No approval parameter.** `authorizeJobOperation` takes exactly two + arguments. There is no parameter through which an `ApprovalRecord` — + approved or otherwise — can reach the evaluator. A test pins the arity. +5. **No readable identity.** Nothing reads an agent id, provider id, rationale, + or metadata, because no such field exists on either argument. + +The maximum autonomous state a future workflow may reach is therefore "ready for +an operator to merge". **C1 implements no such state and no state machine**; it +establishes only that ordinary job authority has no permission that could become +merge. + +`auto_merge.enable` is `DENY`, not `OPERATOR_REQUIRED`, and the distinction is +deliberate: enabling auto-merge delegates the merge decision away from the moment +HEAD is final. An operator asking for auto-merge is asking to not be the +operator. + +### Mandatory hard denials + +Modeled explicitly, so refusing them is a deterministic decision with a stable +reason rather than an accident of falling through to `unknown`: + +`merge` (operator-only), `auto_merge.enable`, `parent_ref.write`, `push.force`, +`history.rewrite`, `branch.delete`, `policy.modify`, `secret.access`, +`deployment.run`, `staging.change`, `production.change`, `database.write`, +`database.migrate`. + +Cross-repository, cross-pull-request, and wrong-HEAD operations are refused by +binding rather than by name, because they are not distinct operations — they are +in-scope operations pointed somewhere else. Unrelated file writes are refused by +file scope. Unknown operations are refused as `OPERATION_UNKNOWN`. + +Force push is denied twice over: by name as `push.force`, and as an operand +check on `repair.push` that runs *before* the ref is examined, so a forced push +to the authorized repair branch is refused for being forced rather than +accidentally allowed. The force flag fails closed: only an absent or literally +`false` value is not a force, so `0`, `''`, `null`, `'false'`, and an object are +all forces. + +A privileged provider or agent label — `root`, `system`, `admin`, +`agentbridge-internal` — changes none of these outcomes. A test sweeps every +label across every job shape and every operand shape. + +### Operator merge authority, defined but not built + +`OperatorMergeAuthorization` records the shape of the only thing that may ever +authorize a merge. **No function in AgentBridge produces one.** There is no +factory, no builder, and no evaluator output that contains one. The boundary that +turns a human decision into a record of this shape does not exist yet, and +building it is an explicit later decision rather than an implementation detail of +whichever layer needs it first. + +`operatorMergeAuthorizes` enforces every required property: operator-originated, +repository-bound, pull-request-bound, exact-HEAD-SHA-bound, structurally +single-use, invalid the moment HEAD changes, and incapable of authorizing another +pull request or a future SHA. + +**No merge executor and no GitHub merge API call exists in this PR.** + +## Execution permits + +An `ExecutionPermit` is the record of one authorization: exactly one job, exactly +one normalized operation, exactly one operand set, bound to the job's repository, +parent pull request, parent HEAD, and policy version at the moment of decision. + +**A permit is not a bearer token.** `permitAuthorizes` re-derives the entire +decision from the trusted job and the untrusted request and then compares, so a +permit only ever authorizes what the evaluator would authorize at the moment of +use. The anti-forgery property follows, and is worth stating in the form it +actually holds: + +> A forged permit that passes re-verification is a permit the evaluator would +> have issued anyway. + +Forgery therefore buys nothing, and a permit widens no authority — it records +authority already derived from trusted configuration. + +### Single use + +Single use is stated structurally. `singleUse` is typed as the literal `true` and +`scope` as the literal `'exactly-one-execution'`, so neither can be widened by +assignment; the object is frozen; and there is no `expiresAt`, `ttl`, `uses`, +`remaining`, `renew`, `refresh`, or `reusable` field for a consumer to read as a +standing right. A test asserts the exact key set. + +**C1 stores nothing and consumes nothing.** What C1 guarantees is that the +*identity* of a permit is a total function of the exact execution it authorizes, +so a consumer that records consumed `permitId`s can detect a replay rather than +being unable to distinguish one. `permitAuthorizes` reports whether a permit is +*valid*; it never reports whether it is *unused*, and it does not pretend to. + +### Permit identity + +`permitId` is derived deterministically — no clock, no randomness, no counter — +matching the purity of every other AgentBridge domain layer. It is **not a +nonce**: two authorizations of byte-identical executions produce the same id, +which is exactly the property that makes replay detectable. + +Two legitimate executions of the same operation are distinguished by `requestId`, +which the caller mints per attempt and which participates in permit identity. +`requestId` confers no authority: an agent that mints a fresh one obtains exactly +the authority it already had, for one more execution of an operation the job +already authorizes. + +The encoding is length-prefixed (`:`), so no operand value can inject +a delimiter and make one execution's id collide with another's. Identity covers +the policy version, job, repository, parent pull request, parent HEAD, request +id, operation, and every operand the operation defines. + +A permit carries **only** the operands its operation defines; every other operand +is `null`. A `source.read` request that also names the protected parent ref +produces a permit whose `ref` is `null`, so an unused operand cannot ride along +into execution. + +## Authorized file scope + +`repository.write` is not modeled at all, so there is no permission that means +"edit the entire repository". A future edit authorization is evaluated against +the **actual requested path**. + +`authorizedPaths` is a list of exact repository-relative paths — not a prefix, +glob, or directory. Directory authority would require normalisation and +containment guarantees this pure model cannot prove, and a `src/a` versus +`src/ab` prefix boundary is a classic escape. An empty list is legitimate: it +describes a verification-only job. + +`readRepositoryRelativePath` rejects, without exception: non-strings, empty +strings, values over 1 024 characters, any `.` or `..` segment, a leading `/`, +an empty segment, a trailing `/`, a leading `~`, `\` anywhere, `:` anywhere, +control characters including NUL, a `.git` segment at any depth in any ASCII +case, and any segment with a leading space or a trailing space or dot. Both the +configured paths and the requested path go through the same reader, and the same +reader is applied all-or-nothing to job configuration: one bad path invalidates +the whole scope rather than leaving a job that looks configured but is not the +one an operator wrote. + +### What path containment does and does not prove + +Stated precisely, because overclaiming here would be worse than not checking: + +**Proved.** The value is a string matching a conservative repository-relative +shape, and it is exactly equal to a path an operator configured. No traversal, +absolute path, drive-absolute path, alternate data stream, NUL truncation, or +`.git` access can be expressed at all. + +**Not proved, and not claimed.** That two equal strings name the same file. A +pure model cannot know about case-insensitive or case-preserving filesystems, +Unicode normalisation applied by the filesystem, symbolic links, hard links, bind +mounts, or junctions. **A future execution layer must re-verify containment +against the real filesystem it is about to touch.** This reader narrows the +input; it does not sandbox it. + +No normalisation of any kind is performed. `SRC/A.TS` is not `src/a.ts` here, and +whether it is on disk is the executor's problem to solve with the filesystem, not +this layer's to guess. + +## Command authority + +C1 never authorizes a shell command string. It authorizes a **class** — `test`, +`lint`, `typecheck`, `build`, `audit` — and a later execution layer resolves a +class to a concrete command through repository policy. There is no field on this +boundary that can carry a command line, argument vector, environment, or shell, +and a request naming `npm test`, `test; rm -rf /`, `sh`, or `powershell` is +refused as an unauthorized class. + +The five classes mirror the verification actions PR 002 already classifies +read-only. C1 does not import that taxonomy — a class here is an authorization +label, not an action kind — but the vocabularies are kept aligned so the two +layers cannot disagree about what verification means. + +Authorization requires two independent conditions: the class must be one C1 +models at all, *and* the job must have been configured to permit it. An +unmodeled class in job configuration invalidates the job rather than being +quietly accepted. + +**No command execution, shell parsing, subprocess spawning, PowerShell, Bash, or +process transport exists in this PR.** + +## Protected parent and stacked validation quarantine + +The mandatory AgentBridge pattern is preserved: + + finding + -> verify against CURRENT HEAD + -> bounded repair specification + -> isolated repair branch and worktree + -> repair agent + -> stacked validation PR targeting the protected parent feature PR + -> independent review + -> CI / typecheck / lint / build / tests + -> policy and evidence gate + -> ready for an operator + -> operator decision + +C1 implements none of that workflow. It encodes only the minimal authority +invariants that stop a later layer from bypassing the quarantine by accident: + +- The protected parent ref is never a write target of any operation. +- Filesystem-shaped operations are bound to the repair worktree, so an edit + cannot land in the parent's checkout. +- The stacked change request must run from the repair branch to the protected + parent ref, in that direction. A change request *from* the parent is refused, + and one targeting an integration branch directly is refused. +- A repair agent cannot become its own sole validator: the job is invalid if + `independentValidatorId` equals `repairAgentId`, and + `satisfiesIndependentValidator` consults **only** `validatorId` against the + trusted configuration. A claimed role, a claimed provider, and a + privileged-sounding label satisfy nothing. + +## Hostile runtime + +All runtime input crossing this boundary is treated as hostile, following the +patterns PR 004, PR 005, and PR 006 established: + +- **Captured intrinsics.** `Object.freeze`, `Object.defineProperty`, + `Object.hasOwn`, `Array.isArray`, `Number.isInteger`, `String`, + `String.prototype.trim`, `String.prototype.charCodeAt`, and `Reflect.apply` + are captured at module load, before any untrusted access is possible. String + methods are captured unbound and invoked through `Reflect.apply`, so neither a + poisoned prototype method nor a poisoned `Function.prototype.call` is on the + path. +- **Own-property reads.** Every untrusted property is read own-only. An + inherited value, including one planted on `Object.prototype` through a + `__proto__` payload, is treated as absent. +- **Guarded reads.** Every read is wrapped, because a getter or Proxy trap may + throw. `Array.isArray` itself is guarded, because it throws on a revoked + Proxy. +- **Trusted snapshots, read exactly once.** Both arguments are read into frozen + snapshots before any decision is made, and everything downstream reads only + the snapshot. A getter that returns a different value on each access cannot + validate one operand and have another reach the decision or the permit. Tests + pin this for the path, the ref, and the authorized-path list. +- **No validation TOCTOU.** The single `readRepairJobAuthorization` pass is both + the validator and the snapshot builder, so `findInvalidRepairJobFields` and + the evaluator cannot drift apart — there is no second implementation to + diverge. +- **Bounds before iteration.** 256-character identifiers, 1 024-character paths, + 512 authorized paths, 64 path segments, 16 command classes. A hostile + `length` — including a Proxy reporting `Number.MAX_SAFE_INTEGER` — is refused + rather than iterated. +- **Identifiers reject, never truncate.** A truncated identifier is worse than + no identifier: git resolves commit prefixes, so a cut SHA can falsely match a + real object and a cut branch name can name a different ref. **C1 truncates + nothing at all, because C1 has no prose field.** An oversized list is likewise + rejected, not shortened. +- **Sparse arrays reject.** A hole reads as `undefined`, which no reader accepts, + so sparseness rejects rather than collapsing. +- **Array building avoids the prototype.** Appends define an own indexed + property rather than using `push` or indexed assignment, so an inherited index + setter is not on the path. +- **Fail closed without throwing.** Every entry point is total. A non-object, a + revoked Proxy, a record of throwing getters, and a payload of wrong types all + yield a refusal, never an exception. +- **Prose and metadata create no authority.** Not because they are ignored, but + because there is no field to put them in. + +Existing hardened readers were **not** refactored to share helpers with C1. The +boundary independence recorded in PR 006 is intentional and takes precedence over +deduplication. C1's four modules do share readers with each other, because they +are one boundary rather than four. + +One cross-boundary convention is pinned by test rather than by import: +`JOB_BOUNDS.MAX_IDENTIFIER_LENGTH` equals `INVOCATION_BOUNDS.MAX_IDENTIFIER_LENGTH`, +because a `jobId` may be correlated with an `invocationId`, and two ids sharing a +256-character prefix must never be able to collapse into one. + +## Determinism + +Given identical arguments, authorization produces byte-equivalent output: no +clock, no randomness, no filesystem, no network, no environment, no mutable +global state, no identifier generation. `permitId` derives from the exact +execution, so no hashing dependency is required. Invalid-field names preserve +declaration order, and nothing is sorted, grouped, or deduplicated. Decisions, +permits, operand records, and all lists are frozen, so an authorization cannot be +rebound to another job, operand, or commit after the fact. + +## What C1 deliberately does not contain + +No Autoflow workflow or state implementation. No `READY_FOR_MERGE` +implementation. No Claude, Codex, or CodeRabbit adapter. No process transport, +`child_process`, filesystem I/O, git execution, worktree creation, commit, push, +GitHub API, pull-request creation, reviewer triggering, or network access. No +merge executor and no auto-merge. No SQLite, Evidence Store persistence, REST +API, HTTP server, dashboard, or frontend. No polling, queues, async +orchestration, retry loops, timers, or schedulers. No executor of any kind: C1 +models and evaluates authorization and nothing else. + +This is one layer of a boundary that does not exist yet, established before the +capability it bounds. diff --git a/src/domain/execution-permit.ts b/src/domain/execution-permit.ts new file mode 100644 index 0000000..b556074 --- /dev/null +++ b/src/domain/execution-permit.ts @@ -0,0 +1,355 @@ +/** + * One-time execution permits, and the separate operator merge authority + * (Cockpit C1). + * + * ## A permit is not a bearer token + * + * An {@link ExecutionPermit} is the record of a single authorization: exactly + * one job, exactly one normalized operation, exactly one set of operands, bound + * to the job's repository, parent pull request, parent HEAD, and policy + * version at the moment the decision was made. + * + * **It is not trusted on its face.** `permitAuthorizes` in + * `job-authorization.ts` re-derives the whole decision from the job and the + * request, and compares. The security property that follows is precise and + * worth stating in the form it actually holds: + * + * > A forged permit that passes re-verification is a permit the evaluator + * > would have issued anyway. + * + * Forgery therefore buys nothing. A permit widens no authority; it records + * authority already derived from trusted configuration. + * + * ## Single use + * + * A permit represents permission for **one** execution. It is not a standing + * "yes", and the model says so structurally: there is no `expiresAt`, no + * `uses`, no `remaining`, no `renew`, no `refresh`, and no scope wider than one + * operation. `singleUse` is typed as the literal `true` and `scope` as the + * literal `'exactly-one-execution'`, so neither can be widened by assignment, + * and the object is frozen. + * + * C1 stores nothing and consumes nothing — there is no persistence in this PR. + * What C1 guarantees is that the *identity* of a permit is a total function of + * the exact execution it authorizes, so a consumer that records consumed + * `permitId`s can detect a replay rather than being unable to distinguish one. + * + * ## Permit identity + * + * `permitId` is derived deterministically — no clock, no randomness, no + * counter, matching the purity of every other AgentBridge domain layer. It is + * **not a nonce**: two authorizations of byte-identical executions produce the + * same id, which is the property that makes replay detectable. + * + * Two legitimate executions of the same operation are distinguished by + * `requestId`, which the caller mints per attempt and which participates in + * permit identity. `requestId` confers no authority: an agent that mints a + * fresh one obtains exactly the authority it already had, for exactly one more + * execution of an operation the job already authorizes. + * + * The encoding is length-prefixed, so no operand value can inject a delimiter + * and make one execution's id collide with another's. + */ + +import type { RepairJobSnapshot } from './repair-job.js'; +import { append, readExactIdentifier, readOwnProperty } from './repair-job.js'; +import { + operandValues, + type PermitOperands, + type RepairAuthorizableOperation, +} from './job-operation.js'; + +const objectFreeze = Object.freeze; +// `String` performs the abstract number-to-string conversion; it does not look +// up `Number.prototype.toString`, so a poisoned prototype is not on the path. +const stringOf = String; + +/** Version tag of the permit identity encoding. Changing it invalidates every id. */ +const PERMIT_ID_SCHEME = 'abp1'; + +/** + * Encode parts into a delimiter-injection-proof string. + * + * Each part is written as `:`, so a part containing `:`, `|`, or + * any other separator cannot be mistaken for a boundary and two different + * executions cannot be encoded to the same id. + */ +function encodeParts(parts: readonly string[]): string { + let encoded = PERMIT_ID_SCHEME; + for (let index = 0; index < parts.length; index += 1) { + const part = parts[index] ?? ''; + encoded += '|' + stringOf(part.length) + ':' + part; + } + return encoded; +} + +/** + * A permit for exactly one execution of exactly one operation. + * + * Every field is a primitive, a literal, or a frozen object of primitives, so + * the record is JSON-serializable and survives a round trip unchanged. + * + * `operation` is typed {@link RepairAuthorizableOperation}, which does not + * include `merge` or any other forbidden operation. **A merge permit does not + * type-check.** This is the type-level half of the merge barrier; the evaluator + * enforces the runtime half. + */ +export interface ExecutionPermit { + /** Deterministic identity of this exact authorization. */ + readonly permitId: string; + /** Policy version that produced the decision. Part of permit identity. */ + readonly policyVersion: string; + /** The one job this permit belongs to. */ + readonly jobId: string; + /** The one repository this permit is valid in. */ + readonly repositoryId: string; + /** The one parent pull request this permit is valid under. */ + readonly parentPullRequestId: string; + /** The exact parent HEAD this permit was issued against. */ + readonly parentHeadSha: string; + /** The caller-minted identity of this one execution attempt. */ + readonly requestId: string; + /** The one operation this permit authorizes. Never a forbidden operation. */ + readonly operation: RepairAuthorizableOperation; + /** The exact operands. Every operand the operation does not define is `null`. */ + readonly operands: PermitOperands; + /** Structural: this permit authorizes one execution, not a standing right. */ + readonly singleUse: true; + /** Structural: there is no wider scope this permit could be widened to. */ + readonly scope: 'exactly-one-execution'; +} + +/** + * Mint a permit for an already-authorized operation. + * + * **Not exported from the domain index.** The evaluator calls it at its single + * `ALLOW_ONCE` return site, after every check has passed. It is not a + * capability: its `operation` parameter cannot be a forbidden operation, and a + * permit it produced outside the evaluator would fail re-verification unless + * the evaluator would have produced it anyway. + */ +export function issueExecutionPermit( + job: RepairJobSnapshot, + operation: RepairAuthorizableOperation, + requestId: string, + operands: PermitOperands, +): ExecutionPermit { + const parts: string[] = []; + append(parts, job.policyVersion); + append(parts, job.jobId); + append(parts, job.repositoryId); + append(parts, job.parentPullRequestId); + append(parts, job.parentHeadSha); + append(parts, requestId); + append(parts, operation); + const values = operandValues(operands); + for (let index = 0; index < values.length; index += 1) { + append(parts, values[index] ?? ''); + } + + return objectFreeze({ + permitId: encodeParts(parts), + policyVersion: job.policyVersion, + jobId: job.jobId, + repositoryId: job.repositoryId, + parentPullRequestId: job.parentPullRequestId, + parentHeadSha: job.parentHeadSha, + requestId, + operation, + operands, + singleUse: true, + scope: 'exactly-one-execution', + }); +} + +/** + * Compare a candidate permit against a freshly issued one, field by field. + * + * The candidate is **untrusted**: it may be a hostile object, a Proxy, or a + * record with throwing getters. Every property is read own-only, once, and + * guarded, so a candidate that changes under observation cannot match on one + * read and be used on another. + * + * `singleUse` and `scope` are compared too. A candidate that omits them, or + * that carries a widened value, is not equal to a permit this layer issued. + */ +export function permitsEqual(candidate: ExecutionPermit, issued: ExecutionPermit): boolean { + const record: unknown = candidate; + if (typeof record !== 'object' || record === null) { + return false; + } + + // Compared as a raw own value, not through the identifier reader: an encoded + // permit id concatenates several identifiers and legitimately exceeds the + // single-identifier bound. + if (readOwnProperty(record, 'permitId') !== issued.permitId) { + return false; + } + if (readOwnProperty(record, 'policyVersion') !== issued.policyVersion) { + return false; + } + if (readOwnProperty(record, 'jobId') !== issued.jobId) { + return false; + } + if (readOwnProperty(record, 'repositoryId') !== issued.repositoryId) { + return false; + } + if (readOwnProperty(record, 'parentPullRequestId') !== issued.parentPullRequestId) { + return false; + } + if (readOwnProperty(record, 'parentHeadSha') !== issued.parentHeadSha) { + return false; + } + if (readOwnProperty(record, 'requestId') !== issued.requestId) { + return false; + } + if (readOwnProperty(record, 'operation') !== issued.operation) { + return false; + } + if (readOwnProperty(record, 'singleUse') !== true) { + return false; + } + if (readOwnProperty(record, 'scope') !== 'exactly-one-execution') { + return false; + } + + const rawOperands = readOwnProperty(record, 'operands'); + if (typeof rawOperands !== 'object' || rawOperands === null) { + return false; + } + if (readOwnProperty(rawOperands, 'force') !== false) { + return false; + } + if (readOwnProperty(rawOperands, 'worktreeId') !== issued.operands.worktreeId) { + return false; + } + if (readOwnProperty(rawOperands, 'path') !== issued.operands.path) { + return false; + } + if (readOwnProperty(rawOperands, 'commandClass') !== issued.operands.commandClass) { + return false; + } + if (readOwnProperty(rawOperands, 'ref') !== issued.operands.ref) { + return false; + } + if (readOwnProperty(rawOperands, 'sourceRef') !== issued.operands.sourceRef) { + return false; + } + if (readOwnProperty(rawOperands, 'targetRef') !== issued.operands.targetRef) { + return false; + } + return true; +} + +/** + * A merge authorization that originated from a human operator. + * + * **This is not job authority and is not reachable from job authority.** No + * function in AgentBridge produces one: there is no factory, no builder, and no + * evaluator output that contains one. The boundary that turns a human decision + * into a record of this shape does not exist yet, and building it is an + * explicit later decision, not an implementation detail of some other layer. + * + * It is defined here so that the *shape* of the only thing that may ever + * authorize a merge is written down while the merge barrier is being + * established, rather than improvised later by whoever needs it first. + * + * It is deliberately **not** an `ApprovalRecord`. That type is human decision + * data about an `ActionRequest` at PR 003's gate; reusing it here would make + * every existing approval a candidate merge authority. + * + * The required properties, all of which {@link operatorMergeAuthorizes} + * enforces: + * + * - operator-originated — `operatorId` names a human, and nothing in the + * domain mints one + * - repository-bound, pull-request-bound, and bound to an exact HEAD SHA + * - single-use, and invalid the moment HEAD changes + * - incapable of authorizing another pull request or a future SHA + */ +export interface OperatorMergeAuthorization { + /** Caller-minted identity of this one operator decision. */ + readonly authorizationId: string; + /** The human who decided. Never an agent, and never inferred from a label. */ + readonly operatorId: string; + /** The one repository this authorization is valid in. */ + readonly repositoryId: string; + /** The one pull request this authorization is valid for. */ + readonly pullRequestId: string; + /** The exact HEAD the operator approved. A different HEAD is a different merge. */ + readonly headSha: string; + /** Caller-supplied timestamp. Data; no clock is read here. */ + readonly authorizedAt: string; + /** Structural: one merge, then nothing. */ + readonly singleUse: true; +} + +/** The exact merge an operator authorization is being checked against. */ +export interface MergeTarget { + readonly repositoryId: string; + readonly pullRequestId: string; + /** The repository's HEAD *now*, supplied by a trusted adapter. */ + readonly currentHeadSha: string; +} + +/** + * Does this operator authorization cover exactly this merge, right now? + * + * Pure, total, and deterministic; never throws. Both arguments are read + * defensively, own-only, and exactly once. + * + * Every comparison is exact string equality, so a HEAD that moved by one commit + * invalidates the authorization, and an authorization for pull request 41 can + * never cover pull request 42. There is no path that widens, refreshes, or + * re-binds an authorization to a newer SHA: a new HEAD requires a new operator + * decision. + * + * C1 executes no merge. This predicate exists so that the merge barrier is + * defined by something more precise than a comment. + */ +export function operatorMergeAuthorizes( + authorization: OperatorMergeAuthorization, + target: MergeTarget, +): boolean { + const record: unknown = authorization; + if (typeof record !== 'object' || record === null) { + return false; + } + const targetRecord: unknown = target; + if (typeof targetRecord !== 'object' || targetRecord === null) { + return false; + } + + const authorizationId = readExactIdentifier(readOwnProperty(record, 'authorizationId')); + const operatorId = readExactIdentifier(readOwnProperty(record, 'operatorId')); + const repositoryId = readExactIdentifier(readOwnProperty(record, 'repositoryId')); + const pullRequestId = readExactIdentifier(readOwnProperty(record, 'pullRequestId')); + const headSha = readExactIdentifier(readOwnProperty(record, 'headSha')); + const authorizedAt = readExactIdentifier(readOwnProperty(record, 'authorizedAt')); + const singleUse = readOwnProperty(record, 'singleUse'); + + const targetRepositoryId = readExactIdentifier(readOwnProperty(targetRecord, 'repositoryId')); + const targetPullRequestId = readExactIdentifier(readOwnProperty(targetRecord, 'pullRequestId')); + const targetHeadSha = readExactIdentifier(readOwnProperty(targetRecord, 'currentHeadSha')); + + if ( + authorizationId === null || + operatorId === null || + repositoryId === null || + pullRequestId === null || + headSha === null || + authorizedAt === null || + singleUse !== true + ) { + return false; + } + if (targetRepositoryId === null || targetPullRequestId === null || targetHeadSha === null) { + return false; + } + + return ( + repositoryId === targetRepositoryId && + pullRequestId === targetPullRequestId && + headSha === targetHeadSha + ); +} diff --git a/src/domain/index.ts b/src/domain/index.ts index da11dd3..394d912 100644 --- a/src/domain/index.ts +++ b/src/domain/index.ts @@ -138,6 +138,62 @@ export { type RejectedClaim, } from './agent-invocation-report.js'; +export { + findInvalidRepairJobFields, + isVerificationCommandClass, + JOB_BOUNDS, + readRepairJobAuthorization, + readRepositoryRelativePath, + REPAIR_JOB_FIELD_ORDER, + satisfiesIndependentValidator, + VERIFICATION_COMMAND_CLASS, + VERIFICATION_COMMAND_CLASSES, + type RepairJobAuthorization, + type RepairJobField, + type RepairJobReadResult, + type RepairJobSnapshot, + type ValidatorClaim, + type VerificationCommandClass, +} from './repair-job.js'; + +export { + FORBIDDEN_OPERATION, + FORBIDDEN_OPERATIONS, + isForbiddenJobOperation, + isRepairAuthorizableOperation, + JOB_OPERATION, + PERMIT_OPERAND_ORDER, + readJobOperation, + REPAIR_AUTHORIZABLE_OPERATIONS, + resolveJobOperation, + UNKNOWN_JOB_OPERATION, + type ForbiddenJobOperation, + type JobOperation, + type JobOperationRequest, + type NormalizedJobOperation, + type PermitOperands, + type RepairAuthorizableOperation, + type UnknownJobOperation, +} from './job-operation.js'; + +export { + operatorMergeAuthorizes, + type ExecutionPermit, + type MergeTarget, + type OperatorMergeAuthorization, +} from './execution-permit.js'; + +export { + authorizeJobOperation, + JOB_AUTHORIZATION, + JOB_AUTHORIZATION_OUTCOMES, + JOB_AUTHORIZATION_REASON, + permitAuthorizes, + type JobAuthorizationDecision, + type JobAuthorizationOutcome, + type JobAuthorizationReason, +} from './job-authorization.js'; + export { currentEvidenceOfKind, evaluateEvidenceFreshness, diff --git a/src/domain/job-authorization.ts b/src/domain/job-authorization.ts new file mode 100644 index 0000000..f0f9a60 --- /dev/null +++ b/src/domain/job-authorization.ts @@ -0,0 +1,531 @@ +/** + * The repair job authorization evaluator, and the merge barrier (Cockpit C1). + * + * trusted job envelope + untrusted operation request -> JobAuthorizationDecision + * + * `authorizeJobOperation` is a pure function of its two arguments. It executes + * nothing, spawns nothing, reads no clock, touches no filesystem or network, + * loads no configuration, and persists nothing. Equal arguments always produce + * an equal result, and no input throws. + * + * ## The decision vocabulary + * + * - `ALLOW_ONCE` — this exact normalized operation, under this exact job + * binding, may be executed **once**, under the accompanying + * {@link ExecutionPermit}. It is not a standing permission and does not + * generalise to a similar operation, a later HEAD, or another job. + * - `DENY` — refused. Nothing at this layer converts a `DENY` into an allow: + * there is no override parameter, no approval parameter, and no field on + * either argument that is consulted after a denial is reached. + * - `OPERATOR_REQUIRED` — outside every autonomous envelope. Only a human + * operator, through the separate `OperatorMergeAuthorization` type, can ever + * authorize it, and no evaluator in AgentBridge produces one. This is **not** + * "escalate and retry": this function never returns a permit alongside it. + * + * The vocabulary deliberately does not reuse PR 003's `ALLOW`/`ESCALATE`/`DENY` + * or its `AUTONOMOUS` outcome. Those answer "what is this action?" for a + * read-only V1; this answers "may this bounded job perform this exact operation + * once?", which is a different question with different operands. + * + * ## The merge barrier + * + * Merge is operator-only. This is a permanent AgentBridge Cockpit invariant + * unless an explicit later architecture decision changes it, and C1 enforces it + * structurally rather than by convention: + * + * 1. `merge` is not a member of `RepairAuthorizableOperation`. `ExecutionPermit + * .operation` is typed to that union, so **a merge permit does not + * type-check.** + * 2. `ALLOW_ONCE` is produced at exactly one `return` in this file, reachable + * only after the operation has been narrowed to `RepairAuthorizableOperation` + * by two type guards. + * 3. The merge check is the *first* decision made, before the job envelope is + * even validated, so no envelope, binding, or operand state can precede it. + * 4. `authorizeJobOperation` **has no approval parameter**. There is no + * argument through which an `ApprovalRecord` — approved or otherwise — can + * reach this evaluator, so no human approval can turn merge into job + * authority. + * 5. Nothing here reads an agent id, a provider id, a rationale, or metadata. + * The request type has no such field, and the normalizer reads no such key. + * + * The maximum autonomous state a future workflow may reach is therefore + * "ready for an operator to merge". C1 implements no such state and no state + * machine; it establishes only that ordinary job authority has no permission + * that could become merge. + */ + +import { + containsValue, + isVerificationCommandClass, + readRepairJobAuthorization, + type RepairJobAuthorization, + type RepairJobField, + type RepairJobSnapshot, +} from './repair-job.js'; +import { + FORBIDDEN_OPERATION, + isForbiddenJobOperation, + JOB_OPERATION, + type JobOperation, + type JobOperationRequest, + type NormalizedJobOperation, + projectOperands, + readJobOperation, + type RepairAuthorizableOperation, + UNKNOWN_JOB_OPERATION, +} from './job-operation.js'; +import { + type ExecutionPermit, + issueExecutionPermit, + permitsEqual, +} from './execution-permit.js'; + +const objectFreeze = Object.freeze; + +/** The three outcomes this evaluator can reach. */ +export const JOB_AUTHORIZATION = objectFreeze({ + /** One execution of this exact operation is authorized, under a permit. */ + ALLOW_ONCE: 'ALLOW_ONCE', + /** Refused. No input at this layer converts this into an allow. */ + DENY: 'DENY', + /** Only a human operator may ever authorize this. Job authority never can. */ + OPERATOR_REQUIRED: 'OPERATOR_REQUIRED', +} as const); + +export type JobAuthorizationOutcome = + (typeof JOB_AUTHORIZATION)[keyof typeof JOB_AUTHORIZATION]; + +/** Every member of the {@link JobAuthorizationOutcome} union. */ +export const JOB_AUTHORIZATION_OUTCOMES: readonly JobAuthorizationOutcome[] = objectFreeze([ + JOB_AUTHORIZATION.ALLOW_ONCE, + JOB_AUTHORIZATION.DENY, + JOB_AUTHORIZATION.OPERATOR_REQUIRED, +]); + +/** Stable, machine-readable rationale for an authorization outcome. */ +export const JOB_AUTHORIZATION_REASON = objectFreeze({ + /** The operation and every operand fall inside the job's envelope. */ + WITHIN_JOB_ENVELOPE: 'WITHIN_JOB_ENVELOPE', + /** Merge. Operator-only, permanently, regardless of every other input. */ + MERGE_IS_OPERATOR_ONLY: 'MERGE_IS_OPERATOR_ONLY', + /** A modeled operation ordinary job authority may never perform. */ + OPERATION_FORBIDDEN: 'OPERATION_FORBIDDEN', + /** The operation name is not modeled at all. */ + OPERATION_UNKNOWN: 'OPERATION_UNKNOWN', + /** The request could not be read as an object. */ + OPERATION_UNREADABLE: 'OPERATION_UNREADABLE', + /** The job envelope is missing or malformed. See `invalidJobFields`. */ + JOB_ENVELOPE_INVALID: 'JOB_ENVELOPE_INVALID', + /** The request names a different job. */ + JOB_MISMATCH: 'JOB_MISMATCH', + /** The request names a different repository. */ + REPOSITORY_MISMATCH: 'REPOSITORY_MISMATCH', + /** The request names a different parent pull request. */ + PARENT_PULL_REQUEST_MISMATCH: 'PARENT_PULL_REQUEST_MISMATCH', + /** The request names a different parent HEAD. */ + PARENT_HEAD_MISMATCH: 'PARENT_HEAD_MISMATCH', + /** The finding was verified against a commit that is no longer the job's HEAD. */ + FINDING_SHA_STALE: 'FINDING_SHA_STALE', + /** A required operand is absent. */ + OPERAND_MISSING: 'OPERAND_MISSING', + /** A path was supplied but is not a usable repository-relative path. */ + PATH_MALFORMED: 'PATH_MALFORMED', + /** The path is not in the job's authorized file scope. */ + PATH_NOT_AUTHORIZED: 'PATH_NOT_AUTHORIZED', + /** The worktree operand is not the job's isolated repair worktree. */ + WORKTREE_NOT_AUTHORIZED: 'WORKTREE_NOT_AUTHORIZED', + /** The verification class is unmodeled or not in the job's authorized set. */ + COMMAND_CLASS_NOT_AUTHORIZED: 'COMMAND_CLASS_NOT_AUTHORIZED', + /** The operation names the protected parent ref as a write target. */ + PROTECTED_REF_MUTATION: 'PROTECTED_REF_MUTATION', + /** The ref operand is not the job's isolated repair branch. */ + REF_NOT_REPAIR_BRANCH: 'REF_NOT_REPAIR_BRANCH', + /** The stacked change request does not target the protected parent ref. */ + CHANGE_REQUEST_TARGET_INVALID: 'CHANGE_REQUEST_TARGET_INVALID', + /** A push was requested with force, or without a readable non-force flag. */ + FORCE_PUSH_FORBIDDEN: 'FORCE_PUSH_FORBIDDEN', +} as const); + +export type JobAuthorizationReason = + (typeof JOB_AUTHORIZATION_REASON)[keyof typeof JOB_AUTHORIZATION_REASON]; + +/** + * The evaluator's answer about one operation. + * + * Every field is a primitive, `null`, a frozen list of strings, or a frozen + * permit, so the record is JSON-serializable and survives a round trip + * unchanged. Echoed identifiers are `null` rather than omitted, so + * `JSON.stringify` cannot silently drop an authorization-relevant field. + * + * The request's rationale, metadata, agent identity, and provider identity are + * **not** echoed, because they were not weighed. Reproducing them would suggest + * otherwise. `requestId` links the decision back to the full request. + */ +export interface JobAuthorizationDecision { + /** The job's identity, or `null` when the envelope was unreadable. */ + readonly jobId: string | null; + /** The job's repository, or `null` when the envelope was unreadable. */ + readonly repositoryId: string | null; + /** The policy version that produced this decision. */ + readonly policyVersion: string | null; + /** The request's own identity, echoed for correlation. */ + readonly requestId: string | null; + /** The resolved operation. `unknown` for anything unmodeled. */ + readonly operation: JobOperation; + /** The outcome. */ + readonly decision: JobAuthorizationOutcome; + /** Stable, machine-readable rationale. */ + readonly reason: JobAuthorizationReason; + /** Job fields that failed validation, in declaration order. */ + readonly invalidJobFields: readonly RepairJobField[]; + /** + * The single reliable answer to "may this be executed, once, now?" + * + * True only at the one `ALLOW_ONCE` return site, where a permit is always + * issued. No other input can raise it. + */ + readonly mayExecuteOnce: boolean; + /** The permit, present only alongside `ALLOW_ONCE`. */ + readonly permit: ExecutionPermit | null; +} + +/** Build a refusal. Never carries a permit; `mayExecuteOnce` is always false. */ +function refuse( + snapshot: RepairJobSnapshot | null, + invalidJobFields: readonly RepairJobField[], + operation: NormalizedJobOperation, + decision: JobAuthorizationOutcome, + reason: JobAuthorizationReason, +): JobAuthorizationDecision { + return objectFreeze({ + jobId: snapshot === null ? null : snapshot.jobId, + repositoryId: snapshot === null ? null : snapshot.repositoryId, + policyVersion: snapshot === null ? null : snapshot.policyVersion, + requestId: operation.requestId, + operation: operation.operation, + decision, + reason, + invalidJobFields, + mayExecuteOnce: false, + permit: null, + }); +} + +/** + * Check the operands one authorizable operation defines. + * + * Returns the refusal reason, or `null` when every operand is inside the + * envelope. Split out so the main evaluator's control flow — and in particular + * its single `ALLOW_ONCE` return — stays readable. + * + * Every comparison is exact string equality against a value from the trusted + * job snapshot. No prefix matching, no normalisation, no case folding. + */ +function checkOperands( + job: RepairJobSnapshot, + operation: RepairAuthorizableOperation, + request: NormalizedJobOperation, +): JobAuthorizationReason | null { + switch (operation) { + case JOB_OPERATION.SOURCE_READ: + case JOB_OPERATION.SOURCE_EDIT: { + if (request.worktreeId !== job.repairWorktreeId) { + return JOB_AUTHORIZATION_REASON.WORKTREE_NOT_AUTHORIZED; + } + if (request.pathMalformed) { + return JOB_AUTHORIZATION_REASON.PATH_MALFORMED; + } + if (request.path === null) { + return JOB_AUTHORIZATION_REASON.OPERAND_MISSING; + } + if (!containsValue(job.authorizedPaths, request.path)) { + return JOB_AUTHORIZATION_REASON.PATH_NOT_AUTHORIZED; + } + return null; + } + case JOB_OPERATION.VERIFICATION_RUN: { + if (request.worktreeId !== job.repairWorktreeId) { + return JOB_AUTHORIZATION_REASON.WORKTREE_NOT_AUTHORIZED; + } + if (request.commandClass === null) { + return JOB_AUTHORIZATION_REASON.OPERAND_MISSING; + } + // Two independent conditions: the class must be one C1 models at all, and + // the job must have been configured to permit it. + if ( + !isVerificationCommandClass(request.commandClass) || + !containsValue(job.authorizedCommandClasses, request.commandClass) + ) { + return JOB_AUTHORIZATION_REASON.COMMAND_CLASS_NOT_AUTHORIZED; + } + return null; + } + case JOB_OPERATION.REPAIR_COMMIT: { + if (request.worktreeId !== job.repairWorktreeId) { + return JOB_AUTHORIZATION_REASON.WORKTREE_NOT_AUTHORIZED; + } + if (request.ref === null) { + return JOB_AUTHORIZATION_REASON.OPERAND_MISSING; + } + if (request.ref === job.protectedParentRef) { + return JOB_AUTHORIZATION_REASON.PROTECTED_REF_MUTATION; + } + if (request.ref !== job.repairBranch) { + return JOB_AUTHORIZATION_REASON.REF_NOT_REPAIR_BRANCH; + } + return null; + } + case JOB_OPERATION.REPAIR_PUSH: { + // Checked before the ref, so a forced push to the *authorized* repair + // branch is refused for being forced rather than accidentally allowed. + if (request.force) { + return JOB_AUTHORIZATION_REASON.FORCE_PUSH_FORBIDDEN; + } + if (request.ref === null) { + return JOB_AUTHORIZATION_REASON.OPERAND_MISSING; + } + if (request.ref === job.protectedParentRef) { + return JOB_AUTHORIZATION_REASON.PROTECTED_REF_MUTATION; + } + if (request.ref !== job.repairBranch) { + return JOB_AUTHORIZATION_REASON.REF_NOT_REPAIR_BRANCH; + } + return null; + } + case JOB_OPERATION.REPAIR_CHANGE_REQUEST: { + if (request.sourceRef === null || request.targetRef === null) { + return JOB_AUTHORIZATION_REASON.OPERAND_MISSING; + } + // The stacked validation change request is the one place the protected + // parent ref may be named, and only as a *target*. Opening a change + // request against a ref does not mutate it; the parent stays untouched + // until an operator merges. + if (request.sourceRef !== job.repairBranch) { + return JOB_AUTHORIZATION_REASON.REF_NOT_REPAIR_BRANCH; + } + if (request.targetRef !== job.protectedParentRef) { + return JOB_AUTHORIZATION_REASON.CHANGE_REQUEST_TARGET_INVALID; + } + return null; + } + } +} + +/** + * Authorize one operation under one repair job. + * + * Pure, total, and deterministic; never throws. Both arguments are read exactly + * once into frozen snapshots before any decision is made, so a getter or Proxy + * that returns a different value on each access cannot validate one operand and + * have a different one reach the decision or the permit. + * + * **There is no third parameter, and there never should be.** Not an approval, + * not an actor, not an override, not a policy escape hatch. Authority is a + * function of trusted configuration and exact operands, and adding an argument + * is how that stops being true. + * + * @param job Trusted job configuration from an operator-controlled boundary. + * @param request Untrusted operation request. + */ +export function authorizeJobOperation( + job: RepairJobAuthorization, + request: JobOperationRequest, +): JobAuthorizationDecision { + const operation = readJobOperation(request); + const jobRead = readRepairJobAuthorization(job); + const snapshot = jobRead.snapshot; + const kind: JobOperation = operation.operation; + + if (!operation.readable) { + return refuse( + snapshot, + jobRead.invalidFields, + operation, + JOB_AUTHORIZATION.DENY, + JOB_AUTHORIZATION_REASON.OPERATION_UNREADABLE, + ); + } + + // --------------------------------------------------------------------- + // The merge barrier. First, unconditional, and above every other check. + // + // The answer to "may a repair job merge?" does not depend on the job, the + // binding, the operands, the provider, or any human record, so nothing about + // any of them is consulted before answering. OPERATOR_REQUIRED is returned + // without a permit, here and nowhere else. + // --------------------------------------------------------------------- + if (kind === FORBIDDEN_OPERATION.MERGE) { + return refuse( + snapshot, + jobRead.invalidFields, + operation, + JOB_AUTHORIZATION.OPERATOR_REQUIRED, + JOB_AUTHORIZATION_REASON.MERGE_IS_OPERATOR_ONLY, + ); + } + + if (isForbiddenJobOperation(kind)) { + return refuse( + snapshot, + jobRead.invalidFields, + operation, + JOB_AUTHORIZATION.DENY, + JOB_AUTHORIZATION_REASON.OPERATION_FORBIDDEN, + ); + } + + if (kind === UNKNOWN_JOB_OPERATION) { + return refuse( + snapshot, + jobRead.invalidFields, + operation, + JOB_AUTHORIZATION.DENY, + JOB_AUTHORIZATION_REASON.OPERATION_UNKNOWN, + ); + } + + if (snapshot === null) { + return refuse( + snapshot, + jobRead.invalidFields, + operation, + JOB_AUTHORIZATION.DENY, + JOB_AUTHORIZATION_REASON.JOB_ENVELOPE_INVALID, + ); + } + + // Binding. Each comparison is exact string equality against the trusted + // snapshot, and a `null` operand can never equal a validated snapshot value, + // so an absent claim fails exactly like a wrong one. + if (operation.jobId !== snapshot.jobId) { + return refuse( + snapshot, + jobRead.invalidFields, + operation, + JOB_AUTHORIZATION.DENY, + JOB_AUTHORIZATION_REASON.JOB_MISMATCH, + ); + } + if (operation.repositoryId !== snapshot.repositoryId) { + return refuse( + snapshot, + jobRead.invalidFields, + operation, + JOB_AUTHORIZATION.DENY, + JOB_AUTHORIZATION_REASON.REPOSITORY_MISMATCH, + ); + } + if (operation.parentPullRequestId !== snapshot.parentPullRequestId) { + return refuse( + snapshot, + jobRead.invalidFields, + operation, + JOB_AUTHORIZATION.DENY, + JOB_AUTHORIZATION_REASON.PARENT_PULL_REQUEST_MISMATCH, + ); + } + if (operation.parentHeadSha !== snapshot.parentHeadSha) { + return refuse( + snapshot, + jobRead.invalidFields, + operation, + JOB_AUTHORIZATION.DENY, + JOB_AUTHORIZATION_REASON.PARENT_HEAD_MISMATCH, + ); + } + + // The finding must have been verified against the commit the job is bound to. + // A repair derived from a finding about some other commit is a repair of + // something that may no longer be there. PR 004 owns CURRENT versus STALE for + // evidence; this is the narrower structural check that the job's own two SHAs + // agree, which C1 can decide without importing that kernel. + if (snapshot.findingHeadSha !== snapshot.parentHeadSha) { + return refuse( + snapshot, + jobRead.invalidFields, + operation, + JOB_AUTHORIZATION.DENY, + JOB_AUTHORIZATION_REASON.FINDING_SHA_STALE, + ); + } + + // Permit identity is a total function of the exact execution, so an execution + // with no identity of its own cannot be authorized. + const requestId = operation.requestId; + if (requestId === null) { + return refuse( + snapshot, + jobRead.invalidFields, + operation, + JOB_AUTHORIZATION.DENY, + JOB_AUTHORIZATION_REASON.OPERAND_MISSING, + ); + } + + const operandFailure = checkOperands(snapshot, kind, operation); + if (operandFailure !== null) { + return refuse( + snapshot, + jobRead.invalidFields, + operation, + JOB_AUTHORIZATION.DENY, + operandFailure, + ); + } + + // --------------------------------------------------------------------- + // The single ALLOW_ONCE return in AgentBridge. + // + // `kind` is narrowed to RepairAuthorizableOperation here by the two guards + // above, which is why no forbidden operation — merge included — can reach + // this line even if every check below it were removed. + // --------------------------------------------------------------------- + return objectFreeze({ + jobId: snapshot.jobId, + repositoryId: snapshot.repositoryId, + policyVersion: snapshot.policyVersion, + requestId, + operation: kind, + decision: JOB_AUTHORIZATION.ALLOW_ONCE, + reason: JOB_AUTHORIZATION_REASON.WITHIN_JOB_ENVELOPE, + invalidJobFields: jobRead.invalidFields, + mayExecuteOnce: true, + permit: issueExecutionPermit(snapshot, kind, requestId, projectOperands(kind, operation)), + }); +} + +/** + * Does this permit authorize this exact operation, under this job, right now? + * + * The permit is **not trusted on its face**. This re-derives the entire + * decision from the trusted job and the untrusted request and then compares, + * so a permit only ever authorizes what the evaluator would authorize at the + * moment of use. Three consequences worth being explicit about: + * + * - A permit issued for job A cannot be presented under job B: the re-derived + * permit binds to B's identity and does not match. + * - A permit issued for one operation or operand cannot be presented for + * another: operands are part of permit identity. + * - A permit issued against one parent HEAD stops verifying the moment the job + * is re-bound to a new HEAD, and re-claiming the new HEAD in the request + * produces a different permit identity rather than reviving the old one. + * + * It does **not** implement consumption. C1 has no store, and single use is a + * property of the permit's meaning and identity, not of a counter this pure + * layer could keep. A consumer must record consumed `permitId`s and refuse a + * repeat; this function tells it whether a permit is *valid*, never whether it + * is *unused*. + * + * Pure, total, and deterministic; never throws. + */ +export function permitAuthorizes( + permit: ExecutionPermit, + job: RepairJobAuthorization, + request: JobOperationRequest, +): boolean { + const decision = authorizeJobOperation(job, request); + if (!decision.mayExecuteOnce || decision.permit === null) { + return false; + } + return permitsEqual(permit, decision.permit); +} diff --git a/src/domain/job-operation.ts b/src/domain/job-operation.ts new file mode 100644 index 0000000..9f5ed17 --- /dev/null +++ b/src/domain/job-operation.ts @@ -0,0 +1,444 @@ +/** + * Structured repair-job operations (Cockpit C1). + * + * A generic action name is **not** sufficient for Cockpit write authority. A + * request to "write the repository" authorizes nothing, because there is no + * such operation: every write-shaped operation names the exact operand that + * makes it decidable — which path, which ref, which worktree, which + * verification class. + * + * This module models and normalizes operations. It decides nothing; + * `job-authorization.ts` does. Nothing here executes: there is no shell + * parsing, no command string, no argument vector, no subprocess, no filesystem, + * and no git. + * + * ## Two vocabularies, deliberately disjoint + * + * {@link JOB_OPERATION} names what a repair job may *ever* be authorized to do. + * {@link FORBIDDEN_OPERATION} names what ordinary job authority may *never* do, + * modeled explicitly so that refusing it is a deterministic decision with a + * stable reason rather than an accident of falling through to `unknown`. + * + * The two are disjoint by construction and the authorizable union is a + * TypeScript type that does not contain `merge`. An `ExecutionPermit` is typed + * to carry only a {@link RepairAuthorizableOperation}, so a merge permit does + * not type-check — the merge barrier is enforced by the type system before any + * runtime check runs. A test pins the disjointness at runtime as well. + */ + +import { + append, + containsValue, + readExactIdentifier, + readOwnProperty, + readRepositoryRelativePath, +} from './repair-job.js'; + +const objectFreeze = Object.freeze; + +/** + * Operations a repair job may be authorized to perform. + * + * Each names the operand that makes it decidable. There is deliberately no + * `repository.write`, no `git.run`, and no `shell.exec`: an operation whose + * authority cannot be checked against an exact operand has no place here. + * + * `repair.change_request` covers creating *or updating* the stacked validation + * pull request. `change_request` is the provider-neutral name PR 006 already + * uses for what a given forge calls a pull or merge request. + */ +export const JOB_OPERATION = objectFreeze({ + /** Read one authorized source file inside the repair worktree. */ + SOURCE_READ: 'source.read', + /** Edit one authorized source file inside the repair worktree. */ + SOURCE_EDIT: 'source.edit', + /** Run one authorized verification command class in the repair worktree. */ + VERIFICATION_RUN: 'verification.run', + /** Commit to the authorized repair branch inside the repair worktree. */ + REPAIR_COMMIT: 'repair.commit', + /** Push the authorized repair branch. Never forced, never another ref. */ + REPAIR_PUSH: 'repair.push', + /** Create or update the stacked validation change request. */ + REPAIR_CHANGE_REQUEST: 'repair.change_request', +} as const); + +export type RepairAuthorizableOperation = + (typeof JOB_OPERATION)[keyof typeof JOB_OPERATION]; + +/** Every member of the {@link RepairAuthorizableOperation} union. */ +export const REPAIR_AUTHORIZABLE_OPERATIONS: readonly RepairAuthorizableOperation[] = + objectFreeze([ + JOB_OPERATION.SOURCE_READ, + JOB_OPERATION.SOURCE_EDIT, + JOB_OPERATION.VERIFICATION_RUN, + JOB_OPERATION.REPAIR_COMMIT, + JOB_OPERATION.REPAIR_PUSH, + JOB_OPERATION.REPAIR_CHANGE_REQUEST, + ]); + +/** + * Operations ordinary repair-job authority may never perform. + * + * These are modeled rather than left unmodeled on purpose. An unmodeled name + * fails closed as `unknown`, which is correct but uninformative; naming these + * makes the refusal explicit, gives it a stable reason code, and makes the + * mandatory hard denials mechanically testable rather than incidental. + * + * **`merge` is the load-bearing member.** It is the only operation anywhere in + * C1 that resolves to `OPERATOR_REQUIRED` rather than `DENY`, because merge is + * not forbidden — it is *operator-only*. Every other member here is forbidden + * outright, for a repair job and for an operator alike, at this layer. + * + * `auto_merge.enable` is `DENY`, not `OPERATOR_REQUIRED`, and the distinction + * is deliberate: enabling auto-merge delegates the merge decision away from the + * human who would otherwise make it at the moment HEAD is final. An operator + * asking for auto-merge is asking to not be the operator. + */ +export const FORBIDDEN_OPERATION = objectFreeze({ + /** Merge. Operator-only, permanently. */ + MERGE: 'merge', + /** Enable auto-merge. Delegates the operator's decision; never permitted. */ + AUTO_MERGE_ENABLE: 'auto_merge.enable', + /** Direct mutation of the protected parent ref. */ + PARENT_REF_WRITE: 'parent_ref.write', + /** Force push, to any ref. */ + PUSH_FORCE: 'push.force', + /** Reset, rebase, amend, or any rewrite of protected integration history. */ + HISTORY_REWRITE: 'history.rewrite', + /** Deletion of any ref. */ + BRANCH_DELETE: 'branch.delete', + /** Modification of the policy that governs this envelope. */ + POLICY_MODIFY: 'policy.modify', + /** Access to secrets, credentials, or tokens. */ + SECRET_ACCESS: 'secret.access', + /** Deployment of anything, anywhere. */ + DEPLOYMENT_RUN: 'deployment.run', + /** Mutation of a staging environment. */ + STAGING_CHANGE: 'staging.change', + /** Mutation of a production environment. */ + PRODUCTION_CHANGE: 'production.change', + /** Any database write. */ + DATABASE_WRITE: 'database.write', + /** Any schema migration. */ + DATABASE_MIGRATE: 'database.migrate', +} as const); + +export type ForbiddenJobOperation = + (typeof FORBIDDEN_OPERATION)[keyof typeof FORBIDDEN_OPERATION]; + +/** Every member of the {@link ForbiddenJobOperation} union. */ +export const FORBIDDEN_OPERATIONS: readonly ForbiddenJobOperation[] = objectFreeze([ + FORBIDDEN_OPERATION.MERGE, + FORBIDDEN_OPERATION.AUTO_MERGE_ENABLE, + FORBIDDEN_OPERATION.PARENT_REF_WRITE, + FORBIDDEN_OPERATION.PUSH_FORCE, + FORBIDDEN_OPERATION.HISTORY_REWRITE, + FORBIDDEN_OPERATION.BRANCH_DELETE, + FORBIDDEN_OPERATION.POLICY_MODIFY, + FORBIDDEN_OPERATION.SECRET_ACCESS, + FORBIDDEN_OPERATION.DEPLOYMENT_RUN, + FORBIDDEN_OPERATION.STAGING_CHANGE, + FORBIDDEN_OPERATION.PRODUCTION_CHANGE, + FORBIDDEN_OPERATION.DATABASE_WRITE, + FORBIDDEN_OPERATION.DATABASE_MIGRATE, +]); + +/** Sentinel for any operation this module does not model. */ +export const UNKNOWN_JOB_OPERATION = 'unknown'; + +export type UnknownJobOperation = typeof UNKNOWN_JOB_OPERATION; + +/** Every operation name that can be resolved, including the unknown sentinel. */ +export type JobOperation = + | RepairAuthorizableOperation + | ForbiddenJobOperation + | UnknownJobOperation; + +/** + * Membership is backed by a `Map`, not a plain object. + * + * A plain-object lookup inherits `Object.prototype`, so `'toString'`, + * `'constructor'`, and `'__proto__'` would resolve to a truthy entry. A `Map` + * has no prototype chain for keys. Same reasoning as PR 002's taxonomy. + */ +const OPERATION_LOOKUP: ReadonlyMap = + new Map([ + ...REPAIR_AUTHORIZABLE_OPERATIONS.map( + (operation) => [operation, operation] as const, + ), + ...FORBIDDEN_OPERATIONS.map((operation) => [operation, operation] as const), + ]); + +/** + * Resolve an untrusted operation name to a modeled member. + * + * Matching is exact and case-sensitive. No trimming, case folding, aliasing, or + * fuzzy matching, because lenient matching on a security boundary is a + * privilege-escalation vector: `'SOURCE.EDIT '` must not become `source.edit`. + * + * Anything unrecognised resolves to {@link UNKNOWN_JOB_OPERATION}, including + * the literal string `'unknown'` — the sentinel names the absence of a model, + * so requesting it by name is still an unmodeled request. Never throws. + */ +export function resolveJobOperation(value: unknown): JobOperation { + if (typeof value !== 'string') { + return UNKNOWN_JOB_OPERATION; + } + return OPERATION_LOOKUP.get(value) ?? UNKNOWN_JOB_OPERATION; +} + +/** Type guard: is this a modeled operation a repair job may be authorized for? */ +export function isRepairAuthorizableOperation( + value: JobOperation, +): value is RepairAuthorizableOperation { + return containsValue(REPAIR_AUTHORIZABLE_OPERATIONS, value); +} + +/** Type guard: is this a modeled operation ordinary job authority may never do? */ +export function isForbiddenJobOperation(value: JobOperation): value is ForbiddenJobOperation { + return containsValue(FORBIDDEN_OPERATIONS, value); +} + +/** + * An untrusted request to perform one operation under one repair job. + * + * Declared shape is advisory: at runtime every property is read defensively and + * any type may arrive. Properties not listed here are ignored. + * + * The binding fields (`jobId`, `repositoryId`, `parentPullRequestId`, + * `parentHeadSha`) are what the requester *claims* to be operating against. + * They are never a source of authority: the evaluator compares each to the + * trusted job envelope and refuses on any mismatch. Their only purpose is to + * make a request that names the wrong repository, pull request, or commit + * refusable instead of silently re-targeted at whatever the job says. + * + * There is deliberately **no** `agentId`, `providerId`, `rationale`, + * `metadata`, `approval`, `role`, `priority`, `urgency`, `confidence`, or + * `override` field. Extra properties carrying such values may be present at + * runtime and are read by nothing; a test pins that adding them changes no + * decision byte. + */ +export interface JobOperationRequest { + /** Caller-minted identity for this one execution attempt. */ + readonly requestId?: string; + /** Claimed job. Must match the trusted envelope exactly. */ + readonly jobId?: string; + /** Claimed repository. Must match the trusted envelope exactly. */ + readonly repositoryId?: string; + /** Claimed parent pull request. Must match the trusted envelope exactly. */ + readonly parentPullRequestId?: string; + /** Claimed parent HEAD. Must match the trusted envelope exactly. */ + readonly parentHeadSha?: string; + /** Operation name. Resolved exactly; anything unmodeled becomes `unknown`. */ + readonly operation?: string; + /** Worktree operand, for filesystem-shaped operations. */ + readonly worktreeId?: string; + /** Path operand, for `source.read` and `source.edit`. */ + readonly path?: string; + /** Verification class operand, for `verification.run`. */ + readonly commandClass?: string; + /** Ref operand, for `repair.commit` and `repair.push`. */ + readonly ref?: string; + /** Change-request source ref operand. */ + readonly sourceRef?: string; + /** Change-request target ref operand. */ + readonly targetRef?: string; + /** Force flag for a push. Anything that is not exactly absent or `false` is force. */ + readonly force?: boolean; +} + +/** + * A frozen, single-read snapshot of one operation request. + * + * Every security-relevant property is read exactly once, own-only, guarded, and + * narrowed, and everything downstream reads only this snapshot. A getter or + * Proxy that returns a different value on each access therefore cannot validate + * one operand and have another reach the decision or the permit. + */ +export interface NormalizedJobOperation { + /** False when the request itself could not be read as an object. */ + readonly readable: boolean; + readonly requestId: string | null; + readonly jobId: string | null; + readonly repositoryId: string | null; + readonly parentPullRequestId: string | null; + readonly parentHeadSha: string | null; + readonly operation: JobOperation; + readonly worktreeId: string | null; + readonly path: string | null; + /** True when a `path` was supplied but did not survive path validation. */ + readonly pathMalformed: boolean; + readonly commandClass: string | null; + readonly ref: string | null; + readonly sourceRef: string | null; + readonly targetRef: string | null; + /** Fails closed: only an absent or literally `false` value is not force. */ + readonly force: boolean; +} + +const UNREADABLE_OPERATION: NormalizedJobOperation = objectFreeze({ + readable: false, + requestId: null, + jobId: null, + repositoryId: null, + parentPullRequestId: null, + parentHeadSha: null, + operation: UNKNOWN_JOB_OPERATION, + worktreeId: null, + path: null, + pathMalformed: false, + commandClass: null, + ref: null, + sourceRef: null, + targetRef: null, + force: true, +}); + +/** + * Read a force flag, failing closed. + * + * Absent or literally `false` is not a force. **Everything else is**, including + * `0`, `''`, `null`, `'false'`, and an object — a value that cannot be read as + * "definitely not forced" is treated as forced, and forced pushes are denied + * unconditionally. + */ +function readForceFlag(value: unknown): boolean { + return !(value === undefined || value === false); +} + +/** + * Normalize an untrusted operation request into a frozen snapshot. + * + * Pure, total, and deterministic; never throws. A non-object request, a revoked + * Proxy, a throwing getter, or a payload of the wrong types all yield a + * snapshot that authorizes nothing rather than an exception. + */ +export function readJobOperation(request: JobOperationRequest): NormalizedJobOperation { + const record: unknown = request; + if (typeof record !== 'object' || record === null) { + return UNREADABLE_OPERATION; + } + + const rawPath = readOwnProperty(record, 'path'); + const path = readRepositoryRelativePath(rawPath); + + return objectFreeze({ + readable: true, + requestId: readExactIdentifier(readOwnProperty(record, 'requestId')), + jobId: readExactIdentifier(readOwnProperty(record, 'jobId')), + repositoryId: readExactIdentifier(readOwnProperty(record, 'repositoryId')), + parentPullRequestId: readExactIdentifier(readOwnProperty(record, 'parentPullRequestId')), + parentHeadSha: readExactIdentifier(readOwnProperty(record, 'parentHeadSha')), + operation: resolveJobOperation(readOwnProperty(record, 'operation')), + worktreeId: readExactIdentifier(readOwnProperty(record, 'worktreeId')), + path, + pathMalformed: path === null && rawPath !== undefined, + commandClass: readExactIdentifier(readOwnProperty(record, 'commandClass')), + ref: readExactIdentifier(readOwnProperty(record, 'ref')), + sourceRef: readExactIdentifier(readOwnProperty(record, 'sourceRef')), + targetRef: readExactIdentifier(readOwnProperty(record, 'targetRef')), + force: readForceFlag(readOwnProperty(record, 'force')), + }); +} + +/** + * The operands an issued permit carries. + * + * Every field an operation does not define is `null`, so an unused operand + * cannot ride along into execution. A `source.read` request that also carries a + * `ref` naming the protected parent produces a permit whose `ref` is `null`: + * the executor is bound to the permit, and the permit contains only what its + * operation means. + * + * `force` is present and always `false` in an issued permit. It is kept rather + * than dropped so that "this permit does not authorize a force push" is a + * value an auditor can read, not an absence they must infer. + */ +export interface PermitOperands { + readonly worktreeId: string | null; + readonly path: string | null; + readonly commandClass: string | null; + readonly ref: string | null; + readonly sourceRef: string | null; + readonly targetRef: string | null; + readonly force: false; +} + +/** Every operand field, in a fixed order, for deterministic comparison. */ +export const PERMIT_OPERAND_ORDER = objectFreeze([ + 'worktreeId', + 'path', + 'commandClass', + 'ref', + 'sourceRef', + 'targetRef', +] as const); + +const NO_OPERANDS = objectFreeze({ + worktreeId: null, + path: null, + commandClass: null, + ref: null, + sourceRef: null, + targetRef: null, + force: false, +} as const); + +/** + * Project the operands one operation actually defines. + * + * Called once at permit issue and again on every re-verification, so the two + * cannot disagree about which operands are part of a permit's identity. + */ +export function projectOperands( + operation: RepairAuthorizableOperation, + normalized: NormalizedJobOperation, +): PermitOperands { + switch (operation) { + case JOB_OPERATION.SOURCE_READ: + case JOB_OPERATION.SOURCE_EDIT: + return objectFreeze({ + ...NO_OPERANDS, + worktreeId: normalized.worktreeId, + path: normalized.path, + }); + case JOB_OPERATION.VERIFICATION_RUN: + return objectFreeze({ + ...NO_OPERANDS, + worktreeId: normalized.worktreeId, + commandClass: normalized.commandClass, + }); + case JOB_OPERATION.REPAIR_COMMIT: + return objectFreeze({ + ...NO_OPERANDS, + worktreeId: normalized.worktreeId, + ref: normalized.ref, + }); + case JOB_OPERATION.REPAIR_PUSH: + return objectFreeze({ ...NO_OPERANDS, ref: normalized.ref }); + case JOB_OPERATION.REPAIR_CHANGE_REQUEST: + return objectFreeze({ + ...NO_OPERANDS, + sourceRef: normalized.sourceRef, + targetRef: normalized.targetRef, + }); + } +} + +/** + * The operand values of a permit, in {@link PERMIT_OPERAND_ORDER} order. + * + * A `null` operand becomes the empty string. No reader on this boundary ever + * returns an empty string — blank values are rejected — so the empty string + * unambiguously means "not part of this operation". + */ +export function operandValues(operands: PermitOperands): readonly string[] { + const values: string[] = []; + append(values, operands.worktreeId ?? ''); + append(values, operands.path ?? ''); + append(values, operands.commandClass ?? ''); + append(values, operands.ref ?? ''); + append(values, operands.sourceRef ?? ''); + append(values, operands.targetRef ?? ''); + return objectFreeze(values); +} diff --git a/src/domain/repair-job.ts b/src/domain/repair-job.ts new file mode 100644 index 0000000..af9715c --- /dev/null +++ b/src/domain/repair-job.ts @@ -0,0 +1,725 @@ +/** + * The repair job authority envelope (Cockpit C1). + * + * A repair job is a **bounded capability envelope**: the complete statement of + * what a future Cockpit execution layer may be authorized to do on behalf of one + * bounded autonomous repair, and nothing else. + * + * trusted job configuration + exact operation operands -> authority + * + * Nothing here executes. There is no filesystem, no git, no subprocess, no + * network, no clock, no persistence, and no identifier generation. This module + * models and validates; `job-authorization.ts` decides. + * + * ## The V1 read-only boundary is preserved + * + * The frozen V1 architecture keeps managed repositories read-only from + * AgentBridge, and C1 does not change that. It states the *shape* a narrowly + * scoped write authority would have to take before any such capability is + * built. Outside a valid repair-job authorization envelope the existing + * read-only boundary is untouched, and inside one nothing is granted that the + * envelope does not name exactly. + * + * ## Authority comes from configuration, never from the requester + * + * The job is **trusted configuration**, supplied by an operator-controlled + * boundary that does not exist yet. The operation request is **untrusted**. + * Agent identity, provider name, rationale, prose, metadata, claimed success, + * and privileged-sounding labels cannot appear in this envelope at all: there is + * no field typed to accept them, so there is nothing for a widening value to + * flow into. + * + * `repairAgentId` and `findingSource` are recorded for audit and are inert as + * authority. Naming an agent `root`, `system`, `admin`, or + * `agentbridge-internal` changes no outcome anywhere in C1. + * + * ## Relationship to the existing layers + * + * PR 002's taxonomy, PR 003's policy gate, PR 004's freshness kernel, PR 005's + * review ingestion, and PR 006's invocation boundary are unchanged and + * un-imported. C1 sits beside them: it answers "what may *this job* do?", not + * "what is this action?" (PR 002/003), "is this evidence current?" (PR 004), or + * "what did a reviewer or agent say?" (PR 005/006). + * + * This module is self-contained on purpose and imports nothing, following the + * boundary-independence convention recorded in PR 006. The three other C1 + * modules import their hostile-input readers from here because they are one + * boundary, not four. + */ + +/** + * Intrinsics captured at module load, before any untrusted property access is + * possible. A hostile getter or Proxy trap runs mid-validation and could + * otherwise repoint the prototype methods this module would rely on afterwards. + * Same pattern as PR 004, PR 005, and PR 006. + */ +const objectFreeze = Object.freeze; +const objectDefineProperty = Object.defineProperty; +const objectHasOwn = Object.hasOwn; +const arrayIsArray = Array.isArray; +const numberIsInteger = Number.isInteger; +const reflectApply = Reflect.apply; +// Captured unbound on purpose and invoked through `Reflect.apply`, so neither a +// poisoned prototype method nor a poisoned `Function.prototype.call` is on the +// path. `this` is supplied explicitly at every call site. +/* eslint-disable @typescript-eslint/unbound-method */ +const stringTrim = String.prototype.trim; +const stringCharCodeAt = String.prototype.charCodeAt; +/* eslint-enable @typescript-eslint/unbound-method */ + +/** Membership test that touches no prototype method. */ +export function containsValue(list: readonly string[], value: unknown): boolean { + for (let index = 0; index < list.length; index += 1) { + if (list[index] === value) { + return true; + } + } + return false; +} + +/** Append by defining an own element, bypassing inherited index setters. */ +export function append(list: T[], value: T): void { + objectDefineProperty(list, list.length, { + value, + writable: true, + enumerable: true, + configurable: true, + }); +} + +/** + * Read one **own** property of an untrusted object. + * + * Own-only on purpose: an inherited property — including one planted on + * `Object.prototype` via a `__proto__` payload — must never supply a value the + * caller did not actually send. Reads are guarded because an own getter or a + * Proxy trap may throw. + */ +export function readOwnProperty(target: object, key: string): unknown { + try { + if (!objectHasOwn(target, key)) { + return undefined; + } + return (target as Record)[key]; + } catch { + return undefined; + } +} + +/** V1 bounds. Every unbounded dimension is capped before iteration. */ +export const JOB_BOUNDS = objectFreeze({ + /** + * Characters permitted in any identifier-shaped field. Oversize **rejects**. + * + * Matches PR 005's and PR 006's identifier bound by convention, because a + * `jobId` may be correlated with an `invocationId` across boundaries. A test + * pins the equality; the modules share no code. + */ + MAX_IDENTIFIER_LENGTH: 256, + /** Characters permitted in a repository-relative path. Oversize rejects. */ + MAX_PATH_LENGTH: 1_024, + /** Entries permitted in `authorizedPaths`. Oversize rejects the whole job. */ + MAX_AUTHORIZED_PATHS: 512, + /** Segments permitted in one path. Oversize rejects the path. */ + MAX_PATH_SEGMENTS: 64, + /** Entries permitted in `authorizedCommandClasses`. */ + MAX_AUTHORIZED_COMMAND_CLASSES: 16, +} as const); + +/** + * Verification command **classes**. + * + * C1 never authorizes a shell command string. It authorizes a class, and a + * later execution layer resolves a class to a concrete command through + * repository policy. There is no field on this boundary that can carry a + * command line, an argument vector, an environment, or a shell. + * + * The five members mirror the verification actions PR 002 already classifies + * read-only (`test.run`, `lint.run`, `typecheck.run`, `build.run`, + * `audit.run`). C1 does not import that taxonomy — a class here is an + * authorization label, not an action kind — but the vocabularies are kept + * aligned so the two layers cannot disagree about what verification means. + */ +export const VERIFICATION_COMMAND_CLASS = objectFreeze({ + TEST: 'test', + LINT: 'lint', + TYPECHECK: 'typecheck', + BUILD: 'build', + AUDIT: 'audit', +} as const); + +export type VerificationCommandClass = + (typeof VERIFICATION_COMMAND_CLASS)[keyof typeof VERIFICATION_COMMAND_CLASS]; + +/** Every member of the {@link VerificationCommandClass} union. */ +export const VERIFICATION_COMMAND_CLASSES: readonly VerificationCommandClass[] = objectFreeze([ + VERIFICATION_COMMAND_CLASS.TEST, + VERIFICATION_COMMAND_CLASS.LINT, + VERIFICATION_COMMAND_CLASS.TYPECHECK, + VERIFICATION_COMMAND_CLASS.BUILD, + VERIFICATION_COMMAND_CLASS.AUDIT, +]); + +/** Type guard: is this untrusted value a modeled verification command class? */ +export function isVerificationCommandClass(value: unknown): value is VerificationCommandClass { + return typeof value === 'string' && containsValue(VERIFICATION_COMMAND_CLASSES, value); +} + +/** + * Read an exact identifier, rejecting rather than aliasing an oversized value. + * + * **Identifiers reject; nothing here truncates.** C1 has no prose field at all, + * so it has no field that is ever cut. A truncated identifier is worse than no + * identifier: git resolves commit prefixes, so a cut SHA can falsely match a + * real object, and a cut branch name can name a different ref entirely. + * + * The value is returned exactly as supplied, never trimmed. Normalising before + * storing would let `" main"` and `"main"` become the same ref on a boundary + * where exactness is the whole point. + */ +export function readExactIdentifier(value: unknown): string | null { + if ( + typeof value !== 'string' || + value.length === 0 || + value.length > JOB_BOUNDS.MAX_IDENTIFIER_LENGTH + ) { + return null; + } + const trimmed: unknown = reflectApply(stringTrim, value, []); + return typeof trimmed === 'string' && trimmed.length > 0 ? value : null; +} + +/** Character code at `index`, or `-1` if the read is unusable. */ +function charCodeAt(value: string, index: number): number { + const code: unknown = reflectApply(stringCharCodeAt, value, [index]); + return typeof code === 'number' && numberIsInteger(code) ? code : -1; +} + +const CODE_SLASH = 0x2f; +const CODE_BACKSLASH = 0x5c; +const CODE_COLON = 0x3a; +const CODE_DOT = 0x2e; +const CODE_TILDE = 0x7e; +const CODE_DELETE = 0x7f; +const CODE_SPACE = 0x20; + +/** Is `value[start, end)` the segment `.git`, in any ASCII case? */ +function isDotGitSegment(value: string, start: number, end: number): boolean { + if (end - start !== 4) { + return false; + } + if (charCodeAt(value, start) !== CODE_DOT) { + return false; + } + // 0x20 folds ASCII upper case to lower case; only these three positions matter. + const g = charCodeAt(value, start + 1) | 0x20; + const i = charCodeAt(value, start + 2) | 0x20; + const t = charCodeAt(value, start + 3) | 0x20; + return g === 0x67 && i === 0x69 && t === 0x74; +} + +/** + * Read a repository-relative path, failing closed on anything questionable. + * + * **What this proves, exactly:** the value is a string that matches a + * conservative repository-relative shape. Authorization is then exact string + * equality against an operator-configured path. + * + * **What this does not prove**, and must not be claimed to: that two equal + * strings name the same file. A pure model cannot know about case-insensitive + * or case-preserving filesystems, Unicode normalisation forms applied by the + * filesystem, symbolic links, hard links, bind mounts, or junctions. A future + * execution layer must re-verify containment against the real filesystem it is + * about to touch; this reader narrows the input, it does not sandbox it. + * + * Rejected, without exception: + * + * - non-strings, empty strings, and values over {@link JOB_BOUNDS.MAX_PATH_LENGTH} + * - any `.` or `..` segment, so traversal never has to be resolved + * - a leading `/`, an empty segment (`//`), or a trailing `/` + * - a leading `~` + * - `\` anywhere, because this model does not know the platform's separator + * semantics and will not guess + * - `:` anywhere, which removes Windows drive-absolute forms (`C:/x`) and NTFS + * alternate data streams (`f.txt:s`) in one rule + * - control characters, including NUL, which truncate paths in some syscalls + * - a `.git` segment at any depth, in any ASCII case, so the repository's own + * metadata, hooks, and config — and a submodule's — are unreachable even if + * an operator misconfigures the scope + * - a segment with a leading space, or a trailing space or dot, because + * Windows strips those and two strings that compare unequal here would name + * one file there + * + * No normalisation of any kind is performed. The value is returned exactly as + * supplied or not at all. + */ +export function readRepositoryRelativePath(value: unknown): string | null { + if (typeof value !== 'string') { + return null; + } + const length = value.length; + if (length === 0 || length > JOB_BOUNDS.MAX_PATH_LENGTH) { + return null; + } + if (charCodeAt(value, 0) === CODE_TILDE) { + return null; + } + + let segments = 0; + let segmentStart = 0; + for (let index = 0; index <= length; index += 1) { + const atEnd = index === length; + const code = atEnd ? CODE_SLASH : charCodeAt(value, index); + + if (!atEnd) { + // An unreadable code reads as -1, which this same bound rejects. + if (code < CODE_SPACE || code === CODE_DELETE) { + return null; + } + if (code === CODE_BACKSLASH || code === CODE_COLON) { + return null; + } + } + + if (code !== CODE_SLASH) { + continue; + } + + const segmentLength = index - segmentStart; + if (segmentLength === 0) { + return null; + } + if (segmentLength === 1 && charCodeAt(value, segmentStart) === CODE_DOT) { + return null; + } + if ( + segmentLength === 2 && + charCodeAt(value, segmentStart) === CODE_DOT && + charCodeAt(value, segmentStart + 1) === CODE_DOT + ) { + return null; + } + // `.git` at any depth: the repository's own metadata, and a submodule's. + if (isDotGitSegment(value, segmentStart, index)) { + return null; + } + // Windows strips trailing spaces and dots from a path component, so + // `a.txt ` and `a.txt` can name the same file there but compare unequal + // here. Rejecting both edges removes the ambiguity instead of guessing + // which platform this will eventually run on. + const first = charCodeAt(value, segmentStart); + const last = charCodeAt(value, index - 1); + if (first === CODE_SPACE || last === CODE_SPACE || last === CODE_DOT) { + return null; + } + segments += 1; + if (segments > JOB_BOUNDS.MAX_PATH_SEGMENTS) { + return null; + } + segmentStart = index + 1; + } + + return value; +} + +/** + * Read a bounded list of untrusted values, all-or-nothing. + * + * One unreadable entry rejects the whole list, and an oversized list is + * rejected rather than truncated. Silently shortening an authorization list + * would produce a job that looks configured but is not the one an operator + * wrote, and silently keeping a prefix of a list an operator got wrong is not + * an improvement on refusing it. + * + * A sparse array yields `undefined` at the holes, which no reader accepts, so + * sparseness rejects rather than collapsing. + */ +function readList( + value: unknown, + maxLength: number, + read: (element: unknown) => T | null, +): readonly T[] | null { + let elements: readonly unknown[] | null; + try { + elements = arrayIsArray(value) ? (value as readonly unknown[]) : null; + } catch { + // `Array.isArray` itself throws on a revoked Proxy. + return null; + } + if (elements === null) { + return null; + } + + let rawLength: unknown; + try { + rawLength = elements.length; + } catch { + return null; + } + if ( + typeof rawLength !== 'number' || + !numberIsInteger(rawLength) || + rawLength < 0 || + rawLength > maxLength + ) { + return null; + } + + const parsed: T[] = []; + for (let index = 0; index < rawLength; index += 1) { + let element: unknown; + try { + element = elements[index]; + } catch { + return null; + } + const value_ = read(element); + if (value_ === null) { + return null; + } + append(parsed, value_); + } + return objectFreeze(parsed); +} + +/** + * Trusted job configuration: the complete authority envelope of one repair job. + * + * Deliberately absent, and never to be added: credentials, tokens, secrets, + * prompt or instruction payloads, command lines, shell strings, callbacks, + * file handles, API clients, rationale, metadata bags, or any field an agent + * could populate. There is no field typed to accept one, so no agent-generated + * value has anywhere to land. + * + * Every field is required. Trusted configuration is all-or-nothing: there is no + * partially configured job and no field that degrades silently. + */ +export interface RepairJobAuthorization { + /** Caller-minted job identity. Exact; never generated here, never truncated. */ + readonly jobId: string; + /** Which policy version authorized this envelope. Audit and permit identity. */ + readonly policyVersion: string; + /** The one repository this job may ever touch. */ + readonly repositoryId: string; + /** The protected parent feature pull request this repair is stacked under. */ + readonly parentPullRequestId: string; + /** + * The protected parent integration ref. + * + * **No job operation may ever write to it.** It appears in exactly one + * authorizable position: as the *target* of the stacked validation change + * request, which does not mutate it. + */ + readonly protectedParentRef: string; + /** The parent pull request's HEAD at the moment the job was configured. */ + readonly parentHeadSha: string; + /** Where the finding came from. A provider-neutral label; grants nothing. */ + readonly findingSource: string; + /** The finding this repair addresses. */ + readonly findingId: string; + /** + * The commit the finding was verified against. + * + * Must equal {@link parentHeadSha} for the job to authorize anything: a + * repair derived from a finding about a commit that is no longer HEAD is a + * repair of something that may no longer exist. + */ + readonly findingHeadSha: string; + /** + * The isolated repair branch. + * + * Must differ from {@link protectedParentRef}. A job whose repair branch is + * the protected parent ref is not a quarantined repair; it is a direct write + * to protected history wearing a repair job's name, and it is rejected as + * malformed configuration rather than evaluated. + */ + readonly repairBranch: string; + /** The isolated repair worktree. Filesystem-shaped operations are bound to it. */ + readonly repairWorktreeId: string; + /** + * Exact repository-relative paths this job may read or edit. + * + * A list of exact paths, not a prefix, glob, or directory. Directory + * authority would require normalisation and containment guarantees this pure + * model cannot prove, and a `src/a` versus `src/ab` prefix boundary is a + * classic escape. An empty list is legitimate: it describes a + * verification-only job. + */ + readonly authorizedPaths: readonly string[]; + /** Verification classes this job may run. An empty list is legitimate. */ + readonly authorizedCommandClasses: readonly VerificationCommandClass[]; + /** The agent performing the repair. Audit only; never authority. */ + readonly repairAgentId: string; + /** + * The validator that must independently validate this repair. + * + * Must differ from {@link repairAgentId}. A repair agent that is also its own + * validator defeats the quarantine the whole pipeline exists to enforce, so + * the job is rejected as malformed configuration. + */ + readonly independentValidatorId: string; +} + +/** + * Every job field, in declaration order. + * + * Invalid-field reporting walks this order, so the result is deterministic. + */ +export const REPAIR_JOB_FIELD_ORDER = objectFreeze([ + 'jobId', + 'policyVersion', + 'repositoryId', + 'parentPullRequestId', + 'protectedParentRef', + 'parentHeadSha', + 'findingSource', + 'findingId', + 'findingHeadSha', + 'repairBranch', + 'repairWorktreeId', + 'authorizedPaths', + 'authorizedCommandClasses', + 'repairAgentId', + 'independentValidatorId', +] as const); + +export type RepairJobField = (typeof REPAIR_JOB_FIELD_ORDER)[number]; + +/** + * A validated, frozen copy of a job's authority envelope. + * + * Authorization reads only a snapshot, never the caller's object. A getter or + * Proxy trap on the configuration cannot therefore validate one repository, + * branch, or path list and have a different one reach the decision or the + * permit. + */ +export interface RepairJobSnapshot { + readonly jobId: string; + readonly policyVersion: string; + readonly repositoryId: string; + readonly parentPullRequestId: string; + readonly protectedParentRef: string; + readonly parentHeadSha: string; + readonly findingSource: string; + readonly findingId: string; + readonly findingHeadSha: string; + readonly repairBranch: string; + readonly repairWorktreeId: string; + readonly authorizedPaths: readonly string[]; + readonly authorizedCommandClasses: readonly VerificationCommandClass[]; + readonly repairAgentId: string; + readonly independentValidatorId: string; +} + +/** The outcome of reading a job envelope exactly once. */ +export interface RepairJobReadResult { + /** The frozen snapshot, or `null` when any field is invalid. */ + readonly snapshot: RepairJobSnapshot | null; + /** Invalid field names in {@link REPAIR_JOB_FIELD_ORDER} order. */ + readonly invalidFields: readonly RepairJobField[]; +} + +const ALL_JOB_FIELDS_INVALID: RepairJobReadResult = objectFreeze({ + snapshot: null, + invalidFields: REPAIR_JOB_FIELD_ORDER, +}); + +/** + * Read and validate a job envelope, in a single pass, exactly once per field. + * + * Pure, total, and deterministic; never throws. This is the only validator: + * {@link findInvalidRepairJobFields} and the authorization evaluator both go + * through it, so there is no second implementation to drift. + * + * Every security-relevant value is read once into a local and never re-read, so + * a getter or Proxy that returns a different value on each access cannot + * validate one operand and hand a different one to the decision. + */ +export function readRepairJobAuthorization(job: RepairJobAuthorization): RepairJobReadResult { + const record: unknown = job; + if (typeof record !== 'object' || record === null) { + return ALL_JOB_FIELDS_INVALID; + } + + const jobId = readExactIdentifier(readOwnProperty(record, 'jobId')); + const policyVersion = readExactIdentifier(readOwnProperty(record, 'policyVersion')); + const repositoryId = readExactIdentifier(readOwnProperty(record, 'repositoryId')); + const parentPullRequestId = readExactIdentifier(readOwnProperty(record, 'parentPullRequestId')); + const protectedParentRef = readExactIdentifier(readOwnProperty(record, 'protectedParentRef')); + const parentHeadSha = readExactIdentifier(readOwnProperty(record, 'parentHeadSha')); + const findingSource = readExactIdentifier(readOwnProperty(record, 'findingSource')); + const findingId = readExactIdentifier(readOwnProperty(record, 'findingId')); + const findingHeadSha = readExactIdentifier(readOwnProperty(record, 'findingHeadSha')); + const repairBranch = readExactIdentifier(readOwnProperty(record, 'repairBranch')); + const repairWorktreeId = readExactIdentifier(readOwnProperty(record, 'repairWorktreeId')); + const authorizedPaths = readList( + readOwnProperty(record, 'authorizedPaths'), + JOB_BOUNDS.MAX_AUTHORIZED_PATHS, + readRepositoryRelativePath, + ); + const authorizedCommandClasses = readList( + readOwnProperty(record, 'authorizedCommandClasses'), + JOB_BOUNDS.MAX_AUTHORIZED_COMMAND_CLASSES, + (element: unknown) => (isVerificationCommandClass(element) ? element : null), + ); + const repairAgentId = readExactIdentifier(readOwnProperty(record, 'repairAgentId')); + const independentValidatorId = readExactIdentifier( + readOwnProperty(record, 'independentValidatorId'), + ); + + const invalidFields: RepairJobField[] = []; + if (jobId === null) { + append(invalidFields, 'jobId'); + } + if (policyVersion === null) { + append(invalidFields, 'policyVersion'); + } + if (repositoryId === null) { + append(invalidFields, 'repositoryId'); + } + if (parentPullRequestId === null) { + append(invalidFields, 'parentPullRequestId'); + } + if (protectedParentRef === null) { + append(invalidFields, 'protectedParentRef'); + } + if (parentHeadSha === null) { + append(invalidFields, 'parentHeadSha'); + } + if (findingSource === null) { + append(invalidFields, 'findingSource'); + } + if (findingId === null) { + append(invalidFields, 'findingId'); + } + if (findingHeadSha === null) { + append(invalidFields, 'findingHeadSha'); + } + // The repair branch must be distinguishable from the protected parent ref, or + // the isolation the whole quarantine depends on does not exist. + if (repairBranch === null || repairBranch === protectedParentRef) { + append(invalidFields, 'repairBranch'); + } + if (repairWorktreeId === null) { + append(invalidFields, 'repairWorktreeId'); + } + if (authorizedPaths === null) { + append(invalidFields, 'authorizedPaths'); + } + if (authorizedCommandClasses === null) { + append(invalidFields, 'authorizedCommandClasses'); + } + if (repairAgentId === null) { + append(invalidFields, 'repairAgentId'); + } + // A repair agent may not be its own independent validator. + if (independentValidatorId === null || independentValidatorId === repairAgentId) { + append(invalidFields, 'independentValidatorId'); + } + + if (invalidFields.length > 0) { + return objectFreeze({ snapshot: null, invalidFields: objectFreeze(invalidFields) }); + } + + // Every value above is non-null here; the narrowing is re-stated per field so + // no assertion operator is used on a security boundary. + if ( + jobId === null || + policyVersion === null || + repositoryId === null || + parentPullRequestId === null || + protectedParentRef === null || + parentHeadSha === null || + findingSource === null || + findingId === null || + findingHeadSha === null || + repairBranch === null || + repairWorktreeId === null || + authorizedPaths === null || + authorizedCommandClasses === null || + repairAgentId === null || + independentValidatorId === null + ) { + return ALL_JOB_FIELDS_INVALID; + } + + return objectFreeze({ + snapshot: objectFreeze({ + jobId, + policyVersion, + repositoryId, + parentPullRequestId, + protectedParentRef, + parentHeadSha, + findingSource, + findingId, + findingHeadSha, + repairBranch, + repairWorktreeId, + authorizedPaths, + authorizedCommandClasses, + repairAgentId, + independentValidatorId, + }), + invalidFields: objectFreeze([] as RepairJobField[]), + }); +} + +/** + * Return the job fields that are missing or invalid, in declaration order. + * + * A thin view over {@link readRepairJobAuthorization}, so a caller that + * pre-validates a job gets exactly the answer the evaluator will get. + */ +export function findInvalidRepairJobFields( + job: RepairJobAuthorization, +): readonly RepairJobField[] { + return readRepairJobAuthorization(job).invalidFields; +} + +/** + * An untrusted claim about who is validating a repair. + * + * Declared shape is advisory: at runtime every property is read defensively and + * any type may arrive. + */ +export interface ValidatorClaim { + readonly validatorId?: string; + /** Read for nothing. Present here only to document that it is ignored. */ + readonly providerId?: string; + /** Read for nothing. A claimed role is not a role. */ + readonly role?: string; +} + +/** + * Does this claim satisfy the job's independent-validator constraint? + * + * The **only** thing consulted is `validatorId`, compared by exact string + * equality against the job's trusted `independentValidatorId`. A claimed role, + * a claimed provider, and a privileged-sounding label are read by nothing. + * + * The repair agent is additionally excluded outright, so the constraint holds + * even if a future change to job validation were to admit a job whose validator + * and repair agent coincide. + * + * C1 implements no validation workflow. This predicate exists so a later layer + * cannot accidentally satisfy the constraint with agent-supplied prose. + */ +export function satisfiesIndependentValidator( + job: RepairJobAuthorization, + claim: ValidatorClaim, +): boolean { + const snapshot = readRepairJobAuthorization(job).snapshot; + if (snapshot === null) { + return false; + } + const record: unknown = claim; + if (typeof record !== 'object' || record === null) { + return false; + } + const validatorId = readExactIdentifier(readOwnProperty(record, 'validatorId')); + if (validatorId === null) { + return false; + } + return validatorId === snapshot.independentValidatorId && validatorId !== snapshot.repairAgentId; +} diff --git a/tests/domain/execution-permit.test.ts b/tests/domain/execution-permit.test.ts new file mode 100644 index 0000000..865e77b --- /dev/null +++ b/tests/domain/execution-permit.test.ts @@ -0,0 +1,409 @@ +import { describe, expect, it } from 'vitest'; + +import { + authorizeJobOperation, + JOB_AUTHORIZATION, + permitAuthorizes, + type ExecutionPermit, + type JobOperationRequest, + type RepairJobAuthorization, +} from '../../src/domain/index.js'; +import { + AUTHORIZED_PATH, + buildEdit, + buildJob, + buildPush, + buildRequest, + HEAD_B, + JOB_B, + NON_OBJECTS, + PARENT_PR_B, + PARENT_REF, + REPAIR_BRANCH, + REPAIR_WORKTREE, + REPO_B, + SECOND_AUTHORIZED_PATH, + throwingRecord, + UNAUTHORIZED_PATH, +} from './repair-job-fixtures.js'; + +/** Issue a permit for the canonical in-scope edit. */ +function issue( + job: RepairJobAuthorization = buildJob(), + request: JobOperationRequest = buildEdit(), +): ExecutionPermit { + const decision = authorizeJobOperation(job, request); + expect(decision.decision).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + const permit = decision.permit; + if (permit === null) { + throw new Error('expected a permit'); + } + return permit; +} + +describe('a permit is bound to exactly one execution', () => { + it('verifies against the job and request it was issued for', () => { + expect(permitAuthorizes(issue(), buildJob(), buildEdit())).toBe(true); + }); + + it('states its single-use scope structurally', () => { + const permit = issue(); + + expect(permit.singleUse).toBe(true); + expect(permit.scope).toBe('exactly-one-execution'); + expect(Object.isFrozen(permit)).toBe(true); + expect(Object.isFrozen(permit.operands)).toBe(true); + + // There is no field a consumer could read as a standing right, a renewal, + // a remaining count, or an expiry it could extend. + for (const key of [ + 'expiresAt', + 'expiry', + 'ttl', + 'uses', + 'usesRemaining', + 'remaining', + 'count', + 'renew', + 'refresh', + 'reusable', + 'persistent', + 'scopes', + 'wildcard', + ]) { + expect(Object.hasOwn(permit, key), key).toBe(false); + } + + expect(Object.keys(permit).sort()).toEqual( + [ + 'jobId', + 'operands', + 'operation', + 'parentHeadSha', + 'parentPullRequestId', + 'permitId', + 'policyVersion', + 'repositoryId', + 'requestId', + 'scope', + 'singleUse', + ].sort(), + ); + }); + + it('gives byte-identical executions the same identity, so replay is detectable', () => { + // Deterministic, not a nonce. A consumer that records consumed permitIds + // can therefore recognise the second presentation of the same execution. + expect(issue().permitId).toBe(issue().permitId); + }); + + it('gives two distinct execution attempts distinct identities', () => { + const first = issue(buildJob(), buildEdit({ requestId: 'req-0001' })); + const second = issue(buildJob(), buildEdit({ requestId: 'req-0002' })); + + expect(first.permitId).not.toBe(second.permitId); + }); + + it('cannot be authorized without a request identity of its own', () => { + const request: Record = { ...buildEdit() }; + delete request['requestId']; + + expect( + authorizeJobOperation(buildJob(), request as unknown as JobOperationRequest) + .mayExecuteOnce, + ).toBe(false); + }); +}); + +describe('cross-job replay', () => { + it('does not verify under a different job', () => { + const permit = issue(); + + expect( + permitAuthorizes(permit, buildJob({ jobId: JOB_B }), buildEdit({ jobId: JOB_B })), + ).toBe(false); + }); + + it('does not verify under a different repository', () => { + const permit = issue(); + + expect( + permitAuthorizes( + permit, + buildJob({ repositoryId: REPO_B }), + buildEdit({ repositoryId: REPO_B }), + ), + ).toBe(false); + }); + + it('does not verify under a different parent pull request', () => { + const permit = issue(); + + expect( + permitAuthorizes( + permit, + buildJob({ parentPullRequestId: PARENT_PR_B }), + buildEdit({ parentPullRequestId: PARENT_PR_B }), + ), + ).toBe(false); + }); + + it('does not verify under a different policy version', () => { + const permit = issue(); + + expect( + permitAuthorizes(permit, buildJob({ policyVersion: 'cockpit-policy-v2' }), buildEdit()), + ).toBe(false); + }); +}); + +describe('cross-operation replay', () => { + it('does not verify for a different operation', () => { + const permit = issue(); + + expect(permitAuthorizes(permit, buildJob(), buildEdit({ operation: 'source.read' }))).toBe( + false, + ); + }); + + it('does not verify for a different path', () => { + const permit = issue(); + + expect( + permitAuthorizes(permit, buildJob(), buildEdit({ path: SECOND_AUTHORIZED_PATH })), + ).toBe(false); + }); + + it('does not verify for a different ref', () => { + const permit = issue(buildJob(), buildPush()); + const job = buildJob({ repairBranch: 'repair/job-0001-b' }); + + expect(permitAuthorizes(permit, job, buildPush({ ref: 'repair/job-0001-b' }))).toBe(false); + }); + + it('does not verify for a different verification class', () => { + const permit = issue( + buildJob(), + buildRequest({ + operation: 'verification.run', + worktreeId: REPAIR_WORKTREE, + commandClass: 'test', + }), + ); + + expect( + permitAuthorizes( + permit, + buildJob(), + buildRequest({ + operation: 'verification.run', + worktreeId: REPAIR_WORKTREE, + commandClass: 'lint', + }), + ), + ).toBe(false); + }); + + it('does not verify for an operation that is no longer in scope', () => { + const permit = issue(); + + expect( + permitAuthorizes(permit, buildJob({ authorizedPaths: [SECOND_AUTHORIZED_PATH] }), buildEdit()), + ).toBe(false); + }); +}); + +describe('a permit is invalid the moment HEAD moves', () => { + it('does not verify once the job is re-bound to a new HEAD', () => { + const permit = issue(); + const movedJob = buildJob({ parentHeadSha: HEAD_B, findingHeadSha: HEAD_B }); + + // The original request still claims the old HEAD, so the job binding refuses. + expect(permitAuthorizes(permit, movedJob, buildEdit())).toBe(false); + // Re-claiming the new HEAD produces a different permit identity rather than + // reviving the old permit. + expect(permitAuthorizes(permit, movedJob, buildEdit({ parentHeadSha: HEAD_B }))).toBe(false); + }); + + it('does not verify when only the request claims the new HEAD', () => { + const permit = issue(); + + expect(permitAuthorizes(permit, buildJob(), buildEdit({ parentHeadSha: HEAD_B }))).toBe(false); + }); + + it('re-issues a distinct permit at the new HEAD', () => { + const before = issue(); + const after = issue( + buildJob({ parentHeadSha: HEAD_B, findingHeadSha: HEAD_B }), + buildEdit({ parentHeadSha: HEAD_B }), + ); + + expect(after.permitId).not.toBe(before.permitId); + }); +}); + +describe('a permit cannot be forged into wider authority', () => { + it('rejects a permit whose fields were widened after issue', () => { + const permit = issue(); + const forged = { + ...permit, + operands: { ...permit.operands, path: UNAUTHORIZED_PATH }, + } as unknown as ExecutionPermit; + + expect(permitAuthorizes(forged, buildJob(), buildEdit())).toBe(false); + expect( + permitAuthorizes(forged, buildJob(), buildEdit({ path: UNAUTHORIZED_PATH })), + ).toBe(false); + }); + + it('rejects a permit that claims not to be single-use', () => { + const permit = issue(); + + for (const widened of [ + { ...permit, singleUse: false }, + { ...permit, scope: 'unlimited' }, + { ...permit, scope: 'exactly-one-execution ' }, + ]) { + expect(permitAuthorizes(widened as unknown as ExecutionPermit, buildJob(), buildEdit())).toBe( + false, + ); + } + }); + + it('rejects a permit whose id was rewritten to match another execution', () => { + const other = issue(buildJob(), buildEdit({ path: SECOND_AUTHORIZED_PATH })); + const forged = { ...issue(), permitId: other.permitId } as unknown as ExecutionPermit; + + expect(permitAuthorizes(forged, buildJob(), buildEdit())).toBe(false); + }); + + it('rejects a permit assembled from scratch for an out-of-scope operation', () => { + // A permit is not a bearer token. Even a perfectly shaped one only ever + // authorizes what the evaluator would authorize at the moment of use. + const forged = { + permitId: 'abp1|whatever', + policyVersion: 'cockpit-policy-v1', + jobId: 'job-0001', + repositoryId: 'github.com/LogicDuke/agentbridge', + parentPullRequestId: '42', + parentHeadSha: 'a'.repeat(40), + requestId: 'req-0001', + operation: 'source.edit', + operands: { + worktreeId: REPAIR_WORKTREE, + path: UNAUTHORIZED_PATH, + commandClass: null, + ref: null, + sourceRef: null, + targetRef: null, + force: false, + }, + singleUse: true, + scope: 'exactly-one-execution', + } as unknown as ExecutionPermit; + + expect( + permitAuthorizes(forged, buildJob(), buildEdit({ path: UNAUTHORIZED_PATH })), + ).toBe(false); + }); + + it('cannot be presented for the protected parent ref', () => { + const permit = issue(buildJob(), buildPush()); + + expect(permitAuthorizes(permit, buildJob(), buildPush({ ref: PARENT_REF }))).toBe(false); + expect(permitAuthorizes(permit, buildJob(), buildPush({ force: true }))).toBe(false); + }); + + it('fails closed on a hostile permit without throwing', () => { + for (const value of NON_OBJECTS) { + expect(() => + permitAuthorizes(value as ExecutionPermit, buildJob(), buildEdit()), + ).not.toThrow(); + expect(permitAuthorizes(value as ExecutionPermit, buildJob(), buildEdit())).toBe( + false, + ); + } + + const throwing = throwingRecord([ + 'permitId', + 'policyVersion', + 'jobId', + 'repositoryId', + 'parentHeadSha', + 'operation', + 'operands', + 'singleUse', + 'scope', + ]) as unknown as ExecutionPermit; + + expect(() => permitAuthorizes(throwing, buildJob(), buildEdit())).not.toThrow(); + expect(permitAuthorizes(throwing, buildJob(), buildEdit())).toBe(false); + }); + + it('never verifies when the underlying decision is not ALLOW_ONCE', () => { + const permit = issue(); + const refusedRequests = [ + buildEdit({ path: UNAUTHORIZED_PATH }), + buildEdit({ operation: 'merge' }), + buildEdit({ operation: 'auto_merge.enable' }), + buildEdit({ operation: 'nonsense' }), + buildPush({ force: true }), + ]; + + for (const request of refusedRequests) { + expect(permitAuthorizes(permit, buildJob(), request), request.operation).toBe(false); + } + }); +}); + +describe('permit identity cannot be collided by operand content', () => { + it('is not confusable by a delimiter inside an operand', () => { + // Length-prefixed encoding: no operand value can straddle a boundary and + // make one execution encode identically to another. + const first = issue( + buildJob({ authorizedPaths: ['a|b/c.ts', 'a/b|c.ts'] }), + buildEdit({ path: 'a|b/c.ts' }), + ); + const second = issue( + buildJob({ authorizedPaths: ['a|b/c.ts', 'a/b|c.ts'] }), + buildEdit({ path: 'a/b|c.ts' }), + ); + + expect(first.permitId).not.toBe(second.permitId); + }); + + it('distinguishes an operand that ends where the next begins', () => { + const first = issue( + buildJob({ authorizedPaths: ['ab/c.ts'] }), + buildEdit({ path: 'ab/c.ts', requestId: 'r1' }), + ); + const second = issue( + buildJob({ authorizedPaths: ['ab/c.ts'] }), + buildEdit({ path: 'ab/c.ts', requestId: 'r1x' }), + ); + + expect(first.permitId).not.toBe(second.permitId); + }); + + it('never omits an operand from identity', () => { + const readPermit = issue(buildJob(), buildEdit({ operation: 'source.read' })); + const editPermit = issue(buildJob(), buildEdit()); + + expect(readPermit.permitId).not.toBe(editPermit.permitId); + expect(readPermit.operands.path).toBe(AUTHORIZED_PATH); + expect(editPermit.operands.path).toBe(AUTHORIZED_PATH); + }); + + it('excludes operands the operation does not define, so they cannot ride along', () => { + const withStowaway = issue( + buildJob(), + buildEdit({ ref: REPAIR_BRANCH, targetRef: PARENT_REF, commandClass: 'test' }), + ); + const clean = issue(buildJob(), buildEdit()); + + expect(withStowaway.permitId).toBe(clean.permitId); + expect(withStowaway.operands.ref).toBeNull(); + expect(withStowaway.operands.targetRef).toBeNull(); + expect(withStowaway.operands.commandClass).toBeNull(); + }); +}); diff --git a/tests/domain/job-authorization-invariants.test.ts b/tests/domain/job-authorization-invariants.test.ts new file mode 100644 index 0000000..bf50859 --- /dev/null +++ b/tests/domain/job-authorization-invariants.test.ts @@ -0,0 +1,737 @@ +import { describe, expect, it } from 'vitest'; + +import { + APPROVAL_STATE, + authorizeJobOperation, + FORBIDDEN_OPERATION, + FORBIDDEN_OPERATIONS, + INVOCATION_BOUNDS, + isForbiddenJobOperation, + isRepairAuthorizableOperation, + JOB_AUTHORIZATION, + JOB_AUTHORIZATION_REASON, + JOB_BOUNDS, + operatorMergeAuthorizes, + readJobOperation, + REPAIR_AUTHORIZABLE_OPERATIONS, + satisfiesIndependentValidator, + type ApprovalRecord, + type JobOperationRequest, + type OperatorMergeAuthorization, + type RepairJobAuthorization, + type ValidatorClaim, +} from '../../src/domain/index.js'; +import { + AUTHORIZED_PATH, + buildEdit, + buildJob, + buildPush, + buildRequest, + HEAD_A, + HEAD_B, + HOSTILE_REQUEST_FIELDS, + NON_OBJECTS, + PARENT_PR_A, + PARENT_REF, + PRIVILEGED_LABELS, + REPAIR_BRANCH, + REPAIR_WORKTREE, + REPO_A, + revokedProxy, + throwingRecord, + unstableRecord, + UNAUTHORIZED_PATH, + withPrototypePollution, +} from './repair-job-fixtures.js'; + +/* ------------------------------------------------------------------------- + * The merge barrier + * ------------------------------------------------------------------------- */ + +describe('merge is operator-only, permanently', () => { + it('answers OPERATOR_REQUIRED for merge and issues no permit', () => { + const decision = authorizeJobOperation( + buildJob(), + buildRequest({ operation: FORBIDDEN_OPERATION.MERGE }), + ); + + expect(decision.decision).toBe(JOB_AUTHORIZATION.OPERATOR_REQUIRED); + expect(decision.reason).toBe(JOB_AUTHORIZATION_REASON.MERGE_IS_OPERATOR_ONLY); + expect(decision.mayExecuteOnce).toBe(false); + expect(decision.permit).toBeNull(); + }); + + it('denies auto-merge outright, which is a stronger answer than OPERATOR_REQUIRED', () => { + const decision = authorizeJobOperation( + buildJob(), + buildRequest({ operation: FORBIDDEN_OPERATION.AUTO_MERGE_ENABLE }), + ); + + // Auto-merge delegates the operator's decision away from the moment HEAD is + // final, so there is no operator path to it either. + expect(decision.decision).toBe(JOB_AUTHORIZATION.DENY); + expect(decision.reason).toBe(JOB_AUTHORIZATION_REASON.OPERATION_FORBIDDEN); + expect(decision.permit).toBeNull(); + }); + + it('refuses merge under every job, operand, and label permutation', () => { + const jobs: readonly RepairJobAuthorization[] = [ + buildJob(), + buildJob({ repairAgentId: 'root', independentValidatorId: 'system' }), + buildJob({ findingSource: 'agentbridge-internal' }), + buildJob({ authorizedPaths: [], authorizedCommandClasses: [] }), + buildJob({ protectedParentRef: 'main' }), + buildJob({ repairBranch: 'main', protectedParentRef: 'main' }), + ]; + const operands: readonly Partial[] = [ + {}, + { ref: PARENT_REF }, + { ref: REPAIR_BRANCH }, + { sourceRef: REPAIR_BRANCH, targetRef: PARENT_REF }, + { worktreeId: REPAIR_WORKTREE, path: AUTHORIZED_PATH }, + { force: true }, + { force: false }, + ]; + + for (const job of jobs) { + for (const operand of operands) { + for (const label of PRIVILEGED_LABELS) { + const request = { + ...buildRequest({ operation: FORBIDDEN_OPERATION.MERGE, ...operand }), + ...HOSTILE_REQUEST_FIELDS, + agentId: label, + providerId: label, + } as unknown as JobOperationRequest; + const decision = authorizeJobOperation(job, request); + + expect(decision.decision, label).toBe(JOB_AUTHORIZATION.OPERATOR_REQUIRED); + expect(decision.mayExecuteOnce, label).toBe(false); + expect(decision.permit, label).toBeNull(); + } + } + } + }); + + it('has no authorizable operation that is a forbidden one', () => { + for (const operation of REPAIR_AUTHORIZABLE_OPERATIONS) { + expect(FORBIDDEN_OPERATIONS, operation).not.toContain(operation); + expect(isForbiddenJobOperation(operation), operation).toBe(false); + } + for (const operation of FORBIDDEN_OPERATIONS) { + expect(REPAIR_AUTHORIZABLE_OPERATIONS, operation).not.toContain(operation); + expect(isRepairAuthorizableOperation(operation), operation).toBe(false); + } + }); + + it('never allows any operation that is forbidden by name', () => { + for (const operation of FORBIDDEN_OPERATIONS) { + const decision = authorizeJobOperation(buildJob(), buildRequest({ operation })); + + expect(decision.decision, operation).not.toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + expect(decision.mayExecuteOnce, operation).toBe(false); + expect(decision.permit, operation).toBeNull(); + } + }); + + it('mentions merge in the operation vocabulary but never in a permit type', () => { + // The type-level half of the barrier: ExecutionPermit.operation is a + // RepairAuthorizableOperation, so this does not compile. + // + // @ts-expect-error merge is not a repair-authorizable operation + const merge: (typeof REPAIR_AUTHORIZABLE_OPERATIONS)[number] = 'merge'; + expect(merge).toBe('merge'); + }); + + it('cannot be turned into job authority by a human ApprovalRecord', () => { + // `authorizeJobOperation` has no approval parameter, so the only way to try + // is to smuggle one through the request. It is read by nothing. + const approval: ApprovalRecord = { + requestId: 'req-0001', + state: APPROVAL_STATE.APPROVED, + decidedBy: 'operator', + decidedAt: '2026-08-14T00:00:00Z', + }; + const plain = authorizeJobOperation( + buildJob(), + buildRequest({ operation: FORBIDDEN_OPERATION.MERGE }), + ); + const withApproval = authorizeJobOperation( + buildJob(), + { + ...buildRequest({ operation: FORBIDDEN_OPERATION.MERGE }), + approval, + approvalState: APPROVAL_STATE.APPROVED, + approved: true, + } as unknown as JobOperationRequest, + ); + + expect(withApproval).toEqual(plain); + expect(withApproval.decision).toBe(JOB_AUTHORIZATION.OPERATOR_REQUIRED); + expect(withApproval.permit).toBeNull(); + expect(authorizeJobOperation).toHaveLength(2); + }); + + it('cannot be reached by an ordinarily forbidden operation carrying an approval', () => { + const forbidden = [ + FORBIDDEN_OPERATION.PARENT_REF_WRITE, + FORBIDDEN_OPERATION.PUSH_FORCE, + FORBIDDEN_OPERATION.HISTORY_REWRITE, + FORBIDDEN_OPERATION.BRANCH_DELETE, + FORBIDDEN_OPERATION.POLICY_MODIFY, + FORBIDDEN_OPERATION.SECRET_ACCESS, + FORBIDDEN_OPERATION.DEPLOYMENT_RUN, + FORBIDDEN_OPERATION.STAGING_CHANGE, + FORBIDDEN_OPERATION.PRODUCTION_CHANGE, + FORBIDDEN_OPERATION.DATABASE_WRITE, + FORBIDDEN_OPERATION.DATABASE_MIGRATE, + ]; + + for (const operation of forbidden) { + const decision = authorizeJobOperation( + buildJob(), + { + ...buildRequest({ operation }), + ...HOSTILE_REQUEST_FIELDS, + } as unknown as JobOperationRequest, + ); + + expect(decision.decision, operation).toBe(JOB_AUTHORIZATION.DENY); + expect(decision.reason, operation).toBe(JOB_AUTHORIZATION_REASON.OPERATION_FORBIDDEN); + expect(decision.permit, operation).toBeNull(); + } + }); + + it('never produces an operator merge authorization from job authority', () => { + const decision = authorizeJobOperation(buildJob(), buildEdit()); + const serialized = JSON.stringify(decision); + + for (const key of ['authorizationId', 'operatorId', 'merge', 'mergeable', 'readyForMerge']) { + expect(serialized, key).not.toContain(key); + } + }); +}); + +describe('operator merge authorization is separate, exact, and single-use', () => { + const authorization: OperatorMergeAuthorization = { + authorizationId: 'op-merge-1', + operatorId: 'human-operator-1', + repositoryId: REPO_A, + pullRequestId: PARENT_PR_A, + headSha: HEAD_A, + authorizedAt: '2026-08-14T00:00:00Z', + singleUse: true, + }; + + it('covers exactly the repository, pull request, and HEAD it names', () => { + expect( + operatorMergeAuthorizes(authorization, { + repositoryId: REPO_A, + pullRequestId: PARENT_PR_A, + currentHeadSha: HEAD_A, + }), + ).toBe(true); + }); + + it('becomes invalid the moment HEAD moves', () => { + expect( + operatorMergeAuthorizes(authorization, { + repositoryId: REPO_A, + pullRequestId: PARENT_PR_A, + currentHeadSha: HEAD_B, + }), + ).toBe(false); + }); + + it('cannot authorize another pull request or another repository', () => { + expect( + operatorMergeAuthorizes(authorization, { + repositoryId: REPO_A, + pullRequestId: '43', + currentHeadSha: HEAD_A, + }), + ).toBe(false); + expect( + operatorMergeAuthorizes(authorization, { + repositoryId: 'github.com/other/repo', + pullRequestId: PARENT_PR_A, + currentHeadSha: HEAD_A, + }), + ).toBe(false); + }); + + it('refuses an authorization that is not structurally single-use', () => { + const widened = { ...authorization, singleUse: false } as unknown as OperatorMergeAuthorization; + + expect( + operatorMergeAuthorizes(widened, { + repositoryId: REPO_A, + pullRequestId: PARENT_PR_A, + currentHeadSha: HEAD_A, + }), + ).toBe(false); + }); + + it('fails closed on hostile input without throwing', () => { + for (const value of NON_OBJECTS) { + expect(() => + operatorMergeAuthorizes(value as OperatorMergeAuthorization, { + repositoryId: REPO_A, + pullRequestId: PARENT_PR_A, + currentHeadSha: HEAD_A, + }), + ).not.toThrow(); + expect( + operatorMergeAuthorizes(value as OperatorMergeAuthorization, { + repositoryId: REPO_A, + pullRequestId: PARENT_PR_A, + currentHeadSha: HEAD_A, + }), + ).toBe(false); + } + + expect( + operatorMergeAuthorizes( + throwingRecord(['authorizationId', 'operatorId', 'repositoryId', 'headSha']) as unknown as OperatorMergeAuthorization, + { repositoryId: REPO_A, pullRequestId: PARENT_PR_A, currentHeadSha: HEAD_A }, + ), + ).toBe(false); + expect( + operatorMergeAuthorizes(revokedProxy() as unknown as OperatorMergeAuthorization, { + repositoryId: REPO_A, + pullRequestId: PARENT_PR_A, + currentHeadSha: HEAD_A, + }), + ).toBe(false); + }); +}); + +/* ------------------------------------------------------------------------- + * Identity, prose, and metadata are inert + * ------------------------------------------------------------------------- */ + +describe('nothing an agent controls can increase authority', () => { + it('produces a byte-identical decision with and without hostile fields', () => { + const clean = authorizeJobOperation(buildJob(), buildEdit()); + const hostile = authorizeJobOperation( + buildJob(), + { ...buildEdit(), ...HOSTILE_REQUEST_FIELDS } as unknown as JobOperationRequest, + ); + + expect(JSON.stringify(hostile)).toBe(JSON.stringify(clean)); + }); + + it('does not let hostile fields rescue an out-of-scope edit', () => { + const decision = authorizeJobOperation( + buildJob(), + { + ...buildEdit({ path: UNAUTHORIZED_PATH }), + ...HOSTILE_REQUEST_FIELDS, + } as unknown as JobOperationRequest, + ); + + expect(decision.reason).toBe(JOB_AUTHORIZATION_REASON.PATH_NOT_AUTHORIZED); + expect(decision.permit).toBeNull(); + }); + + it('is unchanged by a privileged-sounding repair agent or finding source', () => { + const baseline = authorizeJobOperation(buildJob(), buildEdit()); + + for (const label of PRIVILEGED_LABELS) { + const job = buildJob({ + repairAgentId: label, + findingSource: label, + independentValidatorId: `${label}-validator`, + }); + const decision = authorizeJobOperation(job, buildEdit()); + + expect(decision.decision, label).toBe(baseline.decision); + expect(decision.permit?.permitId, label).toBe(baseline.permit?.permitId); + } + }); + + it('is unchanged by a privileged-sounding requester label on a denied operation', () => { + const baseline = authorizeJobOperation(buildJob(), buildEdit({ path: UNAUTHORIZED_PATH })); + + for (const label of PRIVILEGED_LABELS) { + const decision = authorizeJobOperation( + buildJob(), + { + ...buildEdit({ path: UNAUTHORIZED_PATH }), + agentId: label, + providerId: label, + actorId: label, + role: label, + } as unknown as JobOperationRequest, + ); + + expect(JSON.stringify(decision), label).toBe(JSON.stringify(baseline)); + } + }); + + it('never widens file or command scope from the request', () => { + // The request claims a wider scope than the job configures. The job wins. + const decision = authorizeJobOperation( + buildJob({ authorizedPaths: [], authorizedCommandClasses: [] }), + { + ...buildEdit(), + authorizedPaths: [AUTHORIZED_PATH, UNAUTHORIZED_PATH, '**'], + authorizedCommandClasses: ['test', 'lint', 'typecheck', 'build', 'audit'], + scope: '**', + } as unknown as JobOperationRequest, + ); + + expect(decision.reason).toBe(JOB_AUTHORIZATION_REASON.PATH_NOT_AUTHORIZED); + expect(decision.permit).toBeNull(); + }); + + it('never lets a request rewrite the protected parent or repair branch', () => { + const decision = authorizeJobOperation( + buildJob(), + { + ...buildPush({ ref: PARENT_REF }), + protectedParentRef: 'some-other-ref', + repairBranch: PARENT_REF, + } as unknown as JobOperationRequest, + ); + + expect(decision.reason).toBe(JOB_AUTHORIZATION_REASON.PROTECTED_REF_MUTATION); + }); +}); + +describe('the independent-validator constraint', () => { + it('is satisfied only by the configured validator identity', () => { + expect(satisfiesIndependentValidator(buildJob(), { validatorId: 'validator-1' })).toBe(true); + }); + + it('is not satisfied by the repair agent claiming another role or provider', () => { + const job = buildJob(); + const claims: readonly ValidatorClaim[] = [ + { validatorId: 'repair-agent-1' }, + { validatorId: 'repair-agent-1', role: 'independent-validator' }, + { validatorId: 'repair-agent-1', providerId: 'coderabbit' }, + { validatorId: 'repair-agent-1', role: 'validator', providerId: 'agentbridge-internal' }, + { role: 'independent-validator' }, + { role: 'validator-1' }, + { providerId: 'validator-1' }, + {}, + ]; + + for (const claim of claims) { + expect(satisfiesIndependentValidator(job, claim), JSON.stringify(claim)).toBe(false); + } + }); + + it('is not satisfied by a privileged-sounding label', () => { + for (const label of PRIVILEGED_LABELS) { + expect( + satisfiesIndependentValidator(buildJob(), { validatorId: label, role: label }), + label, + ).toBe(false); + } + }); + + it('cannot be satisfied at all when the job is invalid', () => { + const job = buildJob({ independentValidatorId: 'repair-agent-1' }); + + expect(satisfiesIndependentValidator(job, { validatorId: 'repair-agent-1' })).toBe(false); + }); + + it('fails closed on hostile claims without throwing', () => { + for (const claim of NON_OBJECTS) { + expect(() => + satisfiesIndependentValidator(buildJob(), claim as ValidatorClaim), + ).not.toThrow(); + expect(satisfiesIndependentValidator(buildJob(), claim as ValidatorClaim)).toBe( + false, + ); + } + expect( + satisfiesIndependentValidator( + buildJob(), + throwingRecord(['validatorId']) as unknown as ValidatorClaim, + ), + ).toBe(false); + }); +}); + +/* ------------------------------------------------------------------------- + * Hostile runtime + * ------------------------------------------------------------------------- */ + +describe('hostile runtime input fails closed without throwing', () => { + it('survives a non-object job or request', () => { + for (const value of NON_OBJECTS) { + expect(() => + authorizeJobOperation(value as RepairJobAuthorization, buildEdit()), + ).not.toThrow(); + expect( + authorizeJobOperation(value as RepairJobAuthorization, buildEdit()) + .mayExecuteOnce, + ).toBe(false); + + expect(() => + authorizeJobOperation(buildJob(), value as JobOperationRequest), + ).not.toThrow(); + expect( + authorizeJobOperation(buildJob(), value as JobOperationRequest).mayExecuteOnce, + ).toBe(false); + } + }); + + it('survives throwing getters on every field of both arguments', () => { + const jobKeys = [ + 'jobId', + 'policyVersion', + 'repositoryId', + 'parentPullRequestId', + 'protectedParentRef', + 'parentHeadSha', + 'findingSource', + 'findingId', + 'findingHeadSha', + 'repairBranch', + 'repairWorktreeId', + 'authorizedPaths', + 'authorizedCommandClasses', + 'repairAgentId', + 'independentValidatorId', + ]; + const requestKeys = [ + 'requestId', + 'jobId', + 'repositoryId', + 'parentPullRequestId', + 'parentHeadSha', + 'operation', + 'worktreeId', + 'path', + 'commandClass', + 'ref', + 'sourceRef', + 'targetRef', + 'force', + ]; + + expect(() => + authorizeJobOperation(throwingRecord(jobKeys) as unknown as RepairJobAuthorization, buildEdit()), + ).not.toThrow(); + expect( + authorizeJobOperation( + throwingRecord(jobKeys) as unknown as RepairJobAuthorization, + buildEdit(), + ).mayExecuteOnce, + ).toBe(false); + + expect(() => + authorizeJobOperation(buildJob(), throwingRecord(requestKeys) as unknown as JobOperationRequest), + ).not.toThrow(); + expect( + authorizeJobOperation(buildJob(), throwingRecord(requestKeys) as unknown as JobOperationRequest) + .mayExecuteOnce, + ).toBe(false); + }); + + it('survives a revoked Proxy as either argument', () => { + expect(() => + authorizeJobOperation(revokedProxy() as unknown as RepairJobAuthorization, buildEdit()), + ).not.toThrow(); + expect(() => + authorizeJobOperation(buildJob(), revokedProxy() as unknown as JobOperationRequest), + ).not.toThrow(); + expect( + authorizeJobOperation(buildJob(), revokedProxy() as unknown as JobOperationRequest) + .mayExecuteOnce, + ).toBe(false); + }); + + it('survives a revoked Proxy as the authorized path list', () => { + const job = buildJob({ + authorizedPaths: revokedProxy() as unknown as readonly string[], + }); + + expect(() => authorizeJobOperation(job, buildEdit())).not.toThrow(); + expect(authorizeJobOperation(job, buildEdit()).reason).toBe( + JOB_AUTHORIZATION_REASON.JOB_ENVELOPE_INVALID, + ); + }); + + it('survives an array-like Proxy whose length is hostile', () => { + const hostile = new Proxy([AUTHORIZED_PATH], { + get(target, key, receiver): unknown { + if (key === 'length') { + return Number.MAX_SAFE_INTEGER; + } + return Reflect.get(target, key, receiver); + }, + }); + const job = buildJob({ authorizedPaths: hostile }); + + expect(() => authorizeJobOperation(job, buildEdit())).not.toThrow(); + expect(authorizeJobOperation(job, buildEdit()).mayExecuteOnce).toBe(false); + }); +}); + +describe('a value read twice cannot differ between validation and use', () => { + it('reads each request operand exactly once', () => { + // The path reads as authorized first and as an escape afterwards. A + // boundary that validated one read and used another would authorize + // `../../etc/passwd`. + const request = unstableRecord({ ...buildEdit() }, 'path', [ + AUTHORIZED_PATH, + '../../etc/passwd', + '../../etc/passwd', + ]) as unknown as JobOperationRequest; + + const decision = authorizeJobOperation(buildJob(), request); + + // Whatever the first read returned is the value that was both validated and + // written into the permit. The two can never diverge. + expect(decision.permit?.operands.path).toBe(AUTHORIZED_PATH); + expect(decision.decision).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + + const normalized = readJobOperation( + unstableRecord({ ...buildEdit() }, 'path', [ + AUTHORIZED_PATH, + '../../etc/passwd', + ]) as unknown as JobOperationRequest, + ); + expect(normalized.path).toBe(AUTHORIZED_PATH); + // Re-reading the frozen snapshot always answers the same way. + expect(normalized.path).toBe(AUTHORIZED_PATH); + }); + + it('reads the ref exactly once, so a push cannot be validated then redirected', () => { + const request = unstableRecord({ ...buildPush() }, 'ref', [ + REPAIR_BRANCH, + PARENT_REF, + PARENT_REF, + ]) as unknown as JobOperationRequest; + + const decision = authorizeJobOperation(buildJob(), request); + + expect(decision.permit?.operands.ref).toBe(REPAIR_BRANCH); + expect(decision.permit?.operands.ref).not.toBe(PARENT_REF); + }); + + it('reads the job envelope into a snapshot, so a later read cannot widen it', () => { + // `authorizedPaths` reads as a narrow list first and a wide one afterwards. + const job = unstableRecord({ ...buildJob() }, 'authorizedPaths', [ + [AUTHORIZED_PATH], + [AUTHORIZED_PATH, UNAUTHORIZED_PATH], + [AUTHORIZED_PATH, UNAUTHORIZED_PATH], + ]) as unknown as RepairJobAuthorization; + + const decision = authorizeJobOperation(job, buildEdit({ path: UNAUTHORIZED_PATH })); + + expect(decision.reason).toBe(JOB_AUTHORIZATION_REASON.PATH_NOT_AUTHORIZED); + expect(decision.permit).toBeNull(); + }); + + it('copies the authorized path list, so mutating the caller’s array changes nothing', () => { + const paths = [AUTHORIZED_PATH]; + const job = buildJob({ authorizedPaths: paths }); + const before = authorizeJobOperation(job, buildEdit({ path: UNAUTHORIZED_PATH })); + + paths.push(UNAUTHORIZED_PATH); + const after = authorizeJobOperation(job, buildEdit({ path: UNAUTHORIZED_PATH })); + + // The snapshot is taken per call, so this one does change — the guarantee is + // that it cannot change *within* a single evaluation, which the unstable + // getter test above pins. Recorded here so the boundary of the claim is + // explicit rather than assumed. + expect(before.reason).toBe(JOB_AUTHORIZATION_REASON.PATH_NOT_AUTHORIZED); + expect(after.decision).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + expect(Object.isFrozen(before.invalidJobFields)).toBe(true); + }); +}); + +describe('prototype pollution and inherited properties create no authority', () => { + it('ignores authorization fields planted on Object.prototype', () => { + const baseline = authorizeJobOperation(buildJob(), buildEdit({ path: UNAUTHORIZED_PATH })); + + const polluted = withPrototypePollution( + { + path: AUTHORIZED_PATH, + worktreeId: REPAIR_WORKTREE, + ref: REPAIR_BRANCH, + operation: 'source.edit', + force: false, + authorizedPaths: [UNAUTHORIZED_PATH], + mayExecuteOnce: true, + singleUse: true, + }, + () => authorizeJobOperation(buildJob(), buildEdit({ path: UNAUTHORIZED_PATH })), + ); + + expect(JSON.stringify(polluted)).toBe(JSON.stringify(baseline)); + }); + + it('does not let an inherited operand supply a missing own one', () => { + // The request has no own `path` at all; every value would have to come from + // the prototype. + const request = buildRequest({ operation: 'source.edit', worktreeId: REPAIR_WORKTREE }); + + const polluted = withPrototypePollution({ path: AUTHORIZED_PATH }, () => + authorizeJobOperation(buildJob(), request), + ); + + expect(polluted.mayExecuteOnce).toBe(false); + expect(polluted.reason).toBe(JOB_AUTHORIZATION_REASON.OPERAND_MISSING); + }); + + it('does not let an inherited job field supply a missing own one', () => { + const job: Record = { ...buildJob() }; + delete job['repairWorktreeId']; + + const polluted = withPrototypePollution({ repairWorktreeId: REPAIR_WORKTREE }, () => + authorizeJobOperation(job as unknown as RepairJobAuthorization, buildEdit()), + ); + + expect(polluted.reason).toBe(JOB_AUTHORIZATION_REASON.JOB_ENVELOPE_INVALID); + expect(polluted.invalidJobFields).toContain('repairWorktreeId'); + }); + + it('does not resolve a prototype-shaped operation name', () => { + for (const operation of ['__proto__', 'constructor', 'prototype', 'toString', 'valueOf']) { + const decision = authorizeJobOperation(buildJob(), buildEdit({ operation })); + + expect(decision.reason, operation).toBe(JOB_AUTHORIZATION_REASON.OPERATION_UNKNOWN); + } + }); + + it('is unaffected by prototype-shaped keys on the request', () => { + const baseline = authorizeJobOperation(buildJob(), buildEdit()); + + for (const key of ['__proto__', 'constructor', 'prototype', 'toString', 'valueOf']) { + const decision = authorizeJobOperation( + buildJob(), + { ...buildEdit(), [key]: 'polluted' } as unknown as JobOperationRequest, + ); + + expect(JSON.stringify(decision), key).toBe(JSON.stringify(baseline)); + } + }); +}); + +/* ------------------------------------------------------------------------- + * Cross-boundary conventions + * ------------------------------------------------------------------------- */ + +describe('bounds stay aligned with the neighbouring boundaries', () => { + it('uses the same identifier bound as the PR 006 invocation boundary', () => { + // A jobId may be correlated with an invocationId. The two boundaries share + // no code, so the convention is pinned by a test rather than by an import. + expect(JOB_BOUNDS.MAX_IDENTIFIER_LENGTH).toBe(INVOCATION_BOUNDS.MAX_IDENTIFIER_LENGTH); + }); + + it('rejects rather than truncates a maximum-length-plus-one identifier', () => { + const atLimit = 'j'.repeat(JOB_BOUNDS.MAX_IDENTIFIER_LENGTH); + const overLimit = 'j'.repeat(JOB_BOUNDS.MAX_IDENTIFIER_LENGTH + 1); + + expect(readJobOperation({ requestId: atLimit }).requestId).toBe(atLimit); + expect(readJobOperation({ requestId: overLimit }).requestId).toBeNull(); + // The prefix never reaches the output, so two ids sharing a 256-character + // prefix can never collapse into one. + expect(readJobOperation({ requestId: overLimit }).requestId).not.toBe(atLimit); + }); +}); diff --git a/tests/domain/job-authorization.test.ts b/tests/domain/job-authorization.test.ts new file mode 100644 index 0000000..d2acde3 --- /dev/null +++ b/tests/domain/job-authorization.test.ts @@ -0,0 +1,636 @@ +import { describe, expect, it } from 'vitest'; + +import { + authorizeJobOperation, + findInvalidRepairJobFields, + JOB_AUTHORIZATION, + JOB_AUTHORIZATION_REASON, + readRepositoryRelativePath, + type JobOperationRequest, + type RepairJobAuthorization, +} from '../../src/domain/index.js'; +import { + AUTHORIZED_PATH, + buildEdit, + buildJob, + buildPush, + buildRequest, + HEAD_A, + HEAD_B, + JOB_B, + PARENT_PR_B, + PARENT_REF, + REPAIR_BRANCH, + REPAIR_WORKTREE, + REPO_B, + SECOND_AUTHORIZED_PATH, + UNAUTHORIZED_PATH, +} from './repair-job-fixtures.js'; + +/** Nothing was authorized, and no permit escaped. */ +function expectRefused( + decision: ReturnType, + reason: string, +): void { + expect(decision.reason).toBe(reason); + expect(decision.mayExecuteOnce).toBe(false); + expect(decision.permit).toBeNull(); +} + +describe('the authorized edit', () => { + it('allows an exact authorized edit, once, with a permit', () => { + const decision = authorizeJobOperation(buildJob(), buildEdit()); + + expect(decision.decision).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + expect(decision.reason).toBe(JOB_AUTHORIZATION_REASON.WITHIN_JOB_ENVELOPE); + expect(decision.mayExecuteOnce).toBe(true); + expect(decision.permit?.operation).toBe('source.edit'); + expect(decision.permit?.operands.path).toBe(AUTHORIZED_PATH); + expect(decision.permit?.singleUse).toBe(true); + expect(decision.permit?.scope).toBe('exactly-one-execution'); + }); + + it('allows every operation the envelope covers, and only with its own operands', () => { + const job = buildJob(); + const allowed: readonly JobOperationRequest[] = [ + buildEdit({ operation: 'source.read' }), + buildEdit(), + buildEdit({ operation: 'source.edit', path: SECOND_AUTHORIZED_PATH }), + buildRequest({ + operation: 'verification.run', + worktreeId: REPAIR_WORKTREE, + commandClass: 'test', + }), + buildRequest({ + operation: 'repair.commit', + worktreeId: REPAIR_WORKTREE, + ref: REPAIR_BRANCH, + }), + buildPush(), + buildRequest({ + operation: 'repair.change_request', + sourceRef: REPAIR_BRANCH, + targetRef: PARENT_REF, + }), + ]; + + for (const request of allowed) { + const decision = authorizeJobOperation(job, request); + expect(decision.decision, request.operation).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + } + }); + + it('carries only the operands its own operation defines', () => { + // A read request that also names the protected parent ref: the ref has no + // meaning for a read, and must not ride along into the permit. + const decision = authorizeJobOperation( + buildJob(), + buildEdit({ operation: 'source.read', ref: PARENT_REF, sourceRef: PARENT_REF }), + ); + + expect(decision.decision).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + expect(decision.permit?.operands.ref).toBeNull(); + expect(decision.permit?.operands.sourceRef).toBeNull(); + expect(decision.permit?.operands.commandClass).toBeNull(); + expect(decision.permit?.operands.force).toBe(false); + }); +}); + +describe('binding: one job authorizes one repository, pull request, and HEAD', () => { + it('denies a request naming another repository', () => { + expectRefused( + authorizeJobOperation(buildJob(), buildEdit({ repositoryId: REPO_B })), + JOB_AUTHORIZATION_REASON.REPOSITORY_MISMATCH, + ); + }); + + it('denies a request naming another parent pull request', () => { + expectRefused( + authorizeJobOperation(buildJob(), buildEdit({ parentPullRequestId: PARENT_PR_B })), + JOB_AUTHORIZATION_REASON.PARENT_PULL_REQUEST_MISMATCH, + ); + }); + + it('denies a request naming another parent HEAD', () => { + expectRefused( + authorizeJobOperation(buildJob(), buildEdit({ parentHeadSha: HEAD_B })), + JOB_AUTHORIZATION_REASON.PARENT_HEAD_MISMATCH, + ); + }); + + it('denies a request naming another job', () => { + expectRefused( + authorizeJobOperation(buildJob(), buildEdit({ jobId: JOB_B })), + JOB_AUTHORIZATION_REASON.JOB_MISMATCH, + ); + }); + + it('denies a request that omits a binding field rather than defaulting it', () => { + for (const field of [ + 'jobId', + 'repositoryId', + 'parentPullRequestId', + 'parentHeadSha', + ] as const) { + const request: Record = { ...buildEdit() }; + Reflect.deleteProperty(request, field); + const decision = authorizeJobOperation( + buildJob(), + request as unknown as JobOperationRequest, + ); + + expect(decision.mayExecuteOnce, field).toBe(false); + expect(decision.permit, field).toBeNull(); + } + }); + + it('denies when the job binds to a different HEAD than the finding was verified against', () => { + // The job is internally inconsistent: the finding was verified at HEAD_B + // while the job is bound to HEAD_A. The repair would target something the + // finding never described. + expectRefused( + authorizeJobOperation(buildJob({ findingHeadSha: HEAD_B }), buildEdit()), + JOB_AUTHORIZATION_REASON.FINDING_SHA_STALE, + ); + }); + + it('denies a stale-finding job for every authorizable operation, not just edits', () => { + const job = buildJob({ findingHeadSha: HEAD_B }); + for (const request of [buildEdit(), buildPush()]) { + const decision = authorizeJobOperation(job, request); + expect(decision.reason, request.operation).toBe( + JOB_AUTHORIZATION_REASON.FINDING_SHA_STALE, + ); + } + }); +}); + +describe('authorized file scope', () => { + it('denies an edit outside the authorized paths', () => { + expectRefused( + authorizeJobOperation(buildJob(), buildEdit({ path: UNAUTHORIZED_PATH })), + JOB_AUTHORIZATION_REASON.PATH_NOT_AUTHORIZED, + ); + }); + + it('does not treat an authorized path as authority over its directory', () => { + // `src/domain/policy-gate.ts` is authorized. Its directory is not, and + // neither is a sibling, a prefix-extension, or the directory itself. + for (const path of [ + 'src/domain', + 'src/domain/', + 'src/domain/policy-gate.ts.bak', + 'src/domain/policy-gate.tsx', + 'src', + ]) { + const decision = authorizeJobOperation(buildJob(), buildEdit({ path })); + expect(decision.mayExecuteOnce, path).toBe(false); + } + }); + + it('denies malformed and escaping paths without resolving them', () => { + const hostile = [ + '../secrets.env', + 'src/../../etc/passwd', + './src/domain/policy-gate.ts', + '/etc/passwd', + 'C:/Windows/System32/config', + 'src\\domain\\policy-gate.ts', + '~/.ssh/id_rsa', + '.git/config', + '.GIT/hooks/pre-commit', + 'src//domain/policy-gate.ts', + 'src/domain/policy-gate.ts/', + 'src/domain/policy-gate.ts\u0000.png', + 'file.txt:stream', + '', + ' ', + ]; + + for (const path of hostile) { + const decision = authorizeJobOperation(buildJob(), buildEdit({ path })); + expect(decision.mayExecuteOnce, path).toBe(false); + expect(decision.permit, path).toBeNull(); + } + }); + + it('rejects the same hostile paths in job configuration, all-or-nothing', () => { + for (const path of ['../x', '/x', '.git/config', 'a\\b']) { + const job = buildJob({ authorizedPaths: [AUTHORIZED_PATH, path] }); + + expect(findInvalidRepairJobFields(job), path).toContain('authorizedPaths'); + // And the good path in the same list is not salvaged. + expect(authorizeJobOperation(job, buildEdit()).reason, path).toBe( + JOB_AUTHORIZATION_REASON.JOB_ENVELOPE_INVALID, + ); + } + }); + + it('authorizes no file at all when the scope is empty', () => { + const job = buildJob({ authorizedPaths: [] }); + + expect(findInvalidRepairJobFields(job)).toEqual([]); + expectRefused( + authorizeJobOperation(job, buildEdit()), + JOB_AUTHORIZATION_REASON.PATH_NOT_AUTHORIZED, + ); + }); + + it('reads a path exactly, with no normalisation', () => { + expect(readRepositoryRelativePath('src/a.ts')).toBe('src/a.ts'); + // Not case-folded: an authorized path is not its uppercase spelling. + expect(readRepositoryRelativePath('SRC/A.TS')).toBe('SRC/A.TS'); + expect( + authorizeJobOperation(buildJob(), buildEdit({ path: AUTHORIZED_PATH.toUpperCase() })) + .mayExecuteOnce, + ).toBe(false); + }); + + it('rejects segments Windows would silently rewrite', () => { + // `a.txt ` and `a.txt` compare unequal here but can name one file on + // Windows, so neither padded form is readable at all. + for (const path of ['src/a.ts ', ' src/a.ts', 'src /a.ts', 'src/a.ts.', 'src./a.ts']) { + expect(readRepositoryRelativePath(path), path).toBeNull(); + } + expect( + authorizeJobOperation(buildJob(), buildEdit({ path: `${AUTHORIZED_PATH} ` })).mayExecuteOnce, + ).toBe(false); + }); + + it('rejects a .git segment at any depth', () => { + for (const path of [ + '.git/config', + '.GIT/hooks/pre-commit', + '.Git/objects/x', + 'vendor/lib/.git/config', + 'a/b/.git/hooks/post-checkout', + ]) { + expect(readRepositoryRelativePath(path), path).toBeNull(); + } + }); + + it('denies an edit in a worktree that is not the job\u2019s repair worktree', () => { + expectRefused( + authorizeJobOperation(buildJob(), buildEdit({ worktreeId: 'parent-worktree' })), + JOB_AUTHORIZATION_REASON.WORKTREE_NOT_AUTHORIZED, + ); + // An omitted worktree is refused exactly like a wrong one; it does not + // default to the repair worktree. + expectRefused( + authorizeJobOperation( + buildJob(), + buildRequest({ operation: 'source.edit', path: AUTHORIZED_PATH }), + ), + JOB_AUTHORIZATION_REASON.WORKTREE_NOT_AUTHORIZED, + ); + }); +}); + +describe('refs: the repair branch is not the protected parent', () => { + it('distinguishes a push to the repair branch from a push to the parent', () => { + expect(authorizeJobOperation(buildJob(), buildPush()).decision).toBe( + JOB_AUTHORIZATION.ALLOW_ONCE, + ); + expectRefused( + authorizeJobOperation(buildJob(), buildPush({ ref: PARENT_REF })), + JOB_AUTHORIZATION_REASON.PROTECTED_REF_MUTATION, + ); + }); + + it('denies a commit onto the protected parent ref', () => { + expectRefused( + authorizeJobOperation( + buildJob(), + buildRequest({ + operation: 'repair.commit', + worktreeId: REPAIR_WORKTREE, + ref: PARENT_REF, + }), + ), + JOB_AUTHORIZATION_REASON.PROTECTED_REF_MUTATION, + ); + }); + + it('denies a push to any third ref', () => { + for (const ref of ['main', 'develop', 'release/1.0', 'repair/job-0002']) { + const decision = authorizeJobOperation(buildJob(), buildPush({ ref })); + expect(decision.reason, ref).toBe(JOB_AUTHORIZATION_REASON.REF_NOT_REPAIR_BRANCH); + } + }); + + it('always denies a force push, including to the authorized repair branch', () => { + expectRefused( + authorizeJobOperation(buildJob(), buildPush({ force: true })), + JOB_AUTHORIZATION_REASON.FORCE_PUSH_FORBIDDEN, + ); + expectRefused( + authorizeJobOperation(buildJob(), buildPush({ ref: PARENT_REF, force: true })), + JOB_AUTHORIZATION_REASON.FORCE_PUSH_FORBIDDEN, + ); + }); + + it('treats an unreadable force flag as force', () => { + // Only an absent or literally `false` flag is not a force. Anything a + // requester could smuggle in that is merely falsy is still a force. + const forces: readonly unknown[] = [0, '', null, 'false', 'no', NaN, {}, []]; + for (const [index, force] of forces.entries()) { + const decision = authorizeJobOperation( + buildJob(), + buildPush({ force } as unknown as Partial), + ); + expect(decision.reason, `force[${String(index)}]`).toBe( + JOB_AUTHORIZATION_REASON.FORCE_PUSH_FORBIDDEN, + ); + } + }); + + it('allows a push whose force flag is absent', () => { + const request: Record = { ...buildPush() }; + delete request['force']; + + expect( + authorizeJobOperation(buildJob(), request as unknown as JobOperationRequest).decision, + ).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + }); + + it('requires the stacked change request to run from repair branch to protected parent', () => { + const job = buildJob(); + + // The one authorized shape. + expect( + authorizeJobOperation( + job, + buildRequest({ + operation: 'repair.change_request', + sourceRef: REPAIR_BRANCH, + targetRef: PARENT_REF, + }), + ).decision, + ).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + + // Reversed: a change request *from* the protected parent is not a stacked + // validation PR, it is a proposal to move the parent. + expectRefused( + authorizeJobOperation( + job, + buildRequest({ + operation: 'repair.change_request', + sourceRef: PARENT_REF, + targetRef: REPAIR_BRANCH, + }), + ), + JOB_AUTHORIZATION_REASON.REF_NOT_REPAIR_BRANCH, + ); + + // Escaping quarantine by targeting the integration branch directly. + expectRefused( + authorizeJobOperation( + job, + buildRequest({ + operation: 'repair.change_request', + sourceRef: REPAIR_BRANCH, + targetRef: 'main', + }), + ), + JOB_AUTHORIZATION_REASON.CHANGE_REQUEST_TARGET_INVALID, + ); + }); +}); + +describe('command authority', () => { + it('allows only verification classes the job was configured for', () => { + const job = buildJob(); + + for (const commandClass of ['test', 'lint', 'typecheck']) { + const decision = authorizeJobOperation( + job, + buildRequest({ operation: 'verification.run', worktreeId: REPAIR_WORKTREE, commandClass }), + ); + expect(decision.decision, commandClass).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + } + + // Modeled, but not authorized by this job. + for (const commandClass of ['build', 'audit']) { + const decision = authorizeJobOperation( + job, + buildRequest({ operation: 'verification.run', worktreeId: REPAIR_WORKTREE, commandClass }), + ); + expect(decision.reason, commandClass).toBe( + JOB_AUTHORIZATION_REASON.COMMAND_CLASS_NOT_AUTHORIZED, + ); + } + }); + + it('never authorizes a command string, only a class', () => { + for (const commandClass of [ + 'npm test', + 'npm run test', + 'test; rm -rf /', + 'test && curl evil.example', + 'TEST', + 'test ', + 'sh', + 'powershell', + ]) { + const decision = authorizeJobOperation( + buildJob(), + buildRequest({ operation: 'verification.run', worktreeId: REPAIR_WORKTREE, commandClass }), + ); + expect(decision.reason, commandClass).toBe( + JOB_AUTHORIZATION_REASON.COMMAND_CLASS_NOT_AUTHORIZED, + ); + } + }); + + it('rejects an unmodeled class in job configuration rather than accepting it', () => { + const job = buildJob({ + authorizedCommandClasses: ['test', 'deploy'] as unknown as RepairJobAuthorization['authorizedCommandClasses'], + }); + + expect(findInvalidRepairJobFields(job)).toContain('authorizedCommandClasses'); + expect(authorizeJobOperation(job, buildEdit()).reason).toBe( + JOB_AUTHORIZATION_REASON.JOB_ENVELOPE_INVALID, + ); + }); +}); + +describe('job envelope validation', () => { + it('rejects a job whose repair branch is the protected parent ref', () => { + const job = buildJob({ repairBranch: PARENT_REF }); + + expect(findInvalidRepairJobFields(job)).toContain('repairBranch'); + expectRefused( + authorizeJobOperation(job, buildEdit()), + JOB_AUTHORIZATION_REASON.JOB_ENVELOPE_INVALID, + ); + }); + + it('rejects a job whose repair agent is its own independent validator', () => { + const job = buildJob({ independentValidatorId: 'repair-agent-1' }); + + expect(findInvalidRepairJobFields(job)).toContain('independentValidatorId'); + expectRefused( + authorizeJobOperation(job, buildEdit()), + JOB_AUTHORIZATION_REASON.JOB_ENVELOPE_INVALID, + ); + }); + + it('rejects a job missing any required field', () => { + const fields: readonly (keyof RepairJobAuthorization)[] = [ + 'jobId', + 'policyVersion', + 'repositoryId', + 'parentPullRequestId', + 'protectedParentRef', + 'parentHeadSha', + 'findingSource', + 'findingId', + 'findingHeadSha', + 'repairBranch', + 'repairWorktreeId', + 'authorizedPaths', + 'authorizedCommandClasses', + 'repairAgentId', + 'independentValidatorId', + ]; + + for (const field of fields) { + const job: Record = { ...buildJob() }; + Reflect.deleteProperty(job, field); + const typed = job as unknown as RepairJobAuthorization; + + expect(findInvalidRepairJobFields(typed), field).toContain(field); + expect(authorizeJobOperation(typed, buildEdit()).mayExecuteOnce, field).toBe(false); + } + }); + + it('reports invalid fields in declaration order', () => { + const job = buildJob({ repositoryId: '', findingId: ' ', repairAgentId: '' }); + + expect(findInvalidRepairJobFields(job)).toEqual([ + 'repositoryId', + 'findingId', + 'repairAgentId', + ]); + }); + + it('rejects an oversized identifier rather than truncating it', () => { + const long = 'j'.repeat(257); + const job = buildJob({ jobId: long }); + + expect(findInvalidRepairJobFields(job)).toContain('jobId'); + expect(authorizeJobOperation(job, buildEdit({ jobId: long })).mayExecuteOnce).toBe(false); + }); + + it('rejects an oversized authorized-path list rather than truncating it', () => { + const paths: string[] = [AUTHORIZED_PATH]; + for (let index = 0; index < 512; index += 1) { + paths.push(`src/f${String(index)}.ts`); + } + const job = buildJob({ authorizedPaths: paths }); + + expect(findInvalidRepairJobFields(job)).toContain('authorizedPaths'); + expect(authorizeJobOperation(job, buildEdit()).mayExecuteOnce).toBe(false); + }); + + it('rejects a sparse authorized-path list rather than collapsing the holes', () => { + const sparse: string[] = []; + sparse[0] = AUTHORIZED_PATH; + sparse[3] = SECOND_AUTHORIZED_PATH; + const job = buildJob({ authorizedPaths: sparse }); + + expect(findInvalidRepairJobFields(job)).toContain('authorizedPaths'); + }); + + it('accepts a fully valid job with no invalid fields', () => { + expect(findInvalidRepairJobFields(buildJob())).toEqual([]); + }); +}); + +describe('unknown operations', () => { + it('denies every unmodeled operation name', () => { + for (const operation of [ + 'repository.write', + 'git.push', + 'shell.exec', + 'source.write', + 'SOURCE.EDIT', + 'source.edit ', + ' source.edit', + 'source_edit', + 'unknown', + 'toString', + 'constructor', + '__proto__', + 'valueOf', + 'hasOwnProperty', + '', + ]) { + const decision = authorizeJobOperation(buildJob(), buildEdit({ operation })); + + expect(decision.reason, operation).toBe(JOB_AUTHORIZATION_REASON.OPERATION_UNKNOWN); + expect(decision.decision, operation).toBe(JOB_AUTHORIZATION.DENY); + expect(decision.permit, operation).toBeNull(); + } + }); + + it('denies an absent or non-string operation', () => { + const operations: readonly unknown[] = [undefined, null, 0, 1, true, {}, [], Symbol('op')]; + for (const [index, operation] of operations.entries()) { + const decision = authorizeJobOperation( + buildJob(), + buildEdit({ operation } as unknown as Partial), + ); + expect(decision.mayExecuteOnce, `operation[${String(index)}]`).toBe(false); + } + }); +}); + +describe('the decision record', () => { + it('never echoes rationale, metadata, or requester identity', () => { + const decision = authorizeJobOperation(buildJob(), buildEdit()); + const serialized = JSON.stringify(decision); + + for (const key of [ + 'rationale', + 'metadata', + 'agentId', + 'providerId', + 'actorId', + 'approval', + 'approvalState', + 'role', + 'override', + ]) { + expect(Object.hasOwn(decision, key), key).toBe(false); + expect(serialized, key).not.toContain(key); + } + }); + + it('survives a JSON round trip unchanged', () => { + const decision = authorizeJobOperation(buildJob(), buildEdit()); + + expect(JSON.parse(JSON.stringify(decision)) as unknown).toEqual(decision); + }); + + it('is frozen, along with the permit it carries', () => { + const decision = authorizeJobOperation(buildJob(), buildEdit()); + + expect(Object.isFrozen(decision)).toBe(true); + expect(Object.isFrozen(decision.permit)).toBe(true); + expect(Object.isFrozen(decision.permit?.operands)).toBe(true); + }); + + it('is deterministic', () => { + const first = authorizeJobOperation(buildJob(), buildEdit()); + const second = authorizeJobOperation(buildJob(), buildEdit()); + + expect(JSON.stringify(first)).toBe(JSON.stringify(second)); + }); + + it('echoes the exact HEAD it was decided against', () => { + const decision = authorizeJobOperation(buildJob(), buildEdit()); + + expect(decision.permit?.parentHeadSha).toBe(HEAD_A); + }); +}); diff --git a/tests/domain/repair-job-fixtures.ts b/tests/domain/repair-job-fixtures.ts new file mode 100644 index 0000000..53a0eac --- /dev/null +++ b/tests/domain/repair-job-fixtures.ts @@ -0,0 +1,220 @@ +import type { + JobOperationRequest, + RepairJobAuthorization, +} from '../../src/domain/index.js'; + +/** Repository A. Every fixture that is not explicitly cross-repository uses it. */ +export const REPO_A = 'github.com/LogicDuke/agentbridge'; +/** A different repository, for cross-repository attempts. */ +export const REPO_B = 'github.com/LogicDuke/other'; + +export const PARENT_PR_A = '42'; +export const PARENT_PR_B = '43'; + +export const HEAD_A = 'a'.repeat(40); +export const HEAD_B = 'b'.repeat(40); + +export const JOB_A = 'job-0001'; +export const JOB_B = 'job-0002'; + +export const POLICY_VERSION = 'cockpit-policy-v1'; + +export const PARENT_REF = 'feature/pr-042-parent'; +export const REPAIR_BRANCH = 'repair/job-0001'; +export const REPAIR_WORKTREE = 'worktree-job-0001'; + +export const AUTHORIZED_PATH = 'src/domain/policy-gate.ts'; +export const SECOND_AUTHORIZED_PATH = 'tests/domain/policy-gate.test.ts'; +export const UNAUTHORIZED_PATH = 'src/domain/actions.ts'; + +export const REQUEST_ID = 'req-0001'; + +export function buildJob( + overrides: Partial = {}, +): RepairJobAuthorization { + return { + jobId: JOB_A, + policyVersion: POLICY_VERSION, + repositoryId: REPO_A, + parentPullRequestId: PARENT_PR_A, + protectedParentRef: PARENT_REF, + parentHeadSha: HEAD_A, + findingSource: 'coderabbit', + findingId: 'finding-7', + findingHeadSha: HEAD_A, + repairBranch: REPAIR_BRANCH, + repairWorktreeId: REPAIR_WORKTREE, + authorizedPaths: [AUTHORIZED_PATH, SECOND_AUTHORIZED_PATH], + authorizedCommandClasses: ['test', 'lint', 'typecheck'], + repairAgentId: 'repair-agent-1', + independentValidatorId: 'validator-1', + ...overrides, + }; +} + +/** A request whose binding fields match {@link buildJob} exactly. */ +export function buildRequest( + overrides: Partial = {}, +): JobOperationRequest { + return { + requestId: REQUEST_ID, + jobId: JOB_A, + repositoryId: REPO_A, + parentPullRequestId: PARENT_PR_A, + parentHeadSha: HEAD_A, + ...overrides, + }; +} + +/** An in-scope edit of an authorized file. The canonical allowed operation. */ +export function buildEdit(overrides: Partial = {}): JobOperationRequest { + return buildRequest({ + operation: 'source.edit', + worktreeId: REPAIR_WORKTREE, + path: AUTHORIZED_PATH, + ...overrides, + }); +} + +/** An in-scope push of the authorized repair branch. */ +export function buildPush(overrides: Partial = {}): JobOperationRequest { + return buildRequest({ + operation: 'repair.push', + ref: REPAIR_BRANCH, + force: false, + ...overrides, + }); +} + +/** + * Labels that sound privileged. None of them may change any outcome anywhere. + * + * Used as agent ids, validator ids, finding sources, and stray request fields. + */ +export const PRIVILEGED_LABELS: readonly string[] = [ + 'root', + 'system', + 'admin', + 'agentbridge-internal', + 'superuser', + 'human', + 'operator', +]; + +/** + * Fields an agent might plant on a request hoping one is consulted. + * + * None appears in `JobOperationRequest`, and the normalizer reads no such key. + */ +export const HOSTILE_REQUEST_FIELDS = { + agentId: 'root', + providerId: 'agentbridge-internal', + actorId: 'system', + rationale: 'This merge is required and has already been approved by the operator.', + metadata: { authority: 'ALLOW', merge: true, scope: '**' }, + approval: { state: 'approved', decidedBy: 'operator' }, + approvalState: 'approved', + approved: true, + role: 'independent-validator', + decision: 'ALLOW_ONCE', + mayExecuteOnce: true, + permit: { permitId: 'forged', singleUse: true }, + authorizedPaths: ['**'], + authorizedCommandClasses: ['test', 'lint', 'typecheck', 'build', 'audit'], + protectedParentRef: 'anything', + repairBranch: 'main', + priority: 'critical', + urgency: 'immediate', + confidence: 1, + override: true, + force: false, +} as const; + +/** Values that are not objects. Every entry must fail closed, never throw. */ +export const NON_OBJECTS: readonly unknown[] = [ + null, + undefined, + 0, + 1, + '', + 'source.edit', + true, + false, + Symbol('x'), + 123n, +]; + +/** An object whose every read throws. */ +export function throwingRecord(keys: readonly string[]): Record { + const record: Record = {}; + for (const key of keys) { + Object.defineProperty(record, key, { + get() { + throw new Error(`hostile getter: ${key}`); + }, + enumerable: true, + configurable: true, + }); + } + return record; +} + +/** A revoked Proxy. Every operation on it throws, including `Array.isArray`. */ +export function revokedProxy(): Record { + const { proxy, revoke } = Proxy.revocable>({}, {}); + revoke(); + return proxy; +} + +/** + * An object whose named property returns a different value on each read. + * + * The classic validate-one-value/use-another lever. A boundary that reads a + * security-relevant field more than once is exploitable with this. + */ +export function unstableRecord( + base: Record, + key: string, + values: readonly unknown[], +): Record { + let reads = 0; + const record: Record = { ...base }; + Object.defineProperty(record, key, { + get() { + const value = values[Math.min(reads, values.length - 1)]; + reads += 1; + return value; + }, + enumerable: true, + configurable: true, + }); + return record; +} + +/** Run `body` with properties planted on `Object.prototype`, then restore. */ +export function withPrototypePollution( + values: Record, + body: () => T, +): T { + const saved: Record = {}; + for (const key of Object.keys(values)) { + saved[key] = Object.getOwnPropertyDescriptor(Object.prototype, key); + Object.defineProperty(Object.prototype, key, { + value: values[key], + configurable: true, + writable: true, + }); + } + try { + return body(); + } finally { + for (const key of Object.keys(values)) { + const descriptor = saved[key]; + if (descriptor === undefined) { + Reflect.deleteProperty(Object.prototype, key); + } else { + Object.defineProperty(Object.prototype, key, descriptor); + } + } + } +} From 7575192d0ea8a808401a5f8eff873fa7bf465609 Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Sat, 15 Aug 2026 23:36:48 +0200 Subject: [PATCH 02/15] fix: harden cockpit operation resolution 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) --- src/domain/job-operation.ts | 41 +++-- .../job-authorization-invariants.test.ts | 146 ++++++++++++++++++ 2 files changed, 171 insertions(+), 16 deletions(-) diff --git a/src/domain/job-operation.ts b/src/domain/job-operation.ts index 9f5ed17..ccb6144 100644 --- a/src/domain/job-operation.ts +++ b/src/domain/job-operation.ts @@ -154,21 +154,6 @@ export type JobOperation = | ForbiddenJobOperation | UnknownJobOperation; -/** - * Membership is backed by a `Map`, not a plain object. - * - * A plain-object lookup inherits `Object.prototype`, so `'toString'`, - * `'constructor'`, and `'__proto__'` would resolve to a truthy entry. A `Map` - * has no prototype chain for keys. Same reasoning as PR 002's taxonomy. - */ -const OPERATION_LOOKUP: ReadonlyMap = - new Map([ - ...REPAIR_AUTHORIZABLE_OPERATIONS.map( - (operation) => [operation, operation] as const, - ), - ...FORBIDDEN_OPERATIONS.map((operation) => [operation, operation] as const), - ]); - /** * Resolve an untrusted operation name to a modeled member. * @@ -179,12 +164,36 @@ const OPERATION_LOOKUP: ReadonlyMap(body: () => T): T { + const saved = Object.getOwnPropertyDescriptor(Map.prototype, 'get'); + Object.defineProperty(Map.prototype, 'get', { + value: function poisonedGet(): string { + return JOB_OPERATION.SOURCE_EDIT; + }, + writable: true, + configurable: true, + }); + try { + return body(); + } finally { + if (saved === undefined) { + Reflect.deleteProperty(Map.prototype, 'get'); + } else { + Object.defineProperty(Map.prototype, 'get', saved); + } + } +} + +describe('poisoning Map.prototype.get cannot re-resolve an operation', () => { + it('proves the poisoning is actually in effect', () => { + // Without this the whole section could pass vacuously, on a runtime that + // was never hostile in the first place. + const observed = withPoisonedMapGet(() => new Map().get('anything')); + + expect(observed).toBe(JOB_OPERATION.SOURCE_EDIT); + }); + + it('keeps merge resolving to merge', () => { + const resolved = withPoisonedMapGet(() => resolveJobOperation(FORBIDDEN_OPERATION.MERGE)); + + expect(resolved).toBe(FORBIDDEN_OPERATION.MERGE); + }); + + it('keeps auto_merge.enable resolving to auto_merge.enable', () => { + const resolved = withPoisonedMapGet(() => + resolveJobOperation(FORBIDDEN_OPERATION.AUTO_MERGE_ENABLE), + ); + + expect(resolved).toBe(FORBIDDEN_OPERATION.AUTO_MERGE_ENABLE); + }); + + it('keeps an unmodeled operation resolving to unknown', () => { + const resolved = withPoisonedMapGet(() => resolveJobOperation('shell.exec')); + + expect(resolved).toBe(UNKNOWN_JOB_OPERATION); + }); + + it('keeps source.edit resolving to source.edit', () => { + const resolved = withPoisonedMapGet(() => resolveJobOperation(JOB_OPERATION.SOURCE_EDIT)); + + expect(resolved).toBe(JOB_OPERATION.SOURCE_EDIT); + }); + + it('resolves every modeled operation to itself and nothing else', () => { + const modeled = [...REPAIR_AUTHORIZABLE_OPERATIONS, ...FORBIDDEN_OPERATIONS]; + + const resolved = withPoisonedMapGet(() => modeled.map((o) => resolveJobOperation(o))); + + expect(resolved).toStrictEqual(modeled); + }); + + it('does not let a merge request reach ALLOW_ONCE', () => { + // The request carries valid `source.edit` operands, so nothing earlier in + // the evaluator can refuse it on an operand ground. The only thing standing + // between it and a permit is that `merge` still resolves as `merge`. + const decision = withPoisonedMapGet(() => + authorizeJobOperation( + buildJob(), + buildEdit({ operation: FORBIDDEN_OPERATION.MERGE }), + ), + ); + + expect(decision.operation).toBe(FORBIDDEN_OPERATION.MERGE); + expect(decision.decision).toBe(JOB_AUTHORIZATION.OPERATOR_REQUIRED); + expect(decision.reason).toBe(JOB_AUTHORIZATION_REASON.MERGE_IS_OPERATOR_ONLY); + expect(decision.mayExecuteOnce).toBe(false); + expect(decision.permit).toBeNull(); + }); + + it('does not let an auto-merge request reach ALLOW_ONCE', () => { + const decision = withPoisonedMapGet(() => + authorizeJobOperation( + buildJob(), + buildEdit({ operation: FORBIDDEN_OPERATION.AUTO_MERGE_ENABLE }), + ), + ); + + expect(decision.operation).toBe(FORBIDDEN_OPERATION.AUTO_MERGE_ENABLE); + expect(decision.decision).toBe(JOB_AUTHORIZATION.DENY); + expect(decision.reason).toBe(JOB_AUTHORIZATION_REASON.OPERATION_FORBIDDEN); + expect(decision.mayExecuteOnce).toBe(false); + expect(decision.permit).toBeNull(); + }); + + it('does not let an unmodeled request reach ALLOW_ONCE', () => { + const decision = withPoisonedMapGet(() => + authorizeJobOperation(buildJob(), buildEdit({ operation: 'shell.exec' })), + ); + + expect(decision.operation).toBe(UNKNOWN_JOB_OPERATION); + expect(decision.decision).toBe(JOB_AUTHORIZATION.DENY); + expect(decision.reason).toBe(JOB_AUTHORIZATION_REASON.OPERATION_UNKNOWN); + expect(decision.mayExecuteOnce).toBe(false); + expect(decision.permit).toBeNull(); + }); + + it('still authorizes a legitimate source.edit under the same poisoning', () => { + // Fail-closed is not enough on its own: a repair that refused everything + // would satisfy every assertion above and break the boundary instead. + const baseline = authorizeJobOperation(buildJob(), buildEdit()); + const poisoned = withPoisonedMapGet(() => authorizeJobOperation(buildJob(), buildEdit())); + + expect(poisoned.decision).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + expect(poisoned.reason).toBe(JOB_AUTHORIZATION_REASON.WITHIN_JOB_ENVELOPE); + expect(poisoned.mayExecuteOnce).toBe(true); + expect(JSON.stringify(poisoned)).toBe(JSON.stringify(baseline)); + }); + + it('restores Map.prototype.get afterwards', () => { + withPoisonedMapGet(() => undefined); + + expect(new Map([['k', 'v']]).get('k')).toBe('v'); + }); +}); + /* ------------------------------------------------------------------------- * Cross-boundary conventions * ------------------------------------------------------------------------- */ From 96f2f0ff07954ea5579437d954b05180e2d14f0a Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Sun, 16 Aug 2026 01:04:04 +0200 Subject: [PATCH 03/15] fix: clarify cockpit merge authority guarantees 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) --- docs/architecture/C1-repair-job-authority.md | 45 +++++++++- src/domain/execution-permit.ts | 84 +++++++++++++++---- .../job-authorization-invariants.test.ts | 41 ++++++++- 3 files changed, 148 insertions(+), 22 deletions(-) diff --git a/docs/architecture/C1-repair-job-authority.md b/docs/architecture/C1-repair-job-authority.md index 19bd966..d08405f 100644 --- a/docs/architecture/C1-repair-job-authority.md +++ b/docs/architecture/C1-repair-job-authority.md @@ -262,13 +262,50 @@ turns a human decision into a record of this shape does not exist yet, and building it is an explicit later decision rather than an implementation detail of whichever layer needs it first. -`operatorMergeAuthorizes` enforces every required property: operator-originated, -repository-bound, pull-request-bound, exact-HEAD-SHA-bound, structurally -single-use, invalid the moment HEAD changes, and incapable of authorizing another -pull request or a future SHA. +`operatorMergeAuthorizes` checks **binding, and only binding**. The distinction +between what C1 proves and what a later layer must enforce is stated exactly, +because overclaiming here would be worse than not checking. + +**Proved by a `true` result.** The candidate record carries the required +structural fields as readable identifiers; its `singleUse` is literally `true`; +and its `repositoryId`, `pullRequestId`, and `headSha` are exactly equal to the +target repository, target pull request, and the repository's *current* HEAD. Every +comparison is exact string equality, so the record is invalid the moment HEAD +changes and can never cover another pull request, another repository, or a future +SHA. There is no path that widens, refreshes, or re-binds it. + +**Not proved, and not claimed.** + +| Property | C1 | +| --- | --- | +| operator origin | **not proved** — the argument is untrusted data; a plain object literal written by any caller passes | +| human identity / authentication | **not proved** — `operatorId` is a readable string; C1 reads no credential and authenticates nothing | +| trusted minting, signature, possession | **not proved** — C1 has no issuing boundary and no secret material, so it cannot distinguish a minted record from an assembled one | +| uniqueness / one-time consumption | **not proved** — C1 has no consumed-capability store | +| replay prevention | **not proved** — the identical record returns `true` on every call while HEAD is unchanged | + +`singleUse: true` is a **structural intent marker**: it records that the shape is +that of a single-use capability. It is *not* enforced single consumption, and +this document does not claim the record is replay-proof or that it cannot be +reused. Likewise, a non-empty `operatorId` is descriptive data; it does not make +the record operator-originated, authenticated, or human-authorized. + +**A `true` result is therefore not sufficient proof that a merge is +operator-authorized.** It is a necessary binding check. A future trusted operator +boundary — the merge broker, an explicitly reviewed later Cockpit layer — is +responsible for authenticating the operator, establishing that the record was +minted by that boundary rather than supplied by a caller, and consuming the +record so it cannot authorize a second merge. C1 deliberately implements none of +that: no authentication, no signatures, no token issuance, no secret material, no +replay or consumed-capability store, no operator session, and no endpoint. **No merge executor and no GitHub merge API call exists in this PR.** +None of this weakens the merge barrier above. `OperatorMergeAuthorization` is not +wired into `authorizeJobOperation`, and ordinary repair-job authority still +receives `OPERATOR_REQUIRED` with no permit for `merge`, regardless of what any +record of this shape says. + ## Execution permits An `ExecutionPermit` is the record of one authorization: exactly one job, exactly diff --git a/src/domain/execution-permit.ts b/src/domain/execution-permit.ts index b556074..c04ef30 100644 --- a/src/domain/execution-permit.ts +++ b/src/domain/execution-permit.ts @@ -1,5 +1,5 @@ /** - * One-time execution permits, and the separate operator merge authority + * One-time execution permits, and the separate operator merge authority *shape* * (Cockpit C1). * * ## A permit is not a bearer token @@ -242,7 +242,8 @@ export function permitsEqual(candidate: ExecutionPermit, issued: ExecutionPermit } /** - * A merge authorization that originated from a human operator. + * The structural shape a merge authorization must have. **Not proof that one + * exists.** * * **This is not job authority and is not reachable from job authority.** No * function in AgentBridge produces one: there is no factory, no builder, and no @@ -258,19 +259,35 @@ export function permitsEqual(candidate: ExecutionPermit, issued: ExecutionPermit * data about an `ActionRequest` at PR 003's gate; reusing it here would make * every existing approval a candidate merge authority. * - * The required properties, all of which {@link operatorMergeAuthorizes} - * enforces: + * ## A value of this type is untrusted data, not authority + * + * Nothing in C1 establishes where such a value came from. There is no minting + * boundary, so a caller can write the object literal by hand and C1 will read it + * exactly as it reads any other untrusted record. What this layer models is the + * *binding* a merge authority must carry: * - * - operator-originated — `operatorId` names a human, and nothing in the - * domain mints one * - repository-bound, pull-request-bound, and bound to an exact HEAD SHA - * - single-use, and invalid the moment HEAD changes + * - carrying the structural `singleUse: true` marker * - incapable of authorizing another pull request or a future SHA + * + * {@link operatorMergeAuthorizes} checks exactly that binding, and nothing more. + * It does **not** establish operator origin, human identity, authentication, + * trusted minting, uniqueness, one-time consumption, or replay prevention. + * + * A future trusted operator boundary — the merge broker — owns those properties: + * it must authenticate the operator, guarantee that the record was minted by + * that boundary rather than assembled by a caller, and record the record as + * consumed so it cannot authorize a second merge. Until that boundary exists, a + * record of this shape proves nothing about a human. */ export interface OperatorMergeAuthorization { /** Caller-minted identity of this one operator decision. */ readonly authorizationId: string; - /** The human who decided. Never an agent, and never inferred from a label. */ + /** + * Identifier of the operator a future trusted boundary must authenticate. + * Descriptive data here: C1 checks only that it is a readable identifier, and + * never establishes that it names a human rather than an agent or a caller. + */ readonly operatorId: string; /** The one repository this authorization is valid in. */ readonly repositoryId: string; @@ -280,7 +297,11 @@ export interface OperatorMergeAuthorization { readonly headSha: string; /** Caller-supplied timestamp. Data; no clock is read here. */ readonly authorizedAt: string; - /** Structural: one merge, then nothing. */ + /** + * Structural intent: this record is *shaped* as a single-use capability. C1 + * has no consumed-capability store, so single consumption is not enforced + * here — a later trusted boundary must enforce it. + */ readonly singleUse: true; } @@ -293,19 +314,48 @@ export interface MergeTarget { } /** - * Does this operator authorization cover exactly this merge, right now? + * Is this candidate record *structurally bound* to exactly this merge, right + * now? * * Pure, total, and deterministic; never throws. Both arguments are read * defensively, own-only, and exactly once. * - * Every comparison is exact string equality, so a HEAD that moved by one commit - * invalidates the authorization, and an authorization for pull request 41 can - * never cover pull request 42. There is no path that widens, refreshes, or - * re-binds an authorization to a newer SHA: a new HEAD requires a new operator - * decision. + * ## What a `true` result proves + * + * Only that the candidate carries the required structural fields as readable + * identifiers, that its `singleUse` is literally `true`, and that its + * `repositoryId`, `pullRequestId`, and `headSha` are exactly equal to the + * target's repository, pull request, and *current* HEAD. Every comparison is + * exact string equality, so a HEAD that moved by one commit invalidates the + * record, and a record naming pull request 41 can never cover pull request 42. + * There is no path that widens, refreshes, or re-binds a record to a newer SHA: + * a new HEAD requires a new operator decision. + * + * ## What a `true` result does not prove + * + * Stated explicitly, because overclaiming here would be worse than not checking: + * + * - **Operator origin.** The first argument is untrusted data. A plain object + * literal, written by any caller with the right field names, satisfies this + * predicate. + * - **Human identity or authentication.** `operatorId` is a readable string and + * nothing more. C1 performs no authentication and reads no credential. + * - **Trusted minting or possession.** There is no signature, no secret, and no + * issuing boundary, so this predicate cannot distinguish a record a trusted + * boundary minted from one a caller assembled. + * - **Uniqueness, one-time consumption, or replay prevention.** C1 stores + * nothing and consumes nothing. The identical record returns `true` on every + * call for as long as HEAD has not moved. `singleUse: true` is a structural + * intent marker, not enforcement. + * + * **A `true` result is therefore not sufficient proof that a merge is + * operator-authorized.** It is a necessary binding check that a future trusted + * operator boundary / merge broker must run *in addition to* authenticating the + * operator, verifying that it minted the record itself, and recording the record + * as consumed. Those properties belong to that later, explicitly reviewed layer. * - * C1 executes no merge. This predicate exists so that the merge barrier is - * defined by something more precise than a comment. + * C1 executes no merge. This predicate exists so that the binding half of the + * merge barrier is defined by something more precise than a comment. */ export function operatorMergeAuthorizes( authorization: OperatorMergeAuthorization, diff --git a/tests/domain/job-authorization-invariants.test.ts b/tests/domain/job-authorization-invariants.test.ts index 0198261..3d501d7 100644 --- a/tests/domain/job-authorization-invariants.test.ts +++ b/tests/domain/job-authorization-invariants.test.ts @@ -214,7 +214,7 @@ describe('merge is operator-only, permanently', () => { }); }); -describe('operator merge authorization is separate, exact, and single-use', () => { +describe('operator merge authorization is separate, exact, and structurally single-use', () => { const authorization: OperatorMergeAuthorization = { authorizationId: 'op-merge-1', operatorId: 'human-operator-1', @@ -274,6 +274,45 @@ describe('operator merge authorization is separate, exact, and single-use', () = ).toBe(false); }); + // The next two tests pin what C1 deliberately does NOT prove, so that the + // documented limitation cannot drift away from the implementation. They are + // not a statement that this behaviour is desirable forever: the future trusted + // operator boundary / merge broker must supersede both, and when it does these + // tests are expected to be replaced rather than preserved. + it('accepts a plain caller-written literal: it proves binding, not operator origin', () => { + const callerWritten = { + authorizationId: 'assembled-by-any-caller', + operatorId: 'not-authenticated-just-a-string', + repositoryId: REPO_A, + pullRequestId: PARENT_PR_A, + headSha: HEAD_A, + authorizedAt: 'caller-supplied', + singleUse: true, + } as const; + + expect( + operatorMergeAuthorizes(callerWritten, { + repositoryId: REPO_A, + pullRequestId: PARENT_PR_A, + currentHeadSha: HEAD_A, + }), + ).toBe(true); + }); + + it('returns true repeatedly for the same record: C1 has no consumed-capability store', () => { + const target = { + repositoryId: REPO_A, + pullRequestId: PARENT_PR_A, + currentHeadSha: HEAD_A, + }; + + expect([ + operatorMergeAuthorizes(authorization, target), + operatorMergeAuthorizes(authorization, target), + operatorMergeAuthorizes(authorization, target), + ]).toEqual([true, true, true]); + }); + it('fails closed on hostile input without throwing', () => { for (const value of NON_OBJECTS) { expect(() => From ffad93ca8b483f4b02fc955daa3255d5ca15f39e Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Sun, 16 Aug 2026 03:51:05 +0200 Subject: [PATCH 04/15] fix: clarify merge target freshness boundary 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) --- docs/architecture/C1-repair-job-authority.md | 56 +++++++++---- src/domain/execution-permit.ts | 78 +++++++++++++------ .../job-authorization-invariants.test.ts | 2 +- 3 files changed, 97 insertions(+), 39 deletions(-) diff --git a/docs/architecture/C1-repair-job-authority.md b/docs/architecture/C1-repair-job-authority.md index d08405f..582402e 100644 --- a/docs/architecture/C1-repair-job-authority.md +++ b/docs/architecture/C1-repair-job-authority.md @@ -269,20 +269,29 @@ because overclaiming here would be worse than not checking. **Proved by a `true` result.** The candidate record carries the required structural fields as readable identifiers; its `singleUse` is literally `true`; and its `repositoryId`, `pullRequestId`, and `headSha` are exactly equal to the -target repository, target pull request, and the repository's *current* HEAD. Every -comparison is exact string equality, so the record is invalid the moment HEAD -changes and can never cover another pull request, another repository, or a future -SHA. There is no path that widens, refreshes, or re-binds it. +corresponding fields of the **supplied** `MergeTarget` — including +`MergeTarget.currentHeadSha`. Every comparison is exact string equality, so a +candidate can never cover a target naming another pull request, another +repository, or a different SHA, and there is no path that widens, refreshes, or +re-binds it. + +`MergeTarget` is caller-supplied input. `operatorMergeAuthorizes` performs no +repository read, no GitHub API call, no adapter call, and no network access, so +**the binding guarantee is only ever as fresh and as authoritative as the target +handed to it.** C1 compares against a value; it does not observe a repository. **Not proved, and not claimed.** | Property | C1 | | --- | --- | +| target SHA is authoritative | **not proved** — `MergeTarget.currentHeadSha` is an input; C1 cannot distinguish a live HEAD from a stale or invented one | +| target SHA is fresh | **not proved** — C1 cannot know whether the repository moved after the target was built | | operator origin | **not proved** — the argument is untrusted data; a plain object literal written by any caller passes | | human identity / authentication | **not proved** — `operatorId` is a readable string; C1 reads no credential and authenticates nothing | | trusted minting, signature, possession | **not proved** — C1 has no issuing boundary and no secret material, so it cannot distinguish a minted record from an assembled one | +| a changed SHA means a new operator decision | **not proved** — see below | | uniqueness / one-time consumption | **not proved** — C1 has no consumed-capability store | -| replay prevention | **not proved** — the identical record returns `true` on every call while HEAD is unchanged | +| replay prevention | **not proved** — the identical record returns `true` on every call while the same target is supplied | `singleUse: true` is a **structural intent marker**: it records that the shape is that of a single-use capability. It is *not* enforced single consumption, and @@ -290,14 +299,35 @@ this document does not claim the record is replay-proof or that it cannot be reused. Likewise, a non-empty `operatorId` is descriptive data; it does not make the record operator-originated, authenticated, or human-authorized. -**A `true` result is therefore not sufficient proof that a merge is -operator-authorized.** It is a necessary binding check. A future trusted operator -boundary — the merge broker, an explicitly reviewed later Cockpit layer — is -responsible for authenticating the operator, establishing that the record was -minted by that boundary rather than supplied by a caller, and consuming the -record so it cannot authorize a second merge. C1 deliberately implements none of -that: no authentication, no signatures, no token issuance, no secret material, no -replay or consumed-capability store, no operator session, and no endpoint. +On a changed SHA, C1 requires only a **newly matching candidate record**: if the +supplied target SHA changes, the previous candidate stops matching, and some +candidate whose `headSha` equals the newly supplied target SHA would be required. +This document does **not** claim that a new SHA requires a new operator decision. +C1 cannot tell whether such a candidate is a fresh human decision or the same +untrusted caller assembling another literal; the future trusted boundary must +establish that. + +**A `true` result is therefore not sufficient proof that a merge may execute.** +It is a necessary binding check. + +### What the future trusted Merge Broker must do + +An explicitly reviewed later Cockpit layer, not implemented here, must: + +1. authenticate the operator; +2. establish trusted capability minting and origin, so the record cannot have + been assembled by a caller; +3. obtain the authoritative pull-request/repository HEAD immediately before the + merge attempt; +4. require that exact HEAD to match the operator capability; +5. enforce single-use consumption atomically; +6. reject if HEAD changed; +7. perform or request the merge only after all gates still pass. + +C1 performs **none** of those steps and claims none of them: no authentication, +no signatures, no token issuance, no secret material, no repository or live-HEAD +lookup, no replay or consumed-capability store, no operator session, and no +endpoint. **No merge executor and no GitHub merge API call exists in this PR.** diff --git a/src/domain/execution-permit.ts b/src/domain/execution-permit.ts index c04ef30..4abfdb8 100644 --- a/src/domain/execution-permit.ts +++ b/src/domain/execution-permit.ts @@ -266,19 +266,22 @@ export function permitsEqual(candidate: ExecutionPermit, issued: ExecutionPermit * exactly as it reads any other untrusted record. What this layer models is the * *binding* a merge authority must carry: * - * - repository-bound, pull-request-bound, and bound to an exact HEAD SHA + * - repository-bound, pull-request-bound, and bound to one exact HEAD SHA * - carrying the structural `singleUse: true` marker - * - incapable of authorizing another pull request or a future SHA + * - incapable of covering another pull request or a different SHA * * {@link operatorMergeAuthorizes} checks exactly that binding, and nothing more. - * It does **not** establish operator origin, human identity, authentication, - * trusted minting, uniqueness, one-time consumption, or replay prevention. + * It compares the record against a caller-supplied {@link MergeTarget}, so it + * does **not** establish that the target SHA is authoritative or fresh, nor + * operator origin, human identity, authentication, trusted minting, uniqueness, + * one-time consumption, or replay prevention. * * A future trusted operator boundary — the merge broker — owns those properties: * it must authenticate the operator, guarantee that the record was minted by - * that boundary rather than assembled by a caller, and record the record as - * consumed so it cannot authorize a second merge. Until that boundary exists, a - * record of this shape proves nothing about a human. + * that boundary rather than assembled by a caller, obtain the authoritative + * pull-request HEAD immediately before merging and require the record to match + * it, and consume the record so it cannot authorize a second merge. Until that + * boundary exists, a record of this shape proves nothing about a human. */ export interface OperatorMergeAuthorization { /** Caller-minted identity of this one operator decision. */ @@ -293,7 +296,7 @@ export interface OperatorMergeAuthorization { readonly repositoryId: string; /** The one pull request this authorization is valid for. */ readonly pullRequestId: string; - /** The exact HEAD the operator approved. A different HEAD is a different merge. */ + /** The one HEAD SHA this record names. A different SHA is a different merge. */ readonly headSha: string; /** Caller-supplied timestamp. Data; no clock is read here. */ readonly authorizedAt: string; @@ -305,17 +308,30 @@ export interface OperatorMergeAuthorization { readonly singleUse: true; } -/** The exact merge an operator authorization is being checked against. */ +/** + * The merge a candidate authorization is being checked against. + * + * Every field is **caller-supplied input**. C1 reads no repository, no API, and + * no adapter, so it cannot check any of these values against reality. They + * define what the candidate is compared *to*, and nothing more. + */ export interface MergeTarget { readonly repositoryId: string; readonly pullRequestId: string; - /** The repository's HEAD *now*, supplied by a trusted adapter. */ + /** + * The HEAD SHA supplied for this merge target. **C1 does not establish that + * this value is authoritative or current** — it performs no repository, API, + * or adapter observation, so a stale, invented, or caller-constructed SHA is + * indistinguishable from a live one here. The future trusted merge boundary + * must obtain the authoritative repository/pull-request HEAD immediately + * before the merge attempt and supply and enforce that exact value. + */ readonly currentHeadSha: string; } /** - * Is this candidate record *structurally bound* to exactly this merge, right - * now? + * Is this candidate record *structurally bound* to exactly this **supplied** + * merge target? * * Pure, total, and deterministic; never throws. Both arguments are read * defensively, own-only, and exactly once. @@ -325,16 +341,21 @@ export interface MergeTarget { * Only that the candidate carries the required structural fields as readable * identifiers, that its `singleUse` is literally `true`, and that its * `repositoryId`, `pullRequestId`, and `headSha` are exactly equal to the - * target's repository, pull request, and *current* HEAD. Every comparison is - * exact string equality, so a HEAD that moved by one commit invalidates the - * record, and a record naming pull request 41 can never cover pull request 42. - * There is no path that widens, refreshes, or re-binds a record to a newer SHA: - * a new HEAD requires a new operator decision. + * corresponding fields of the supplied {@link MergeTarget} — including + * `target.currentHeadSha`, which is an **input value, not an observation**. + * Every comparison is exact string equality, so a candidate naming pull request + * 41 can never cover a target naming pull request 42, and a candidate whose + * `headSha` differs from the supplied target SHA never matches. There is no path + * that widens, refreshes, or re-binds a candidate to a different SHA. * * ## What a `true` result does not prove * * Stated explicitly, because overclaiming here would be worse than not checking: * + * - **That the target SHA is authoritative or fresh.** C1 fetches nothing and + * observes no repository, so it cannot tell a live HEAD from a stale or + * invented one, and cannot know whether the repository moved after the target + * was built. The binding is only ever as good as the supplied target. * - **Operator origin.** The first argument is untrusted data. A plain object * literal, written by any caller with the right field names, satisfies this * predicate. @@ -343,16 +364,23 @@ export interface MergeTarget { * - **Trusted minting or possession.** There is no signature, no secret, and no * issuing boundary, so this predicate cannot distinguish a record a trusted * boundary minted from one a caller assembled. + * - **That a changed SHA reflects a new operator decision.** If the supplied + * target SHA changes, the previous candidate simply stops matching, and *some* + * candidate whose `headSha` equals the newly supplied target SHA would be + * required. C1 cannot tell whether such a candidate is a fresh human decision + * or the same untrusted caller assembling another literal. * - **Uniqueness, one-time consumption, or replay prevention.** C1 stores * nothing and consumes nothing. The identical record returns `true` on every - * call for as long as HEAD has not moved. `singleUse: true` is a structural - * intent marker, not enforcement. - * - * **A `true` result is therefore not sufficient proof that a merge is - * operator-authorized.** It is a necessary binding check that a future trusted - * operator boundary / merge broker must run *in addition to* authenticating the - * operator, verifying that it minted the record itself, and recording the record - * as consumed. Those properties belong to that later, explicitly reviewed layer. + * call for as long as the same target is supplied. `singleUse: true` is a + * structural intent marker, not enforcement. + * + * **A `true` result is therefore not sufficient proof that a merge may + * execute.** It is a necessary binding check that a future trusted operator + * boundary / merge broker must run *in addition to* authenticating the operator, + * verifying that it minted the record itself, obtaining the authoritative + * pull-request HEAD at merge time and requiring the candidate to match that + * value, and consuming the record atomically. Those properties belong to that + * later, explicitly reviewed layer. * * C1 executes no merge. This predicate exists so that the binding half of the * merge barrier is defined by something more precise than a comment. diff --git a/tests/domain/job-authorization-invariants.test.ts b/tests/domain/job-authorization-invariants.test.ts index 3d501d7..24f2a16 100644 --- a/tests/domain/job-authorization-invariants.test.ts +++ b/tests/domain/job-authorization-invariants.test.ts @@ -235,7 +235,7 @@ describe('operator merge authorization is separate, exact, and structurally sing ).toBe(true); }); - it('becomes invalid the moment HEAD moves', () => { + it('stops matching once a different target SHA is supplied', () => { expect( operatorMergeAuthorizes(authorization, { repositoryId: REPO_A, From 14b4fb3674c6409bc29ca2061221e0a5b2030b7a Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Sun, 16 Aug 2026 06:32:14 +0200 Subject: [PATCH 05/15] fix: reject inherited repair list elements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) --- docs/architecture/C1-repair-job-authority.md | 25 +- src/domain/repair-job.ts | 83 ++++- .../job-authorization-invariants.test.ts | 317 +++++++++++++++++- 3 files changed, 416 insertions(+), 9 deletions(-) diff --git a/docs/architecture/C1-repair-job-authority.md b/docs/architecture/C1-repair-job-authority.md index 582402e..a83c7b1 100644 --- a/docs/architecture/C1-repair-job-authority.md +++ b/docs/architecture/C1-repair-job-authority.md @@ -522,8 +522,29 @@ patterns PR 004, PR 005, and PR 006 established: real object and a cut branch name can name a different ref. **C1 truncates nothing at all, because C1 has no prose field.** An oversized list is likewise rejected, not shortened. -- **Sparse arrays reject.** A hole reads as `undefined`, which no reader accepts, - so sparseness rejects rather than collapsing. +- **List entries are own elements.** Every entry of an authorization list is + read as an **own** indexed property, so only an element the supplied object + reports as its own can become an authorized path or command class. For any + array whose own-property introspection is truthful — every ordinary array, + however its prototype chain is arranged — that is exactly the elements the job + configuration supplied: an index with no own element is a sparse hole, and a + hole rejects the whole list rather than collapsing, shortening, or taking a + default, so an inherited numeric property planted on a custom array prototype + or on `Array.prototype` is refused however well-formed its value looks. + Provenance decides, not shape. A hole is not authorization, and prototype + state is not authorization. + + The guarantee stops where the runtime's own-property report does, and the + boundary is documented rather than papered over. A Proxy *defines* the + observable result of `Object.hasOwn` and of the subsequent read, so one whose + `getOwnPropertyDescriptor` trap claims a hole is own while the read forwards + through the target's prototype will pass an inherited value through. The + reader performs one own check and one guarded read and re-validates nothing + afterwards, but those are two observations rather than one atomic one, and a + Proxy may answer them inconsistently. C1 establishes provenance no further + than the supplied object's own report, and claims no more. This widens no + authority: a caller able to supply such a Proxy can supply the same value as a + dense own element instead, which is configuration, not an attack. - **Array building avoids the prototype.** Appends define an own indexed property rather than using `push` or indexed assignment, so an inherited index setter is not on the path. diff --git a/src/domain/repair-job.ts b/src/domain/repair-job.ts index af9715c..4a39216 100644 --- a/src/domain/repair-job.ts +++ b/src/domain/repair-job.ts @@ -106,6 +106,70 @@ export function readOwnProperty(target: object, key: string): unknown { } } +/** + * Absence marker for {@link readOwnElement}. + * + * A bare object literal, so producing it calls no global function: this module + * captures every intrinsic it relies on at load, and a sentinel is not a reason + * to add a fresh dependency on a mutable global. It is used only through + * reference identity, never inspected, and never frozen because nothing reads + * it. + * + * The claim is exactly this and no more: **the reference is module-private.** + * It is not exported and no entry point returns it, so it is not among the + * values a caller ordinarily has to hand. That is why `undefined` was not used + * instead — `undefined` is also a legitimate, and rejected, element *value*, + * and an authorization list must refuse a missing element on its own rather + * than depend on the element reader to refuse whatever turned up. + */ +const NO_OWN_ELEMENT = {}; + +/** + * Read one **own** indexed element of an untrusted array. + * + * The same own-only discipline as {@link readOwnProperty}, through the same + * captured `Object.hasOwn`, except that absence is reported as + * {@link NO_OWN_ELEMENT} instead of collapsing into `undefined`. + * + * An ordinary `elements[index]` walks the prototype chain, so at a sparse hole + * it resolves whatever a custom array prototype — or `Array.prototype` itself — + * carries at that numeric key. Provenance is what decides authority here, not + * the value's shape: an inherited entry can be a perfectly well-formed + * repository path or a genuine command class, and a value nobody put in the + * operator's array is still not authorization. + * + * **What this proves, exactly:** for any array whose own-property introspection + * is truthful — every ordinary array, however its prototype chain is arranged — + * a sparse hole and an inherited numeric property are both refused, because + * `Object.hasOwn` answers `false` and the index is never read at all. + * + * **What it does not prove**, and must not be claimed to: that a value which + * survives came from a real own element. A Proxy *defines* the observable + * result of both operations, so one whose `getOwnPropertyDescriptor` trap + * claims an index is own while the read forwards through the target's + * prototype will pass an inherited value through. There is one own check and + * one read, and nothing re-validates afterwards — but they are two separate + * observations, not one atomic one, and a Proxy may answer them + * inconsistently. C1 takes an object's own-property report at face value and + * establishes provenance no further than that report. This grants no authority + * a caller did not already have: anyone able to supply such a Proxy can supply + * the same value as a dense own element instead, which is not an attack but a + * configuration. + * + * Both operations are guarded, because a getter or a Proxy trap may throw. + * Either way the answer is absence, never an exception. + */ +function readOwnElement(elements: object, index: number): unknown { + try { + if (!objectHasOwn(elements, index)) { + return NO_OWN_ELEMENT; + } + return (elements as Record)[index]; + } catch { + return NO_OWN_ELEMENT; + } +} + /** V1 bounds. Every unbounded dimension is capped before iteration. */ export const JOB_BOUNDS = objectFreeze({ /** @@ -331,8 +395,15 @@ export function readRepositoryRelativePath(value: unknown): string | null { * wrote, and silently keeping a prefix of a list an operator got wrong is not * an improvement on refusing it. * - * A sparse array yields `undefined` at the holes, which no reader accepts, so - * sparseness rejects rather than collapsing. + * Every entry is obtained through {@link readOwnElement}, so a list entry can + * only ever be one the supplied object reports as its **own**. For an ordinary + * array that is exactly the elements the job configuration actually supplied: + * an index with no own element is a sparse hole, and a hole rejects the whole + * list — it is never skipped, defaulted, or filled from the prototype chain — + * so an inherited numeric property planted on a custom array prototype or on + * `Array.prototype` is refused however well-formed its value looks. A Proxy + * that misreports ownership is the documented limit of that guarantee, and it + * widens nothing; see {@link readOwnElement}. */ function readList( value: unknown, @@ -367,10 +438,10 @@ function readList( const parsed: T[] = []; for (let index = 0; index < rawLength; index += 1) { - let element: unknown; - try { - element = elements[index]; - } catch { + const element = readOwnElement(elements, index); + if (element === NO_OWN_ELEMENT) { + // A sparse hole, or an own-check or read that threw. Either way this + // index carries no own element, so the whole list is refused. return null; } const value_ = read(element); diff --git a/tests/domain/job-authorization-invariants.test.ts b/tests/domain/job-authorization-invariants.test.ts index 24f2a16..60e2ce1 100644 --- a/tests/domain/job-authorization-invariants.test.ts +++ b/tests/domain/job-authorization-invariants.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { APPROVAL_STATE, @@ -18,11 +18,14 @@ import { resolveJobOperation, satisfiesIndependentValidator, UNKNOWN_JOB_OPERATION, + findInvalidRepairJobFields, + readRepairJobAuthorization, type ApprovalRecord, type JobOperationRequest, type OperatorMergeAuthorization, type RepairJobAuthorization, type ValidatorClaim, + type VerificationCommandClass, } from '../../src/domain/index.js'; import { AUTHORIZED_PATH, @@ -898,6 +901,318 @@ describe('poisoning Map.prototype.get cannot re-resolve an operation', () => { }); }); +/** + * Build a length-2 array holding `own` at index 0 and a genuine **hole** at 1. + * + * `Array.isArray` still answers `true` and `length` is still 2, so every bound + * and shape check upstream is satisfied; the only thing wrong with index 1 is + * that nothing ever put an own element there. + */ +function withHoleAtOne(own: string): string[] { + const sparse: string[] = []; + sparse[0] = own; + sparse.length = 2; + return sparse; +} + +/** The same hole, with `victim` reachable at index 1 through a custom prototype. */ +function sparseWithInheritedElement(own: string, victim: string): string[] { + const sparse = withHoleAtOne(own); + Object.setPrototypeOf(sparse, { 1: victim }); + return sparse; +} + +/** Run `body` with `victim` planted at `Array.prototype[1]`, then restore. */ +function withArrayPrototypeElement(victim: string, body: () => T): T { + const saved = Object.getOwnPropertyDescriptor(Array.prototype, 1); + Object.defineProperty(Array.prototype, 1, { + value: victim, + writable: true, + configurable: true, + enumerable: false, + }); + try { + return body(); + } finally { + if (saved === undefined) { + Reflect.deleteProperty(Array.prototype, 1); + } else { + Object.defineProperty(Array.prototype, 1, saved); + } + } +} + +describe('an inherited numeric property cannot become an authorization entry', () => { + it('proves the custom-prototype setup is actually active', () => { + // Without this the section could pass vacuously, on an array that was never + // hostile: the hole must be a hole, and the inherited value must be there. + const sparse = sparseWithInheritedElement(AUTHORIZED_PATH, UNAUTHORIZED_PATH); + + expect(Array.isArray(sparse)).toBe(true); + expect(sparse.length).toBe(2); + expect(Object.hasOwn(sparse, 1)).toBe(false); + // An ordinary indexed read — the thing the reader must not do — resolves it. + expect(sparse[1]).toBe(UNAUTHORIZED_PATH); + }); + + it('rejects an authorized-path list whose hole is filled by a custom prototype', () => { + const job = buildJob({ + authorizedPaths: sparseWithInheritedElement(AUTHORIZED_PATH, UNAUTHORIZED_PATH), + }); + + expect(readRepairJobAuthorization(job).snapshot).toBeNull(); + expect(findInvalidRepairJobFields(job)).toContain('authorizedPaths'); + }); + + it('does not let a custom-prototype path reach ALLOW_ONCE or a permit', () => { + const job = buildJob({ + authorizedPaths: sparseWithInheritedElement(AUTHORIZED_PATH, UNAUTHORIZED_PATH), + }); + + const decision = authorizeJobOperation(job, buildEdit({ path: UNAUTHORIZED_PATH })); + + expect(decision.decision).toBe(JOB_AUTHORIZATION.DENY); + expect(decision.reason).toBe(JOB_AUTHORIZATION_REASON.JOB_ENVELOPE_INVALID); + expect(decision.mayExecuteOnce).toBe(false); + expect(decision.permit).toBeNull(); + // The fabricated path is nowhere in the answer, not even as an echo. + expect(JSON.stringify(decision)).not.toContain(UNAUTHORIZED_PATH); + }); + + it('proves the Array.prototype pollution is actually active', () => { + const observed = withArrayPrototypeElement(UNAUTHORIZED_PATH, () => { + const sparse = withHoleAtOne(AUTHORIZED_PATH); + return { own: Object.hasOwn(sparse, 1), read: sparse[1], length: sparse.length }; + }); + + expect(observed.own).toBe(false); + expect(observed.read).toBe(UNAUTHORIZED_PATH); + expect(observed.length).toBe(2); + }); + + it('rejects an authorized-path list whose hole is filled by Array.prototype', () => { + const outcome = withArrayPrototypeElement(UNAUTHORIZED_PATH, () => { + const job = buildJob({ authorizedPaths: withHoleAtOne(AUTHORIZED_PATH) }); + + return { + snapshot: readRepairJobAuthorization(job).snapshot, + invalidFields: findInvalidRepairJobFields(job), + decision: authorizeJobOperation(job, buildEdit({ path: UNAUTHORIZED_PATH })), + }; + }); + + expect(outcome.snapshot).toBeNull(); + expect(outcome.invalidFields).toContain('authorizedPaths'); + expect(outcome.decision.decision).toBe(JOB_AUTHORIZATION.DENY); + expect(outcome.decision.reason).toBe(JOB_AUTHORIZATION_REASON.JOB_ENVELOPE_INVALID); + expect(outcome.decision.mayExecuteOnce).toBe(false); + expect(outcome.decision.permit).toBeNull(); + }); + + it('restores Array.prototype afterwards', () => { + withArrayPrototypeElement(UNAUTHORIZED_PATH, () => undefined); + + expect(Object.hasOwn(Array.prototype, 1)).toBe(false); + expect(withHoleAtOne(AUTHORIZED_PATH)[1]).toBeUndefined(); + }); + + it('does not let an inherited command class enter the trusted snapshot', () => { + // `test` is a genuine, well-formed verification class. Shape is not the + // question; nobody put it in this job's list. + const sparse = sparseWithInheritedElement('lint', 'test'); + const job = buildJob({ + authorizedCommandClasses: sparse as unknown as readonly VerificationCommandClass[], + }); + + expect(Object.hasOwn(sparse, 1)).toBe(false); + expect(readRepairJobAuthorization(job).snapshot).toBeNull(); + expect(findInvalidRepairJobFields(job)).toContain('authorizedCommandClasses'); + }); + + it('does not let an inherited command class gain command-class authority', () => { + const job = buildJob({ + authorizedCommandClasses: sparseWithInheritedElement( + 'lint', + 'test', + ) as unknown as readonly VerificationCommandClass[], + }); + + const decision = authorizeJobOperation( + job, + buildRequest({ + operation: 'verification.run', + worktreeId: REPAIR_WORKTREE, + commandClass: 'test', + }), + ); + + expect(decision.decision).toBe(JOB_AUTHORIZATION.DENY); + expect(decision.mayExecuteOnce).toBe(false); + expect(decision.permit).toBeNull(); + }); + + it('still accepts dense own lists, and still reaches ALLOW_ONCE inside them', () => { + const job = buildJob({ + authorizedPaths: [AUTHORIZED_PATH, UNAUTHORIZED_PATH], + authorizedCommandClasses: ['lint', 'test'], + }); + + expect(findInvalidRepairJobFields(job)).toEqual([]); + expect(readRepairJobAuthorization(job).snapshot?.authorizedPaths).toEqual([ + AUTHORIZED_PATH, + UNAUTHORIZED_PATH, + ]); + + // The same path that had to be refused when it was merely inherited is + // authorized the moment an operator actually puts it in the list. + const edit = authorizeJobOperation(job, buildEdit({ path: UNAUTHORIZED_PATH })); + expect(edit.decision).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + expect(edit.reason).toBe(JOB_AUTHORIZATION_REASON.WITHIN_JOB_ENVELOPE); + expect(edit.mayExecuteOnce).toBe(true); + expect(edit.permit).not.toBeNull(); + + const verify = authorizeJobOperation( + job, + buildRequest({ + operation: 'verification.run', + worktreeId: REPAIR_WORKTREE, + commandClass: 'test', + }), + ); + expect(verify.decision).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + expect(verify.mayExecuteOnce).toBe(true); + }); + + it('keeps a plain sparse hole failing closed with nothing inherited at all', () => { + const job = buildJob({ authorizedPaths: withHoleAtOne(AUTHORIZED_PATH) }); + + expect(findInvalidRepairJobFields(job)).toContain('authorizedPaths'); + expect(authorizeJobOperation(job, buildEdit()).mayExecuteOnce).toBe(false); + }); + + it('rejects rather than throws when the own check itself is hostile', () => { + // A Proxy whose `getOwnPropertyDescriptor` trap throws: the own-only read + // must fail closed, and the entry point must stay total. + const hostile = new Proxy([AUTHORIZED_PATH, AUTHORIZED_PATH], { + getOwnPropertyDescriptor(): PropertyDescriptor { + throw new Error('hostile own-property trap'); + }, + }); + const job = buildJob({ authorizedPaths: hostile }); + + expect(() => findInvalidRepairJobFields(job)).not.toThrow(); + expect(findInvalidRepairJobFields(job)).toContain('authorizedPaths'); + expect(authorizeJobOperation(job, buildEdit()).mayExecuteOnce).toBe(false); + }); + + it('rejects rather than throws when an element getter is hostile', () => { + const hostile: string[] = [AUTHORIZED_PATH, AUTHORIZED_PATH]; + Object.defineProperty(hostile, 1, { + get(): string { + throw new Error('hostile element getter'); + }, + configurable: true, + enumerable: true, + }); + const job = buildJob({ authorizedPaths: hostile }); + + expect(() => authorizeJobOperation(job, buildEdit())).not.toThrow(); + expect(findInvalidRepairJobFields(job)).toContain('authorizedPaths'); + expect(authorizeJobOperation(job, buildEdit()).mayExecuteOnce).toBe(false); + }); + + it('pins the documented limit: a Proxy that misreports ownership widens nothing', () => { + // The honest boundary, recorded so the guarantee is not read as stronger + // than it is. A Proxy *defines* the observable result of `Object.hasOwn` + // and of the read, so one that claims a hole is own while the read forwards + // through the target's prototype passes the inherited value through. No + // reader can tell it apart from a truthful object, and this test does not + // pretend otherwise — it pins the *consequence*, which is that nothing is + // widened. + const target = withHoleAtOne(AUTHORIZED_PATH); + Object.setPrototypeOf(target, { 1: UNAUTHORIZED_PATH }); + const liar = new Proxy(target, { + getOwnPropertyDescriptor(t, key): PropertyDescriptor | undefined { + if (key === '1') { + return { value: UNAUTHORIZED_PATH, writable: true, enumerable: true, configurable: true }; + } + return Reflect.getOwnPropertyDescriptor(t, key); + }, + }); + + // The lie is in effect: the object reports the hole as its own. + expect(Object.hasOwn(target, 1)).toBe(false); + expect(Object.hasOwn(liar, 1)).toBe(true); + + const viaProxy = authorizeJobOperation( + buildJob({ authorizedPaths: liar }), + buildEdit({ path: UNAUTHORIZED_PATH }), + ); + // The same caller, supplying the same value as a plain dense own element — + // which needs no Proxy and no lie at all. + const viaDenseArray = authorizeJobOperation( + buildJob({ authorizedPaths: [AUTHORIZED_PATH, UNAUTHORIZED_PATH] }), + buildEdit({ path: UNAUTHORIZED_PATH }), + ); + + // Byte-identical: the Proxy reaches exactly what configuration already + // reaches, so it is not an escalation, and the documentation says so. + expect(JSON.stringify(viaProxy)).toBe(JSON.stringify(viaDenseArray)); + }); + + it('needs no global Symbol call to evaluate, so the sentinel depends on no mutable global', async () => { + // The absence marker is a bare object literal. A module that built it with + // `Symbol(...)` would call the global factory during evaluation; this + // module must not, so a replaced `Symbol` is not on its load path at all. + const saved = globalThis.Symbol; + let calls = 0; + // A Proxy over the real Symbol keeps every own property — `Symbol.iterator` + // and friends — so the module loader itself is unaffected. + const counting = new Proxy(saved, { + apply(target, thisArg, args: readonly unknown[]): unknown { + calls += 1; + return Reflect.apply(target as (...a: readonly unknown[]) => unknown, thisArg, args); + }, + }); + + let fresh: typeof import('../../src/domain/repair-job.js'); + try { + globalThis.Symbol = counting; + vi.resetModules(); + // Awaited inside the try, so the module actually evaluates while the + // counting Symbol is installed. + fresh = await import('../../src/domain/repair-job.js'); + } finally { + globalThis.Symbol = saved; + } + + expect(calls).toBe(0); + // And the freshly evaluated module still behaves. + expect(fresh.findInvalidRepairJobFields(buildJob())).toEqual([]); + expect( + fresh.findInvalidRepairJobFields(buildJob({ authorizedPaths: withHoleAtOne(AUTHORIZED_PATH) })), + ).toContain('authorizedPaths'); + }); + + it('leaves the merge and auto-merge barriers exactly where they were', () => { + // The A03 repair touches list-element provenance and nothing else. + const job = buildJob({ + authorizedPaths: sparseWithInheritedElement(AUTHORIZED_PATH, UNAUTHORIZED_PATH), + }); + + const merge = authorizeJobOperation(job, buildRequest({ operation: 'merge' })); + expect(merge.decision).toBe(JOB_AUTHORIZATION.OPERATOR_REQUIRED); + expect(merge.reason).toBe(JOB_AUTHORIZATION_REASON.MERGE_IS_OPERATOR_ONLY); + expect(merge.mayExecuteOnce).toBe(false); + expect(merge.permit).toBeNull(); + + const auto = authorizeJobOperation(job, buildRequest({ operation: 'auto_merge.enable' })); + expect(auto.decision).toBe(JOB_AUTHORIZATION.DENY); + expect(auto.reason).toBe(JOB_AUTHORIZATION_REASON.OPERATION_FORBIDDEN); + expect(auto.permit).toBeNull(); + }); +}); + /* ------------------------------------------------------------------------- * Cross-boundary conventions * ------------------------------------------------------------------------- */ From 52f1a79a98520d5a3dc2eb2ce8a662ea36cc6a56 Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Sun, 16 Aug 2026 17:29:17 +0200 Subject: [PATCH 06/15] fix: reject Git-equivalent repair refs --- docs/architecture/C1-repair-job-authority.md | 72 +++- src/domain/index.ts | 1 + src/domain/job-authorization.ts | 18 + src/domain/job-operation.ts | 42 ++- src/domain/repair-job.ts | 248 ++++++++++++- tests/domain/execution-permit.test.ts | 4 +- .../job-authorization-invariants.test.ts | 350 +++++++++++++++++- tests/domain/job-authorization.test.ts | 9 +- tests/domain/repair-job-fixtures.ts | 21 +- 9 files changed, 733 insertions(+), 32 deletions(-) diff --git a/docs/architecture/C1-repair-job-authority.md b/docs/architecture/C1-repair-job-authority.md index a83c7b1..db504d1 100644 --- a/docs/architecture/C1-repair-job-authority.md +++ b/docs/architecture/C1-repair-job-authority.md @@ -101,12 +101,12 @@ care. | `policyVersion` | which policy authorized the envelope; part of permit identity | | `repositoryId` | the one repository this job may ever touch | | `parentPullRequestId` | the protected parent feature pull request | -| `protectedParentRef` | the parent integration ref, which no operation may write | +| `protectedParentRef` | the parent integration ref, which no operation may write; canonical `refs/heads/` | | `parentHeadSha` | the exact commit the job is bound to | | `findingSource` | where the finding came from; a provider-neutral label, inert | | `findingId` | the finding being repaired | | `findingHeadSha` | the commit the finding was verified against | -| `repairBranch` | the isolated repair branch | +| `repairBranch` | the isolated repair branch; canonical `refs/heads/` | | `repairWorktreeId` | the isolated repair worktree | | `authorizedPaths` | exact repository-relative paths, not a prefix or glob | | `authorizedCommandClasses` | verification classes, never command strings | @@ -121,12 +121,58 @@ offending names in declaration order. Two structural invariants are enforced as configuration validity rather than as a runtime check that could be forgotten: -- `repairBranch !== protectedParentRef`. A job whose repair branch *is* the - protected parent is not a quarantined repair; it is a direct write to - protected history wearing a repair job's name. +- `repairBranch` and `protectedParentRef` denote **different branches**. A job + whose repair branch *is* the protected parent is not a quarantined repair; it + is a direct write to protected history wearing a repair job's name. - `independentValidatorId !== repairAgentId`. A repair agent that is its own validator defeats the quarantine the whole pipeline exists to enforce. +### Branch refs have exactly one accepted spelling + +"Different branches", not "different strings". Git resolves `main`, +`heads/main`, and `refs/heads/main` to one and the same ref, so a boundary that +compares ref strings has three names for one authority target. Configuring +`protectedParentRef: 'refs/heads/main'` beside `repairBranch: 'main'` would +otherwise read as a quarantined repair, and a `repair.push` naming `main` would +pass every check and produce an `ExecutionPermit` whose ref denotes the protected +branch. + +C1 cannot ask git which ref a shorthand resolves to — it runs no git, spawns no +subprocess, opens no file, and observes no repository, and the answer depends on +what exists in a repository at the moment the name is used. So C1 does not +resolve; it **narrows**. `readCanonicalBranchRef` accepts exactly one spelling +and refuses every other spelling of the same branch as malformed: + +- the literal, case-sensitive prefix `refs/heads/`, followed by a non-empty name +- name segments separated by single `/`, each non-empty +- segment characters drawn only from `A-Z`, `a-z`, `0-9`, `-`, `_`, and `.` +- no segment beginning or ending with `.`, no `..` anywhere, and no segment + ending in `.lock` in any ASCII case + +The property that buys: **two accepted refs denote the same branch if and only if +they are equal strings.** That is what makes the distinctness invariant mean +something. The conservative ASCII character set is part of the guarantee, not a +convenience — it removes Unicode normalisation, under which an NFC and an NFD +spelling of one name are unequal strings a filesystem-backed loose ref can +resolve to a single ref, and it removes `~`, `^`, `:`, `?`, `*`, `[`, `\`, `@{`, +and whitespace in one rule. Nothing is normalised, prefixed, or case-folded on +the way in: a value is accepted exactly as supplied or refused. + +The same reader is applied to **every** security-relevant ref position — the two +job fields, and the `ref`, `sourceRef`, and `targetRef` request operands — so +validation can never compare a canonical configured value against an +uncanonical request operand. A supplied operand that is not canonical is refused +`REF_MALFORMED` before any comparison, rather than compared as though it were a +different branch. + +**What this does not prove**, and must not be claimed to: that two unequal +accepted refs are two distinct refs on every filesystem. Git stores loose refs as +files, so on a case-insensitive filesystem `refs/heads/Main` and +`refs/heads/main` can be one ref while comparing unequal. C1 observes no +filesystem, so it refuses the ambiguous case instead of pretending it away: the +job's two configured refs are additionally compared with ASCII case folded, and a +pair that differs only by case is rejected as malformed configuration. + One relationship is enforced at authorization time, because it is about freshness rather than shape: `findingHeadSha` must equal `parentHeadSha`, or every operation is denied `FINDING_SHA_STALE`. A repair derived from a finding @@ -150,6 +196,10 @@ authority cannot be checked against an exact operand has no place in the model. | `repair.push` | ref, non-force | ref is exactly the repair branch and the push is not forced | | `repair.change_request` | source ref, target ref | repair branch → protected parent ref | +Every ref operand is read through the same canonical branch-ref reader the job +envelope uses, so "exactly the repair branch" is a claim about a branch and not +about a spelling. + `repair.change_request` is the **only** operation that may name the protected parent ref, and only as a change-request *target*. Opening a change request against a ref does not mutate it: the parent stays untouched until an operator @@ -188,7 +238,7 @@ Every refusal carries a stable, machine-readable reason: `OPERATION_UNREADABLE`, `JOB_ENVELOPE_INVALID`, `JOB_MISMATCH`, `REPOSITORY_MISMATCH`, `PARENT_PULL_REQUEST_MISMATCH`, `PARENT_HEAD_MISMATCH`, `FINDING_SHA_STALE`, `OPERAND_MISSING`, `PATH_MALFORMED`, `PATH_NOT_AUTHORIZED`, -`WORKTREE_NOT_AUTHORIZED`, `COMMAND_CLASS_NOT_AUTHORIZED`, +`WORKTREE_NOT_AUTHORIZED`, `COMMAND_CLASS_NOT_AUTHORIZED`, `REF_MALFORMED`, `PROTECTED_REF_MUTATION`, `REF_NOT_REPAIR_BRANCH`, `CHANGE_REQUEST_TARGET_INVALID`, `FORCE_PUSH_FORBIDDEN`. @@ -474,7 +524,9 @@ The mandatory AgentBridge pattern is preserved: C1 implements none of that workflow. It encodes only the minimal authority invariants that stop a later layer from bypassing the quarantine by accident: -- The protected parent ref is never a write target of any operation. +- The protected parent ref is never a write target of any operation, under any + spelling: refs are canonical everywhere, so an alias of the parent cannot be + presented as a different branch. - Filesystem-shaped operations are bound to the repair worktree, so an edit cannot land in the parent's checkout. - The stacked change request must run from the repair branch to the protected @@ -522,6 +574,12 @@ patterns PR 004, PR 005, and PR 006 established: real object and a cut branch name can name a different ref. **C1 truncates nothing at all, because C1 has no prose field.** An oversized list is likewise rejected, not shortened. +- **Refs are narrowed, never repaired.** A non-canonical branch ref is refused, + not rewritten into the canonical spelling. Repairing a spelling would be + choosing an authority target on the caller's behalf, which is exactly the + decision the boundary exists to refuse. The reader is a pure function of a + primitive string captured by a single own-property read, so it introduces no + second observation of untrusted state and no validation TOCTOU. - **List entries are own elements.** Every entry of an authorization list is read as an **own** indexed property, so only an element the supplied object reports as its own can become an authorized path or command class. For any diff --git a/src/domain/index.ts b/src/domain/index.ts index 394d912..aaf8a35 100644 --- a/src/domain/index.ts +++ b/src/domain/index.ts @@ -142,6 +142,7 @@ export { findInvalidRepairJobFields, isVerificationCommandClass, JOB_BOUNDS, + readCanonicalBranchRef, readRepairJobAuthorization, readRepositoryRelativePath, REPAIR_JOB_FIELD_ORDER, diff --git a/src/domain/job-authorization.ts b/src/domain/job-authorization.ts index f0f9a60..dcb7e57 100644 --- a/src/domain/job-authorization.ts +++ b/src/domain/job-authorization.ts @@ -136,6 +136,8 @@ export const JOB_AUTHORIZATION_REASON = objectFreeze({ WORKTREE_NOT_AUTHORIZED: 'WORKTREE_NOT_AUTHORIZED', /** The verification class is unmodeled or not in the job's authorized set. */ COMMAND_CLASS_NOT_AUTHORIZED: 'COMMAND_CLASS_NOT_AUTHORIZED', + /** A ref operand was supplied but is not a canonical `refs/heads/` ref. */ + REF_MALFORMED: 'REF_MALFORMED', /** The operation names the protected parent ref as a write target. */ PROTECTED_REF_MUTATION: 'PROTECTED_REF_MUTATION', /** The ref operand is not the job's isolated repair branch. */ @@ -264,6 +266,12 @@ function checkOperands( if (request.worktreeId !== job.repairWorktreeId) { return JOB_AUTHORIZATION_REASON.WORKTREE_NOT_AUTHORIZED; } + // A non-canonical spelling is refused for being unusable, before any + // comparison: `main` and `heads/main` may both denote `refs/heads/main`, + // so comparing either as a distinct string is exactly the bypass. + if (request.refMalformed) { + return JOB_AUTHORIZATION_REASON.REF_MALFORMED; + } if (request.ref === null) { return JOB_AUTHORIZATION_REASON.OPERAND_MISSING; } @@ -281,6 +289,9 @@ function checkOperands( if (request.force) { return JOB_AUTHORIZATION_REASON.FORCE_PUSH_FORBIDDEN; } + if (request.refMalformed) { + return JOB_AUTHORIZATION_REASON.REF_MALFORMED; + } if (request.ref === null) { return JOB_AUTHORIZATION_REASON.OPERAND_MISSING; } @@ -293,6 +304,13 @@ function checkOperands( return null; } case JOB_OPERATION.REPAIR_CHANGE_REQUEST: { + // Both ends are narrowed to the canonical spelling before either is + // compared, so source/target separation is separation of branches rather + // than of strings: an alias of the protected parent cannot be presented + // as the source, and an alias of the repair branch cannot be the target. + if (request.sourceRefMalformed || request.targetRefMalformed) { + return JOB_AUTHORIZATION_REASON.REF_MALFORMED; + } if (request.sourceRef === null || request.targetRef === null) { return JOB_AUTHORIZATION_REASON.OPERAND_MISSING; } diff --git a/src/domain/job-operation.ts b/src/domain/job-operation.ts index ccb6144..abe5b5c 100644 --- a/src/domain/job-operation.ts +++ b/src/domain/job-operation.ts @@ -29,6 +29,7 @@ import { append, containsValue, + readCanonicalBranchRef, readExactIdentifier, readOwnProperty, readRepositoryRelativePath, @@ -246,11 +247,17 @@ export interface JobOperationRequest { readonly path?: string; /** Verification class operand, for `verification.run`. */ readonly commandClass?: string; - /** Ref operand, for `repair.commit` and `repair.push`. */ + /** + * Ref operand, for `repair.commit` and `repair.push`. + * + * Read through the same canonical branch-ref reader the job envelope uses, so + * an alternate spelling of a configured ref cannot be compared against it as + * if it were a different branch. + */ readonly ref?: string; - /** Change-request source ref operand. */ + /** Change-request source ref operand. Canonical branch ref. */ readonly sourceRef?: string; - /** Change-request target ref operand. */ + /** Change-request target ref operand. Canonical branch ref. */ readonly targetRef?: string; /** Force flag for a push. Anything that is not exactly absent or `false` is force. */ readonly force?: boolean; @@ -279,8 +286,14 @@ export interface NormalizedJobOperation { readonly pathMalformed: boolean; readonly commandClass: string | null; readonly ref: string | null; + /** True when a `ref` was supplied but is not a canonical branch ref. */ + readonly refMalformed: boolean; readonly sourceRef: string | null; + /** True when a `sourceRef` was supplied but is not a canonical branch ref. */ + readonly sourceRefMalformed: boolean; readonly targetRef: string | null; + /** True when a `targetRef` was supplied but is not a canonical branch ref. */ + readonly targetRefMalformed: boolean; /** Fails closed: only an absent or literally `false` value is not force. */ readonly force: boolean; } @@ -298,8 +311,11 @@ const UNREADABLE_OPERATION: NormalizedJobOperation = objectFreeze({ pathMalformed: false, commandClass: null, ref: null, + refMalformed: false, sourceRef: null, + sourceRefMalformed: false, targetRef: null, + targetRefMalformed: false, force: true, }); @@ -331,6 +347,17 @@ export function readJobOperation(request: JobOperationRequest): NormalizedJobOpe const rawPath = readOwnProperty(record, 'path'); const path = readRepositoryRelativePath(rawPath); + // Each ref operand is read once, own-only, and narrowed to the one canonical + // branch-ref spelling. A supplied value that is not canonical becomes `null` + // and is flagged, so it is refused for being unusable rather than compared — + // as a distinct string — against a canonical value it may in fact alias. + const rawRef = readOwnProperty(record, 'ref'); + const ref = readCanonicalBranchRef(rawRef); + const rawSourceRef = readOwnProperty(record, 'sourceRef'); + const sourceRef = readCanonicalBranchRef(rawSourceRef); + const rawTargetRef = readOwnProperty(record, 'targetRef'); + const targetRef = readCanonicalBranchRef(rawTargetRef); + return objectFreeze({ readable: true, requestId: readExactIdentifier(readOwnProperty(record, 'requestId')), @@ -343,9 +370,12 @@ export function readJobOperation(request: JobOperationRequest): NormalizedJobOpe path, pathMalformed: path === null && rawPath !== undefined, commandClass: readExactIdentifier(readOwnProperty(record, 'commandClass')), - ref: readExactIdentifier(readOwnProperty(record, 'ref')), - sourceRef: readExactIdentifier(readOwnProperty(record, 'sourceRef')), - targetRef: readExactIdentifier(readOwnProperty(record, 'targetRef')), + ref, + refMalformed: ref === null && rawRef !== undefined, + sourceRef, + sourceRefMalformed: sourceRef === null && rawSourceRef !== undefined, + targetRef, + targetRefMalformed: targetRef === null && rawTargetRef !== undefined, force: readForceFlag(readOwnProperty(record, 'force')), }); } diff --git a/src/domain/repair-job.ts b/src/domain/repair-job.ts index 4a39216..a0c03fe 100644 --- a/src/domain/repair-job.ts +++ b/src/domain/repair-job.ts @@ -266,6 +266,14 @@ const CODE_DOT = 0x2e; const CODE_TILDE = 0x7e; const CODE_DELETE = 0x7f; const CODE_SPACE = 0x20; +const CODE_HYPHEN = 0x2d; +const CODE_UNDERSCORE = 0x5f; +const CODE_DIGIT_0 = 0x30; +const CODE_DIGIT_9 = 0x39; +const CODE_UPPER_A = 0x41; +const CODE_UPPER_Z = 0x5a; +const CODE_LOWER_A = 0x61; +const CODE_LOWER_Z = 0x7a; /** Is `value[start, end)` the segment `.git`, in any ASCII case? */ function isDotGitSegment(value: string, start: number, end: number): boolean { @@ -386,6 +394,208 @@ export function readRepositoryRelativePath(value: unknown): string | null { return value; } +/** + * The one accepted spelling of a branch ref. + * + * Fully qualified, lower case, and matched literally. Git resolves the shorthand + * `main`, the partially qualified `heads/main`, and the fully qualified + * `refs/heads/main` to one and the same ref, so a boundary that compares ref + * *strings* has three spellings for one authority target unless it fixes the + * spelling first. C1 fixes it here. + */ +const BRANCH_REF_PREFIX = 'refs/heads/'; + +/** Is this the code of a character C1 accepts inside a branch name? */ +function isBranchNameCharacter(code: number): boolean { + return ( + (code >= CODE_LOWER_A && code <= CODE_LOWER_Z) || + (code >= CODE_UPPER_A && code <= CODE_UPPER_Z) || + (code >= CODE_DIGIT_0 && code <= CODE_DIGIT_9) || + code === CODE_HYPHEN || + code === CODE_UNDERSCORE || + code === CODE_DOT + ); +} + +/** Does `value` begin with the literal, case-sensitive {@link BRANCH_REF_PREFIX}? */ +function hasBranchRefPrefix(value: string): boolean { + const prefixLength = BRANCH_REF_PREFIX.length; + if (value.length <= prefixLength) { + return false; + } + for (let index = 0; index < prefixLength; index += 1) { + // An unreadable code reads as -1 and cannot equal a prefix character. + if (charCodeAt(value, index) !== charCodeAt(BRANCH_REF_PREFIX, index)) { + return false; + } + } + return true; +} + +/** Is `value[start, end)` a segment ending in `.lock`, in any ASCII case? */ +function endsWithDotLockSuffix(value: string, start: number, end: number): boolean { + if (end - start < 5) { + return false; + } + if (charCodeAt(value, end - 5) !== CODE_DOT) { + return false; + } + // 0x20 folds ASCII upper case to lower case; only these four positions matter. + const l = charCodeAt(value, end - 4) | 0x20; + const o = charCodeAt(value, end - 3) | 0x20; + const c = charCodeAt(value, end - 2) | 0x20; + const k = charCodeAt(value, end - 1) | 0x20; + return l === 0x6c && o === 0x6f && c === 0x63 && k === 0x6b; +} + +/** + * Read a branch ref in the one canonical spelling C1 accepts. + * + * **Why one spelling, rather than a resolver.** Git's own shorthand rules make + * `main`, `heads/main`, and `refs/heads/main` three names for one branch, and + * the ambiguity is not decidable from the string alone: whether `main` resolves + * to a branch, a tag, or a remote-tracking ref depends on what exists in a + * repository at the moment the name is used. C1 is pure TypeScript by + * construction — it runs no git, spawns no subprocess, opens no file, and + * observes no repository — so it cannot ask which ref a shorthand denotes, and a + * boundary that guesses would be guessing about authority. + * + * So C1 does not resolve; it **narrows**. Exactly one spelling is accepted, and + * every other spelling of the same branch is refused as malformed rather than + * silently treated as a different ref. The property that buys is precise: + * + * > Two accepted refs denote the same branch if and only if they are equal + * > strings — up to the ASCII-case caveat below. + * + * That is what makes `repairBranch !== protectedParentRef` mean "two different + * branches" instead of "two different strings". Before this, a job configured + * with `protectedParentRef: 'refs/heads/main'` and `repairBranch: 'main'` was + * accepted as a quarantined repair, and a `repair.push` naming `main` passed + * every check and produced an `ExecutionPermit` whose ref denotes the protected + * branch. + * + * Accepted, and nothing else: + * + * - a value that survives {@link readExactIdentifier}, so the identifier bound + * applies and nothing is trimmed or truncated + * - the literal, case-sensitive prefix `refs/heads/`, followed by a non-empty + * name; `Refs/Heads/x`, `heads/x`, `x`, `refs/tags/x`, `refs/remotes/…`, and a + * bare `refs/heads/` are all refused + * - name segments separated by single `/`, each non-empty, so `//`, a leading + * `/`, and a trailing `/` are refused + * - segment characters drawn only from `A-Z`, `a-z`, `0-9`, `-`, `_`, and `.` + * - no segment beginning or ending with `.`, no `..` anywhere, and no segment + * ending in `.lock` in any ASCII case — the ref-name forms git itself refuses + * + * The conservative character set is deliberate and is part of the guarantee. + * Restricting names to ASCII removes Unicode normalisation entirely: without it + * an NFC and an NFD spelling of one branch name are unequal strings that a + * filesystem-backed loose ref can resolve to a single ref, which is the same + * aliasing failure in a different alphabet. It also removes `~`, `^`, `:`, `?`, + * `*`, `[`, `\`, `@{`, and whitespace — every character git rejects in a ref + * name, plus the revision-syntax operators that make `x^{}` and `x@{1}` name + * something other than `x`. A branch name outside this set is refused, never + * rewritten. + * + * **What this does not prove**, and must not be claimed to: that two unequal + * accepted refs are two distinct refs *on every filesystem*. Git stores loose + * refs as files, so on a case-insensitive filesystem `refs/heads/Main` and + * `refs/heads/main` can be one ref while comparing unequal here. That residue is + * handled where it matters — {@link mayDenoteSameBranchRef} compares the job's + * two configured refs case-insensitively, so such a pair is refused as + * configuration — rather than pretended away here. C1 observes no filesystem and + * cannot do better than refuse the ambiguous case. + * + * The value is returned exactly as supplied, or not at all. No normalisation, + * no prefixing, no case folding: a boundary that repaired the spelling would be + * choosing an authority target on the caller's behalf. + */ +export function readCanonicalBranchRef(value: unknown): string | null { + const identifier = readExactIdentifier(value); + if (identifier === null) { + return null; + } + if (!hasBranchRefPrefix(identifier)) { + return null; + } + + const length = identifier.length; + let segmentStart = BRANCH_REF_PREFIX.length; + for (let index = segmentStart; index <= length; index += 1) { + const atEnd = index === length; + const code = atEnd ? CODE_SLASH : charCodeAt(identifier, index); + + if (!atEnd && code !== CODE_SLASH) { + // An unreadable code reads as -1, which is not a name character. + if (!isBranchNameCharacter(code)) { + return null; + } + // `..` is a revision-range operator and git refuses it in a ref name. + if (code === CODE_DOT && charCodeAt(identifier, index - 1) === CODE_DOT) { + return null; + } + continue; + } + + const segmentLength = index - segmentStart; + if (segmentLength === 0) { + return null; + } + if (charCodeAt(identifier, segmentStart) === CODE_DOT) { + return null; + } + if (charCodeAt(identifier, index - 1) === CODE_DOT) { + return null; + } + if (endsWithDotLockSuffix(identifier, segmentStart, index)) { + return null; + } + segmentStart = index + 1; + } + + return identifier; +} + +/** + * Could these two accepted branch refs denote the same branch? + * + * Both arguments are already-validated canonical refs, so this is exact string + * equality widened by one conservative allowance: ASCII case. Git stores loose + * refs as files, and on a case-insensitive filesystem `refs/heads/Main` and + * `refs/heads/main` can be the same ref while comparing unequal. C1 observes no + * filesystem and cannot tell which kind it will run on, so it treats such a pair + * as possibly-identical and the job that configures one is refused. + * + * Conservative in the safe direction: it answers `true` — refuse — whenever it + * cannot establish that the two refs are distinct, including for a character it + * could not read at all. Only ASCII case is folded, because the canonical reader + * admits no other alphabet. + */ +function mayDenoteSameBranchRef(left: string, right: string): boolean { + if (left === right) { + return true; + } + if (left.length !== right.length) { + return false; + } + for (let index = 0; index < left.length; index += 1) { + const leftCode = charCodeAt(left, index); + const rightCode = charCodeAt(right, index); + if (leftCode === -1 || rightCode === -1) { + // Unreadable: cannot establish distinctness, so refuse the pair. + return true; + } + const foldedLeft = + leftCode >= CODE_UPPER_A && leftCode <= CODE_UPPER_Z ? leftCode | 0x20 : leftCode; + const foldedRight = + rightCode >= CODE_UPPER_A && rightCode <= CODE_UPPER_Z ? rightCode | 0x20 : rightCode; + if (foldedLeft !== foldedRight) { + return false; + } + } + return true; +} + /** * Read a bounded list of untrusted values, all-or-nothing. * @@ -475,7 +685,8 @@ export interface RepairJobAuthorization { /** The protected parent feature pull request this repair is stacked under. */ readonly parentPullRequestId: string; /** - * The protected parent integration ref. + * The protected parent integration ref, in the canonical `refs/heads/` + * spelling {@link readCanonicalBranchRef} defines. * * **No job operation may ever write to it.** It appears in exactly one * authorizable position: as the *target* of the stacked validation change @@ -497,12 +708,19 @@ export interface RepairJobAuthorization { */ readonly findingHeadSha: string; /** - * The isolated repair branch. + * The isolated repair branch, in the canonical `refs/heads/` spelling + * {@link readCanonicalBranchRef} defines. + * + * Must denote a different branch from {@link protectedParentRef}. A job whose + * repair branch is the protected parent ref is not a quarantined repair; it is + * a direct write to protected history wearing a repair job's name, and it is + * rejected as malformed configuration rather than evaluated. * - * Must differ from {@link protectedParentRef}. A job whose repair branch is - * the protected parent ref is not a quarantined repair; it is a direct write - * to protected history wearing a repair job's name, and it is rejected as - * malformed configuration rather than evaluated. + * "Different branch", not "different string": both refs are read through the + * canonical reader, so an alternate spelling of the protected parent — `main`, + * `heads/main` — cannot pass as an isolated repair branch, and a pair that + * differs only by ASCII case is refused too because a case-insensitive + * filesystem can store the two as one loose ref. */ readonly repairBranch: string; /** The isolated repair worktree. Filesystem-shaped operations are bound to it. */ @@ -616,12 +834,14 @@ export function readRepairJobAuthorization(job: RepairJobAuthorization): RepairJ const policyVersion = readExactIdentifier(readOwnProperty(record, 'policyVersion')); const repositoryId = readExactIdentifier(readOwnProperty(record, 'repositoryId')); const parentPullRequestId = readExactIdentifier(readOwnProperty(record, 'parentPullRequestId')); - const protectedParentRef = readExactIdentifier(readOwnProperty(record, 'protectedParentRef')); + const protectedParentRef = readCanonicalBranchRef( + readOwnProperty(record, 'protectedParentRef'), + ); const parentHeadSha = readExactIdentifier(readOwnProperty(record, 'parentHeadSha')); const findingSource = readExactIdentifier(readOwnProperty(record, 'findingSource')); const findingId = readExactIdentifier(readOwnProperty(record, 'findingId')); const findingHeadSha = readExactIdentifier(readOwnProperty(record, 'findingHeadSha')); - const repairBranch = readExactIdentifier(readOwnProperty(record, 'repairBranch')); + const repairBranch = readCanonicalBranchRef(readOwnProperty(record, 'repairBranch')); const repairWorktreeId = readExactIdentifier(readOwnProperty(record, 'repairWorktreeId')); const authorizedPaths = readList( readOwnProperty(record, 'authorizedPaths'), @@ -666,9 +886,15 @@ export function readRepairJobAuthorization(job: RepairJobAuthorization): RepairJ if (findingHeadSha === null) { append(invalidFields, 'findingHeadSha'); } - // The repair branch must be distinguishable from the protected parent ref, or - // the isolation the whole quarantine depends on does not exist. - if (repairBranch === null || repairBranch === protectedParentRef) { + // The repair branch must be a *different branch* from the protected parent + // ref, or the isolation the whole quarantine depends on does not exist. Both + // refs are canonical here, so unequal strings are different branches — except + // for the ASCII-case pair a case-insensitive filesystem can collapse into one + // loose ref, which `mayDenoteSameBranchRef` refuses as well. + if ( + repairBranch === null || + (protectedParentRef !== null && mayDenoteSameBranchRef(repairBranch, protectedParentRef)) + ) { append(invalidFields, 'repairBranch'); } if (repairWorktreeId === null) { diff --git a/tests/domain/execution-permit.test.ts b/tests/domain/execution-permit.test.ts index 865e77b..fc63a6f 100644 --- a/tests/domain/execution-permit.test.ts +++ b/tests/domain/execution-permit.test.ts @@ -176,9 +176,9 @@ describe('cross-operation replay', () => { it('does not verify for a different ref', () => { const permit = issue(buildJob(), buildPush()); - const job = buildJob({ repairBranch: 'repair/job-0001-b' }); + const job = buildJob({ repairBranch: 'refs/heads/repair/job-0001-b' }); - expect(permitAuthorizes(permit, job, buildPush({ ref: 'repair/job-0001-b' }))).toBe(false); + expect(permitAuthorizes(permit, job, buildPush({ ref: 'refs/heads/repair/job-0001-b' }))).toBe(false); }); it('does not verify for a different verification class', () => { diff --git a/tests/domain/job-authorization-invariants.test.ts b/tests/domain/job-authorization-invariants.test.ts index 60e2ce1..968bdbc 100644 --- a/tests/domain/job-authorization-invariants.test.ts +++ b/tests/domain/job-authorization-invariants.test.ts @@ -13,6 +13,7 @@ import { JOB_BOUNDS, JOB_OPERATION, operatorMergeAuthorizes, + readCanonicalBranchRef, readJobOperation, REPAIR_AUTHORIZABLE_OPERATIONS, resolveJobOperation, @@ -39,6 +40,7 @@ import { NON_OBJECTS, PARENT_PR_A, PARENT_REF, + PARENT_REF_ALIASES, PRIVILEGED_LABELS, REPAIR_BRANCH, REPAIR_WORKTREE, @@ -86,8 +88,8 @@ describe('merge is operator-only, permanently', () => { buildJob({ repairAgentId: 'root', independentValidatorId: 'system' }), buildJob({ findingSource: 'agentbridge-internal' }), buildJob({ authorizedPaths: [], authorizedCommandClasses: [] }), - buildJob({ protectedParentRef: 'main' }), - buildJob({ repairBranch: 'main', protectedParentRef: 'main' }), + buildJob({ protectedParentRef: 'refs/heads/main' }), + buildJob({ repairBranch: 'refs/heads/main', protectedParentRef: 'refs/heads/main' }), ]; const operands: readonly Partial[] = [ {}, @@ -1235,3 +1237,347 @@ describe('bounds stay aligned with the neighbouring boundaries', () => { expect(readJobOperation({ requestId: overLimit }).requestId).not.toBe(atLimit); }); }); + +/* ------------------------------------------------------------------------- + * C1-A04: Git-equivalent branch-ref spellings + * + * Git resolves `main`, `heads/main`, and `refs/heads/main` to one ref. A + * boundary that compares ref *strings* therefore has three names for one + * authority target unless it fixes the spelling first, and the quarantine + * invariant — "the repair branch and the protected parent are distinct actual + * branches" — degrades into "the two strings are unequal". + * ------------------------------------------------------------------------- */ + +/** Spellings of one and the same branch. One simple, one nested. */ +const ALIAS_FAMILIES: readonly { + readonly branch: string; + readonly spellings: readonly string[]; +}[] = [ + { + branch: 'refs/heads/main', + spellings: ['main', 'heads/main', 'refs/heads/main'], + }, + { + branch: 'refs/heads/feature/pr-042-parent', + spellings: [ + 'feature/pr-042-parent', + 'heads/feature/pr-042-parent', + 'refs/heads/feature/pr-042-parent', + ], + }, +]; + +/** The operations that mutate a ref, and so must never accept an alias. */ +const REF_WRITE_OPERATIONS: readonly string[] = [ + JOB_OPERATION.REPAIR_COMMIT, + JOB_OPERATION.REPAIR_PUSH, +]; + +/** Alias spellings of the fixture repair branch. */ +const REPAIR_BRANCH_ALIASES: readonly string[] = ['repair/job-0001', 'heads/repair/job-0001']; + +function refRequest(operation: string, ref: string): JobOperationRequest { + return buildRequest({ operation, worktreeId: REPAIR_WORKTREE, ref, force: false }); +} + +describe('the canonical branch-ref reader', () => { + it('accepts only the fully qualified refs/heads/ spelling', () => { + expect(readCanonicalBranchRef('refs/heads/main')).toBe('refs/heads/main'); + expect(readCanonicalBranchRef(PARENT_REF)).toBe(PARENT_REF); + expect(readCanonicalBranchRef('refs/heads/repair/c1-a04_ref.alias-1')).toBe( + 'refs/heads/repair/c1-a04_ref.alias-1', + ); + }); + + it('refuses every other spelling of the same branch', () => { + for (const family of ALIAS_FAMILIES) { + for (const spelling of family.spellings) { + if (spelling === family.branch) { + expect(readCanonicalBranchRef(spelling), spelling).toBe(spelling); + continue; + } + expect(readCanonicalBranchRef(spelling), spelling).toBeNull(); + } + } + for (const alias of [...PARENT_REF_ALIASES, ...REPAIR_BRANCH_ALIASES]) { + expect(readCanonicalBranchRef(alias), alias).toBeNull(); + } + }); + + it('refuses partially qualified, differently rooted, and mis-cased prefixes', () => { + for (const value of [ + 'refs/heads/', + 'refs/head/main', + 'refs/tags/main', + 'refs/remotes/origin/main', + 'Refs/Heads/main', + 'REFS/HEADS/main', + '/refs/heads/main', + 'refs/heads//main', + 'refs/heads/main/', + ' refs/heads/main', + 'refs/heads/main ', + 'refs/heads/main\n', + ]) { + expect(readCanonicalBranchRef(value), JSON.stringify(value)).toBeNull(); + } + }); + + it('refuses the ref-name forms git itself refuses, and the revision operators', () => { + for (const value of [ + 'refs/heads/.hidden', + 'refs/heads/main.', + 'refs/heads/feature/.x', + 'refs/heads/a..b', + 'refs/heads/../../etc/passwd', + 'refs/heads/main.lock', + 'refs/heads/main.LOCK', + 'refs/heads/feature/x.lock', + 'refs/heads/main@{1}', + 'refs/heads/main^{}', + 'refs/heads/main~1', + 'refs/heads/ma in', + 'refs/heads/ma:in', + 'refs/heads/ma?in', + 'refs/heads/ma*in', + 'refs/heads/ma[in', + 'refs/heads/ma\\in', + // Non-ASCII is refused outright: the precomposed and decomposed spellings + // below are unequal strings that a loose ref can resolve to a single ref. + 'refs/heads/café', + 'refs/heads/café', + ]) { + expect(readCanonicalBranchRef(value), JSON.stringify(value)).toBeNull(); + } + }); + + it('fails closed on hostile values without throwing', () => { + for (const value of NON_OBJECTS) { + expect(() => readCanonicalBranchRef(value)).not.toThrow(); + expect(readCanonicalBranchRef(value)).toBeNull(); + } + expect(readCanonicalBranchRef({ toString: () => 'refs/heads/main' })).toBeNull(); + expect(readCanonicalBranchRef(['refs/heads/main'])).toBeNull(); + expect(readCanonicalBranchRef(revokedProxy())).toBeNull(); + // The identifier bound applies, and rejects rather than truncating. + expect( + readCanonicalBranchRef('refs/heads/' + 'a'.repeat(JOB_BOUNDS.MAX_IDENTIFIER_LENGTH)), + ).toBeNull(); + }); +}); + +describe('an alias spelling can never separate a repair branch from its parent', () => { + it('rejects the verified A04 configuration outright', () => { + // The reported exploit exactly: two spellings, one branch. + const job = buildJob({ protectedParentRef: 'refs/heads/main', repairBranch: 'main' }); + + expect(findInvalidRepairJobFields(job)).toContain('repairBranch'); + expect(readRepairJobAuthorization(job).snapshot).toBeNull(); + + for (const operation of REF_WRITE_OPERATIONS) { + const decision = authorizeJobOperation(job, refRequest(operation, 'main')); + + expect(decision.decision, operation).toBe(JOB_AUTHORIZATION.DENY); + expect(decision.reason, operation).toBe(JOB_AUTHORIZATION_REASON.JOB_ENVELOPE_INVALID); + expect(decision.mayExecuteOnce, operation).toBe(false); + expect(decision.permit, operation).toBeNull(); + } + }); + + it('rejects every alias pairing of one branch, for every ref-writing operation', () => { + for (const family of ALIAS_FAMILIES) { + for (const protectedParentRef of family.spellings) { + for (const repairBranch of family.spellings) { + const job = buildJob({ protectedParentRef, repairBranch }); + const label = `${protectedParentRef} | ${repairBranch}`; + + // The two refs denote one branch, so this is never a quarantined job. + // Whichever field is the offending one — a non-canonical spelling is + // reported against itself, a canonical collision against + // `repairBranch` — the envelope is refused and nothing is snapshotted. + expect(readRepairJobAuthorization(job).snapshot, label).toBeNull(); + if (protectedParentRef === family.branch) { + expect(findInvalidRepairJobFields(job), label).toContain('repairBranch'); + } else { + expect(findInvalidRepairJobFields(job), label).toContain('protectedParentRef'); + } + + for (const operation of REF_WRITE_OPERATIONS) { + for (const ref of family.spellings) { + const decision = authorizeJobOperation(job, refRequest(operation, ref)); + const attempt = `${label} -> ${operation} ${ref}`; + + expect(decision.decision, attempt).not.toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + expect(decision.mayExecuteOnce, attempt).toBe(false); + expect(decision.permit, attempt).toBeNull(); + } + } + } + } + } + }); + + it('rejects a repair branch that differs from the parent only by ASCII case', () => { + // Git stores loose refs as files, so on a case-insensitive filesystem these + // pairs can be one ref. C1 observes no filesystem, so it refuses the pair. + const pairs: readonly (readonly [string, string])[] = [ + ['refs/heads/main', 'refs/heads/Main'], + ['refs/heads/Main', 'refs/heads/main'], + ['refs/heads/feature/pr-042-parent', 'refs/heads/Feature/PR-042-Parent'], + ]; + + for (const [protectedParentRef, repairBranch] of pairs) { + const job = buildJob({ protectedParentRef, repairBranch }); + const label = `${protectedParentRef} | ${repairBranch}`; + + expect(findInvalidRepairJobFields(job), label).toContain('repairBranch'); + for (const operation of REF_WRITE_OPERATIONS) { + const decision = authorizeJobOperation(job, refRequest(operation, repairBranch)); + + expect(decision.mayExecuteOnce, label).toBe(false); + expect(decision.permit, label).toBeNull(); + } + } + }); +}); + +describe('an alias spelling in a request is refused, never compared', () => { + it('denies a commit or push naming an alias of the repair branch', () => { + for (const alias of REPAIR_BRANCH_ALIASES) { + for (const operation of REF_WRITE_OPERATIONS) { + const decision = authorizeJobOperation(buildJob(), refRequest(operation, alias)); + const label = `${operation} ${alias}`; + + expect(decision.reason, label).toBe(JOB_AUTHORIZATION_REASON.REF_MALFORMED); + expect(decision.decision, label).toBe(JOB_AUTHORIZATION.DENY); + expect(decision.mayExecuteOnce, label).toBe(false); + expect(decision.permit, label).toBeNull(); + } + } + }); + + it('denies a commit or push naming an alias of the protected parent', () => { + for (const alias of PARENT_REF_ALIASES) { + for (const operation of REF_WRITE_OPERATIONS) { + const decision = authorizeJobOperation(buildJob(), refRequest(operation, alias)); + const label = `${operation} ${alias}`; + + expect(decision.reason, label).toBe(JOB_AUTHORIZATION_REASON.REF_MALFORMED); + expect(decision.mayExecuteOnce, label).toBe(false); + expect(decision.permit, label).toBeNull(); + } + } + + // The canonical spelling of the parent is still refused, and still refused + // as an escape attempt rather than as a malformed operand. + for (const operation of REF_WRITE_OPERATIONS) { + expect( + authorizeJobOperation(buildJob(), refRequest(operation, PARENT_REF)).reason, + operation, + ).toBe(JOB_AUTHORIZATION_REASON.PROTECTED_REF_MUTATION); + } + }); + + it('never carries an alias operand into a normalized request', () => { + for (const alias of [...PARENT_REF_ALIASES, ...REPAIR_BRANCH_ALIASES]) { + const normalized = readJobOperation(refRequest(JOB_OPERATION.REPAIR_PUSH, alias)); + + expect(normalized.ref, alias).toBeNull(); + expect(normalized.refMalformed, alias).toBe(true); + } + }); +}); + +describe('change-request source and target separation survives aliasing', () => { + it('refuses an alias on either end', () => { + const job = buildJob(); + const cases: readonly (readonly [string, string])[] = [ + ['repair/job-0001', PARENT_REF], + ['heads/repair/job-0001', PARENT_REF], + [REPAIR_BRANCH, 'feature/pr-042-parent'], + [REPAIR_BRANCH, 'heads/feature/pr-042-parent'], + ['repair/job-0001', 'feature/pr-042-parent'], + ]; + + for (const [sourceRef, targetRef] of cases) { + const decision = authorizeJobOperation( + job, + buildRequest({ operation: JOB_OPERATION.REPAIR_CHANGE_REQUEST, sourceRef, targetRef }), + ); + const label = `${sourceRef} -> ${targetRef}`; + + expect(decision.reason, label).toBe(JOB_AUTHORIZATION_REASON.REF_MALFORMED); + expect(decision.decision, label).toBe(JOB_AUTHORIZATION.DENY); + expect(decision.permit, label).toBeNull(); + } + }); + + it('never models a change request whose source and target are one branch', () => { + for (const family of ALIAS_FAMILIES) { + for (const sourceRef of family.spellings) { + for (const targetRef of family.spellings) { + const decision = authorizeJobOperation( + buildJob({ protectedParentRef: family.branch, repairBranch: sourceRef }), + buildRequest({ + operation: JOB_OPERATION.REPAIR_CHANGE_REQUEST, + sourceRef, + targetRef, + }), + ); + const label = `${sourceRef} -> ${targetRef}`; + + expect(decision.decision, label).not.toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + expect(decision.mayExecuteOnce, label).toBe(false); + expect(decision.permit, label).toBeNull(); + } + } + } + }); +}); + +describe('legitimate distinct canonical refs still authorize the bounded operations', () => { + it('allows a push, a commit, and a stacked change request', () => { + const job = buildJob(); + + const push = authorizeJobOperation(job, refRequest(JOB_OPERATION.REPAIR_PUSH, REPAIR_BRANCH)); + expect(push.decision).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + expect(push.reason).toBe(JOB_AUTHORIZATION_REASON.WITHIN_JOB_ENVELOPE); + expect(push.permit?.operands.ref).toBe(REPAIR_BRANCH); + + const commit = authorizeJobOperation( + job, + refRequest(JOB_OPERATION.REPAIR_COMMIT, REPAIR_BRANCH), + ); + expect(commit.decision).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + expect(commit.permit?.operands.ref).toBe(REPAIR_BRANCH); + + const changeRequest = authorizeJobOperation( + job, + buildRequest({ + operation: JOB_OPERATION.REPAIR_CHANGE_REQUEST, + sourceRef: REPAIR_BRANCH, + targetRef: PARENT_REF, + }), + ); + expect(changeRequest.decision).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + expect(changeRequest.permit?.operands.sourceRef).toBe(REPAIR_BRANCH); + expect(changeRequest.permit?.operands.targetRef).toBe(PARENT_REF); + }); + + it('allows a nested repair branch stacked under a nested protected parent', () => { + const repairBranch = 'refs/heads/repair/c1-a04-ref-alias'; + const job = buildJob({ + protectedParentRef: 'refs/heads/feature/pr-042-parent', + repairBranch, + }); + + expect(findInvalidRepairJobFields(job)).toHaveLength(0); + const decision = authorizeJobOperation( + job, + refRequest(JOB_OPERATION.REPAIR_PUSH, repairBranch), + ); + + expect(decision.decision).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); + expect(decision.permit?.operands.ref).toBe(repairBranch); + }); +}); diff --git a/tests/domain/job-authorization.test.ts b/tests/domain/job-authorization.test.ts index d2acde3..b928a56 100644 --- a/tests/domain/job-authorization.test.ts +++ b/tests/domain/job-authorization.test.ts @@ -312,7 +312,12 @@ describe('refs: the repair branch is not the protected parent', () => { }); it('denies a push to any third ref', () => { - for (const ref of ['main', 'develop', 'release/1.0', 'repair/job-0002']) { + for (const ref of [ + 'refs/heads/main', + 'refs/heads/develop', + 'refs/heads/release/1.0', + 'refs/heads/repair/job-0002', + ]) { const decision = authorizeJobOperation(buildJob(), buildPush({ ref })); expect(decision.reason, ref).toBe(JOB_AUTHORIZATION_REASON.REF_NOT_REPAIR_BRANCH); } @@ -389,7 +394,7 @@ describe('refs: the repair branch is not the protected parent', () => { buildRequest({ operation: 'repair.change_request', sourceRef: REPAIR_BRANCH, - targetRef: 'main', + targetRef: 'refs/heads/main', }), ), JOB_AUTHORIZATION_REASON.CHANGE_REQUEST_TARGET_INVALID, diff --git a/tests/domain/repair-job-fixtures.ts b/tests/domain/repair-job-fixtures.ts index 53a0eac..6224689 100644 --- a/tests/domain/repair-job-fixtures.ts +++ b/tests/domain/repair-job-fixtures.ts @@ -19,10 +19,27 @@ export const JOB_B = 'job-0002'; export const POLICY_VERSION = 'cockpit-policy-v1'; -export const PARENT_REF = 'feature/pr-042-parent'; -export const REPAIR_BRANCH = 'repair/job-0001'; +/** + * Refs are canonical `refs/heads/` spellings everywhere in these fixtures. + * That is the only spelling C1 accepts, so a fixture in any other spelling would + * be testing an invalid envelope rather than a configured job. + */ +export const PARENT_REF = 'refs/heads/feature/pr-042-parent'; +export const REPAIR_BRANCH = 'refs/heads/repair/job-0001'; export const REPAIR_WORKTREE = 'worktree-job-0001'; +/** + * Alternate spellings git resolves to the same ref as {@link PARENT_REF}. + * + * None of these may ever be accepted as a repair branch or a ref operand: each + * one denotes the protected parent, and C1 must not be able to mistake it for a + * different branch merely because the strings differ. + */ +export const PARENT_REF_ALIASES: readonly string[] = [ + 'feature/pr-042-parent', + 'heads/feature/pr-042-parent', +]; + export const AUTHORIZED_PATH = 'src/domain/policy-gate.ts'; export const SECOND_AUTHORIZED_PATH = 'tests/domain/policy-gate.test.ts'; export const UNAUTHORIZED_PATH = 'src/domain/actions.ts'; From c743b65217cee372ba259cd4a1890469d529eed9 Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Wed, 19 Aug 2026 23:34:31 +0200 Subject: [PATCH 07/15] docs: clarify symbolic ref resolution boundary --- docs/architecture/C1-repair-job-authority.md | 108 ++++++++++++++----- src/domain/repair-job.ts | 100 +++++++++++------ 2 files changed, 153 insertions(+), 55 deletions(-) diff --git a/docs/architecture/C1-repair-job-authority.md b/docs/architecture/C1-repair-job-authority.md index db504d1..cc8833b 100644 --- a/docs/architecture/C1-repair-job-authority.md +++ b/docs/architecture/C1-repair-job-authority.md @@ -121,15 +121,18 @@ offending names in declaration order. Two structural invariants are enforced as configuration validity rather than as a runtime check that could be forgotten: -- `repairBranch` and `protectedParentRef` denote **different branches**. A job - whose repair branch *is* the protected parent is not a quarantined repair; it - is a direct write to protected history wearing a repair job's name. +- `repairBranch` and `protectedParentRef` are **different branch refs** under + C1's canonical comparison rule. A job whose repair branch *is* the protected + parent is not a quarantined repair; it is a direct write to protected history + wearing a repair job's name. This compares canonical ref *names*; establishing + that two accepted names resolve to distinct targets in a repository is the + later trusted execution boundary's obligation, described below. - `independentValidatorId !== repairAgentId`. A repair agent that is its own validator defeats the quarantine the whole pipeline exists to enforce. ### Branch refs have exactly one accepted spelling -"Different branches", not "different strings". Git resolves `main`, +"Different branch refs", not "different strings". Git resolves `main`, `heads/main`, and `refs/heads/main` to one and the same ref, so a boundary that compares ref strings has three names for one authority target. Configuring `protectedParentRef: 'refs/heads/main'` beside `repairBranch: 'main'` would @@ -149,14 +152,18 @@ and refuses every other spelling of the same branch as malformed: - no segment beginning or ending with `.`, no `..` anywhere, and no segment ending in `.lock` in any ASCII case -The property that buys: **two accepted refs denote the same branch if and only if -they are equal strings.** That is what makes the distinctness invariant mean -something. The conservative ASCII character set is part of the guarantee, not a -convenience — it removes Unicode normalisation, under which an NFC and an NFD -spelling of one name are unequal strings a filesystem-backed loose ref can -resolve to a single ref, and it removes `~`, `^`, `:`, `?`, `*`, `[`, `\`, `@{`, -and whitespace in one rule. Nothing is normalised, prefixed, or case-folded on -the way in: a value is accepted exactly as supplied or refused. +The property that buys is a property of ref *names*, not of repository state: +**two accepted refs are the same canonical ref name if and only if they are equal +strings.** That is what closes caller-controlled textual aliasing and what makes +the distinctness invariant mean something at this layer. It is not a claim that +two unequal canonical names denote two distinct targets in a repository; see +*What canonical ref names do and do not prove* below. The conservative ASCII +character set is part of the guarantee, not a convenience — it removes Unicode +normalisation, under which an NFC and an NFD spelling of one name are unequal +strings a filesystem-backed loose ref can resolve to a single ref, and it removes +`~`, `^`, `:`, `?`, `*`, `[`, `\`, `@{`, and whitespace in one rule. Nothing is +normalised, prefixed, or case-folded on the way in: a value is accepted exactly +as supplied or refused. The same reader is applied to **every** security-relevant ref position — the two job fields, and the `ref`, `sourceRef`, and `targetRef` request operands — so @@ -165,14 +172,6 @@ uncanonical request operand. A supplied operand that is not canonical is refused `REF_MALFORMED` before any comparison, rather than compared as though it were a different branch. -**What this does not prove**, and must not be claimed to: that two unequal -accepted refs are two distinct refs on every filesystem. Git stores loose refs as -files, so on a case-insensitive filesystem `refs/heads/Main` and -`refs/heads/main` can be one ref while comparing unequal. C1 observes no -filesystem, so it refuses the ambiguous case instead of pretending it away: the -job's two configured refs are additionally compared with ASCII case folded, and a -pair that differs only by case is rejected as malformed configuration. - One relationship is enforced at authorization time, because it is about freshness rather than shape: `findingHeadSha` must equal `parentHeadSha`, or every operation is denied `FINDING_SHA_STALE`. A repair derived from a finding @@ -181,6 +180,55 @@ PR 004 remains the owner of `CURRENT` versus `STALE` for evidence; this is the narrower structural check that the job's own two SHAs agree, which C1 can decide without importing that kernel or producing a second answer to PR 004's question. +### What canonical ref names do and do not prove + +Stated precisely, because overclaiming here would be worse than not checking. + +**Proved.** An accepted value is a string in the one canonical `refs/heads/` +shape, and two accepted values that remain unequal under C1's documented +comparison rule are two different canonical ref names. Caller-supplied textual +aliasing is closed within that structural authority: the originally proven bypass +— configuring or requesting `main`, `heads/main`, and `refs/heads/main` against +one another so the protected parent could be presented as a different branch — is +refused as `REF_MALFORMED` before any comparison, and a configured pair that +collides under the comparison rule invalidates the job rather than authorizing it. + +**Not proved, and not claimed.** That two different canonical ref names are two +distinct branch targets in a repository. C1 does not establish repository-resolved +ref identity, does not detect whether an accepted ref is symbolic, does not +resolve a symbolic ref's target, does not determine whether two distinct canonical +names ultimately dereference to the same repository target, and observes no live +repository state. Two independent reasons stand: + +- **Symbolic refs.** A repository may hold a canonical-looking ref — say + `refs/heads/repair` — that is itself a symbolic ref to `refs/heads/main`. + Whether such a ref exists, and what it points at, is repository state at the + moment the name is used. C1 runs no git, spawns no subprocess, opens no file, + and observes no repository, so no string comparison it performs can decide it. +- **Filesystem identity.** Git stores loose refs as files, so on a + case-insensitive filesystem `refs/heads/Main` and `refs/heads/main` can be one + ref while comparing unequal. C1 observes no filesystem, so it refuses the + ambiguous case instead of pretending it away: the job's two configured refs are + additionally compared with ASCII case folded, and a pair that differs only by + case is rejected as malformed configuration. + +The case fold is a conservative refusal, not a resolution. It narrows one +filesystem-dependent collision that is characterisable from the strings alone; it +establishes nothing about symbolic refs, which are not decidable from a string at +all. + +**A future trusted repository/Git execution boundary must close the rest.** Before +exercising any ref-mutating authority represented by an `ExecutionPermit`, that +boundary must resolve the requested ref against the actual repository, resolve or +reject repository-dependent symbolic refs, and establish that the repair ref's +resolved target is not the protected parent's. It must **fail closed** — refuse +the operation — if the requested repair ref resolves or dereferences to the +protected parent, or if safe target identity cannot be established at all. + +Writing that obligation down adds no runtime git authority to C1 and grants no new +authority anywhere: C1 gains no git invocation, no filesystem access, no +subprocess, and no network, and remains pure TypeScript. + ## Operations are structured, not named A generic action name is not sufficient for Cockpit write authority. There is no @@ -197,8 +245,10 @@ authority cannot be checked against an exact operand has no place in the model. | `repair.change_request` | source ref, target ref | repair branch → protected parent ref | Every ref operand is read through the same canonical branch-ref reader the job -envelope uses, so "exactly the repair branch" is a claim about a branch and not -about a spelling. +envelope uses, so "exactly the repair branch" is a claim about a canonical ref +name and not about a caller's chosen spelling. It is not a claim about what that +name resolves to in a repository, which only the later trusted execution boundary +can establish. `repair.change_request` is the **only** operation that may name the protected parent ref, and only as a change-request *target*. Opening a change request @@ -404,6 +454,13 @@ actually holds: Forgery therefore buys nothing, and a permit widens no authority — it records authority already derived from trusted configuration. +A permit is also **not a repository-safety finding**. That a `repair.commit` or +`repair.push` ref operand passed C1's canonical syntax validation says nothing +about what that ref resolves to in the repository the operation would touch, so a +permit must never be read as proof that repository-level ref resolution is safe. +The trusted execution boundary that acts on a permit performs its own resolution +and fails closed; see *What canonical ref names do and do not prove* above. + ### Single use Single use is stated structurally. `singleUse` is typed as the literal `true` and @@ -525,8 +582,11 @@ C1 implements none of that workflow. It encodes only the minimal authority invariants that stop a later layer from bypassing the quarantine by accident: - The protected parent ref is never a write target of any operation, under any - spelling: refs are canonical everywhere, so an alias of the parent cannot be - presented as a different branch. + *spelling*: refs are canonical everywhere, so a caller cannot present a textual + alias of the parent as a different branch. Repository-dependent aliasing — a + canonical repair ref that is symbolic to the parent — is not visible to a pure + string boundary, and is the later trusted execution boundary's to resolve or + reject before any ref-mutating operation runs. - Filesystem-shaped operations are bound to the repair worktree, so an edit cannot land in the parent's checkout. - The stacked change request must run from the repair branch to the protected diff --git a/src/domain/repair-job.ts b/src/domain/repair-job.ts index a0c03fe..47a1fc9 100644 --- a/src/domain/repair-job.ts +++ b/src/domain/repair-job.ts @@ -462,17 +462,18 @@ function endsWithDotLockSuffix(value: string, start: number, end: number): boole * * So C1 does not resolve; it **narrows**. Exactly one spelling is accepted, and * every other spelling of the same branch is refused as malformed rather than - * silently treated as a different ref. The property that buys is precise: + * silently treated as a different ref. The property that buys is precise, and it + * is a property of ref *names* rather than of repository state: * - * > Two accepted refs denote the same branch if and only if they are equal - * > strings — up to the ASCII-case caveat below. + * > Two accepted refs are the same canonical ref name if and only if they are + * > equal strings — up to the ASCII-case caveat below. * * That is what makes `repairBranch !== protectedParentRef` mean "two different - * branches" instead of "two different strings". Before this, a job configured - * with `protectedParentRef: 'refs/heads/main'` and `repairBranch: 'main'` was - * accepted as a quarantined repair, and a `repair.push` naming `main` passed - * every check and produced an `ExecutionPermit` whose ref denotes the protected - * branch. + * canonical ref names" instead of "two different strings", which is what closes + * caller-controlled textual aliasing. Before this, a job configured with + * `protectedParentRef: 'refs/heads/main'` and `repairBranch: 'main'` was accepted + * as a quarantined repair, and a `repair.push` naming `main` passed every check + * and produced an `ExecutionPermit` whose ref denotes the protected branch. * * Accepted, and nothing else: * @@ -498,13 +499,34 @@ function endsWithDotLockSuffix(value: string, start: number, end: number): boole * rewritten. * * **What this does not prove**, and must not be claimed to: that two unequal - * accepted refs are two distinct refs *on every filesystem*. Git stores loose - * refs as files, so on a case-insensitive filesystem `refs/heads/Main` and - * `refs/heads/main` can be one ref while comparing unequal here. That residue is - * handled where it matters — {@link mayDenoteSameBranchRef} compares the job's - * two configured refs case-insensitively, so such a pair is refused as - * configuration — rather than pretended away here. C1 observes no filesystem and - * cannot do better than refuse the ambiguous case. + * accepted refs are two distinct branch targets in a repository. What is proved + * is a structural canonical ref-name representation and this module's own + * documented string-comparison rules; repository-resolved ref identity is not + * established here. Two residues stand: + * + * - **Symbolic refs.** A repository may hold a canonical-looking ref — say + * `refs/heads/repair` — that is itself a symbolic ref to `refs/heads/main`. + * Whether such a ref exists, and what it dereferences to, is repository state + * at the moment the name is used. C1 does not detect that a ref is symbolic, + * does not resolve a symbolic ref's target, and cannot tell whether two + * distinct canonical names ultimately dereference to one repository target. + * - **Filesystem identity.** Git stores loose refs as files, so on a + * case-insensitive filesystem `refs/heads/Main` and `refs/heads/main` can be + * one ref while comparing unequal here. That residue is handled where it + * matters — {@link mayDenoteSameBranchRef} compares the job's two configured + * refs case-insensitively, so such a pair is refused as configuration — rather + * than pretended away here. C1 observes no filesystem and cannot do better + * than refuse the ambiguous case. + * + * The symbolic-ref residue is not narrowable from a string at all. A later + * trusted repository/Git execution boundary must resolve the requested ref + * against the actual repository and **fail closed** — refusing the operation — + * before exercising any ref-mutating authority carried by an `ExecutionPermit`, + * if the repair ref resolves or dereferences to the protected parent or if safe + * target identity cannot be established. Nothing here acquires git invocation, + * filesystem access, a subprocess, or network to decide it. See + * `docs/architecture/C1-repair-job-authority.md`, "What canonical ref names do + * and do not prove". * * The value is returned exactly as supplied, or not at all. No normalisation, * no prefixing, no case folding: a boundary that repaired the spelling would be @@ -557,7 +579,7 @@ export function readCanonicalBranchRef(value: unknown): string | null { } /** - * Could these two accepted branch refs denote the same branch? + * Could these two accepted canonical ref names collapse into one ref? * * Both arguments are already-validated canonical refs, so this is exact string * equality widened by one conservative allowance: ASCII case. Git stores loose @@ -567,9 +589,16 @@ export function readCanonicalBranchRef(value: unknown): string | null { * as possibly-identical and the job that configures one is refused. * * Conservative in the safe direction: it answers `true` — refuse — whenever it - * cannot establish that the two refs are distinct, including for a character it - * could not read at all. Only ASCII case is folded, because the canonical reader - * admits no other alphabet. + * cannot establish that the two *names* are distinct, including for a character + * it could not read at all. Only ASCII case is folded, because the canonical + * reader admits no other alphabet. + * + * A `false` result means only that the two names are distinct under this rule. + * It is **not** a finding that they denote distinct targets in a repository: this + * compares strings and resolves nothing, so a canonical name that is a symbolic + * ref to the other still answers `false` here. Repository-resolved identity is + * the later trusted repository/Git execution boundary's to establish; see + * {@link readCanonicalBranchRef}. */ function mayDenoteSameBranchRef(left: string, right: string): boolean { if (left === right) { @@ -711,16 +740,23 @@ export interface RepairJobAuthorization { * The isolated repair branch, in the canonical `refs/heads/` spelling * {@link readCanonicalBranchRef} defines. * - * Must denote a different branch from {@link protectedParentRef}. A job whose - * repair branch is the protected parent ref is not a quarantined repair; it is - * a direct write to protected history wearing a repair job's name, and it is - * rejected as malformed configuration rather than evaluated. + * Must be a different canonical ref name from {@link protectedParentRef}. A + * job whose repair branch is the protected parent ref is not a quarantined + * repair; it is a direct write to protected history wearing a repair job's + * name, and it is rejected as malformed configuration rather than evaluated. * - * "Different branch", not "different string": both refs are read through the - * canonical reader, so an alternate spelling of the protected parent — `main`, - * `heads/main` — cannot pass as an isolated repair branch, and a pair that - * differs only by ASCII case is refused too because a case-insensitive + * "Different branch ref", not "different string": both refs are read through + * the canonical reader, so an alternate spelling of the protected parent — + * `main`, `heads/main` — cannot pass as an isolated repair branch, and a pair + * that differs only by ASCII case is refused too because a case-insensitive * filesystem can store the two as one loose ref. + * + * That closes caller-controlled textual aliasing only. It does not establish + * that the two names resolve to distinct targets in a repository — a canonical + * repair ref that is symbolic to the parent is invisible to a pure string + * boundary — which the later trusted repository/Git execution boundary must + * resolve or reject before any ref-mutating operation runs. See + * {@link readCanonicalBranchRef}. */ readonly repairBranch: string; /** The isolated repair worktree. Filesystem-shaped operations are bound to it. */ @@ -886,11 +922,13 @@ export function readRepairJobAuthorization(job: RepairJobAuthorization): RepairJ if (findingHeadSha === null) { append(invalidFields, 'findingHeadSha'); } - // The repair branch must be a *different branch* from the protected parent + // The repair branch must be a *different branch ref* from the protected parent // ref, or the isolation the whole quarantine depends on does not exist. Both - // refs are canonical here, so unequal strings are different branches — except - // for the ASCII-case pair a case-insensitive filesystem can collapse into one - // loose ref, which `mayDenoteSameBranchRef` refuses as well. + // refs are canonical here, so unequal strings are different canonical ref names + // — except for the ASCII-case pair a case-insensitive filesystem can collapse + // into one loose ref, which `mayDenoteSameBranchRef` refuses as well. This is a + // name-level check: repository-resolved identity, including a canonical ref + // that is symbolic to the parent, is the later trusted execution boundary's. if ( repairBranch === null || (protectedParentRef !== null && mayDenoteSameBranchRef(repairBranch, protectedParentRef)) From fc49e9aa81f949807dd9d5b2acc4afbd792032e1 Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Thu, 20 Aug 2026 01:17:08 +0200 Subject: [PATCH 08/15] docs: complete execution boundary ref contract --- docs/architecture/C1-repair-job-authority.md | 103 +++++++++++++++---- src/domain/repair-job.ts | 30 ++++-- 2 files changed, 102 insertions(+), 31 deletions(-) diff --git a/docs/architecture/C1-repair-job-authority.md b/docs/architecture/C1-repair-job-authority.md index cc8833b..957efeb 100644 --- a/docs/architecture/C1-repair-job-authority.md +++ b/docs/architecture/C1-repair-job-authority.md @@ -217,13 +217,63 @@ filesystem-dependent collision that is characterisable from the strings alone; i establishes nothing about symbolic refs, which are not decidable from a string at all. -**A future trusted repository/Git execution boundary must close the rest.** Before -exercising any ref-mutating authority represented by an `ExecutionPermit`, that -boundary must resolve the requested ref against the actual repository, resolve or -reject repository-dependent symbolic refs, and establish that the repair ref's -resolved target is not the protected parent's. It must **fail closed** — refuse -the operation — if the requested repair ref resolves or dereferences to the -protected parent, or if safe target identity cannot be established at all. +**A future trusted repository/Git execution boundary must close the rest.** C1 +establishes structural canonical ref-name authority; it cannot establish live +repository identity, cannot bind the target a mutation will actually reach, and +cannot enforce anything across a concurrent change. Before acting on any authority +an `ExecutionPermit` records — not only operations that write a ref — that +boundary must satisfy the requirements below, and must **fail closed** — refuse +the operation — wherever a required identity cannot be safely established, +wherever resolution cycles or is otherwise indeterminate, or wherever an effective +identity is or dereferences to the protected parent. + +*Which identity is compared.* The isolation question is about the **effective +ref-name referent** — the terminal ref reached by resolving a symbolic-ref chain — +not about commit-object identity. A freshly created repair branch may legitimately +point at the **same commit object** as the protected parent until its first repair +commit, so distinct commit OIDs are neither necessary nor sufficient: two +different branch refs may share one commit OID, and commit-object equality does not +make two refs the same authority target. The boundary must detect and reject a +symbolic or effective ref-name identity that aliases the protected parent, and +must not rest the check on whether two refs currently resolve to the same commit. + +*Binding the effective mutation target.* A resolved ref *name* is not the target a +mutation will advance, and the boundary must bind the two before it acts: + +- **`repair.commit`.** A commit advances the branch reached through the authorized + worktree's effective `HEAD` referent, not whatever ref name the request carried. + The boundary must bind that effective `HEAD` referent to the authorized repair + ref and refuse to commit if the worktree is detached, attached to the protected + parent, attached to any other ref, or its safe binding cannot be established. +- **`repair.push`.** The authorized repair ref identifies a source, not a + destination. The boundary must bind the push's **effective destination ref** to + the authorized repair ref and must not let a caller-selected destination refspec + redirect the push; the receiving/mutation side must fail closed if the effective + destination is the protected parent or cannot be proven to be the authorized + repair ref. + +*Operands that set direction without mutating a ref.* The obligation is not +limited to ref-mutating operations. `repair.change_request` mutates no ref, but +its `sourceRef` and `targetRef` fix the effective direction of the stacked +validation request, which the quarantine requires to run **from** the repair +branch **to** the protected parent. The boundary must establish that the effective +source identity is the authorized repair ref and the effective target identity is +the protected parent ref, reject a symbolic or effective alias that changes that +direction, and fail closed if either effective identity cannot be safely +established. + +*Concurrency is not closed by a pre-check.* A resolve-then-check-then-mutate +sequence is **not** an atomic security guarantee: the effective ref or referent +can change between the comparison and the update, so a name observed as an +ordinary repair ref can become symbolic to the protected parent before the +mutation lands. The invariant must be enforced **at the actual mutation/receiving +boundary**, by a mechanism whose semantics prevent an unchecked identity change +between comparison and update — not by an earlier client-side observation this +boundary later trusts. + +This document states the required invariant, not an implementation: it names no +git command, lock, or transaction mechanism, and it claims no more atomicity than +the eventual executor's own primitives can actually provide. Writing that obligation down adds no runtime git authority to C1 and grants no new authority anywhere: C1 gains no git invocation, no filesystem access, no @@ -244,11 +294,14 @@ authority cannot be checked against an exact operand has no place in the model. | `repair.push` | ref, non-force | ref is exactly the repair branch and the push is not forced | | `repair.change_request` | source ref, target ref | repair branch → protected parent ref | -Every ref operand is read through the same canonical branch-ref reader the job -envelope uses, so "exactly the repair branch" is a claim about a canonical ref -name and not about a caller's chosen spelling. It is not a claim about what that -name resolves to in a repository, which only the later trusted execution boundary -can establish. +Every ref operand — the `repair.commit` and `repair.push` ref, and the +`repair.change_request` source and target refs alike — is read through the same +canonical branch-ref reader the job envelope uses, so "exactly the repair branch" +is a claim about a canonical ref name and not about a caller's chosen spelling. It +is not a claim about the effective ref-name referent that name reaches in a +repository, about which ref a commit or push would actually advance, or about the +effective direction of a change request — all of which only the later trusted +execution boundary can establish. `repair.change_request` is the **only** operation that may name the protected parent ref, and only as a change-request *target*. Opening a change request @@ -454,12 +507,17 @@ actually holds: Forgery therefore buys nothing, and a permit widens no authority — it records authority already derived from trusted configuration. -A permit is also **not a repository-safety finding**. That a `repair.commit` or -`repair.push` ref operand passed C1's canonical syntax validation says nothing -about what that ref resolves to in the repository the operation would touch, so a -permit must never be read as proof that repository-level ref resolution is safe. -The trusted execution boundary that acts on a permit performs its own resolution -and fails closed; see *What canonical ref names do and do not prove* above. +A permit is also **not a repository-safety finding**. That a ref operand — a +`repair.commit` or `repair.push` ref, or a `repair.change_request` `sourceRef` or +`targetRef` — passed C1's canonical syntax validation says nothing about the +effective ref-name referent it reaches in the repository the operation would +touch, about which ref a commit or push would actually advance, or about the +effective direction of a change request, so a permit must never be read as proof +that repository-level ref identity, the effective mutation target, or the +change-request direction is safe. The trusted execution boundary that acts on a +permit binds the effective target, resolves the effective identity, enforces it at +the mutation/receiving boundary, and fails closed; see *What canonical ref names +do and do not prove* above. ### Single use @@ -584,9 +642,12 @@ invariants that stop a later layer from bypassing the quarantine by accident: - The protected parent ref is never a write target of any operation, under any *spelling*: refs are canonical everywhere, so a caller cannot present a textual alias of the parent as a different branch. Repository-dependent aliasing — a - canonical repair ref that is symbolic to the parent — is not visible to a pure - string boundary, and is the later trusted execution boundary's to resolve or - reject before any ref-mutating operation runs. + canonical repair ref that is symbolic to the parent, a worktree `HEAD` or push + destination whose effective target is the parent, a change-request source or + target whose effective direction is reversed, or an effective ref that changes + after a pre-check — is not visible to a pure string boundary, and is the later + trusted execution boundary's to bind, resolve, or reject before it acts on any + authority a permit records. - Filesystem-shaped operations are bound to the repair worktree, so an edit cannot land in the parent's checkout. - The stacked change request must run from the repair branch to the protected diff --git a/src/domain/repair-job.ts b/src/domain/repair-job.ts index 47a1fc9..bf64b8d 100644 --- a/src/domain/repair-job.ts +++ b/src/domain/repair-job.ts @@ -518,13 +518,20 @@ function endsWithDotLockSuffix(value: string, start: number, end: number): boole * than pretended away here. C1 observes no filesystem and cannot do better * than refuse the ambiguous case. * - * The symbolic-ref residue is not narrowable from a string at all. A later - * trusted repository/Git execution boundary must resolve the requested ref - * against the actual repository and **fail closed** — refusing the operation — - * before exercising any ref-mutating authority carried by an `ExecutionPermit`, - * if the repair ref resolves or dereferences to the protected parent or if safe - * target identity cannot be established. Nothing here acquires git invocation, - * filesystem access, a subprocess, or network to decide it. See + * The symbolic-ref residue is not narrowable from a string at all, and neither is + * the effective target a mutation would reach. A later trusted repository/Git + * execution boundary must, before acting on any authority an `ExecutionPermit` + * records, resolve the requested ref's effective ref-name referent against the + * actual repository — the terminal ref reached through a symbolic-ref chain, not + * commit-object identity, since a fresh repair branch may legitimately share the + * protected parent's commit OID — and bind the effective mutation target it will + * actually advance: the worktree's effective `HEAD` referent for a commit, and the + * effective destination ref for a push. It must **fail closed** — refusing the + * operation — if that effective identity is or dereferences to the protected + * parent, if it changes between comparison and update, or if it cannot be safely + * established; a resolve-then-mutate pre-check is not itself atomic, so the + * invariant is enforced at the mutation/receiving boundary. Nothing here acquires + * git invocation, filesystem access, a subprocess, or network to decide it. See * `docs/architecture/C1-repair-job-authority.md`, "What canonical ref names do * and do not prove". * @@ -596,8 +603,11 @@ export function readCanonicalBranchRef(value: unknown): string | null { * A `false` result means only that the two names are distinct under this rule. * It is **not** a finding that they denote distinct targets in a repository: this * compares strings and resolves nothing, so a canonical name that is a symbolic - * ref to the other still answers `false` here. Repository-resolved identity is - * the later trusted repository/Git execution boundary's to establish; see + * ref to the other still answers `false` here, and — since two different branch + * refs may legitimately share one commit object — commit-object equality is not + * the question either. Repository-resolved identity, meaning the effective + * ref-name referent reached by resolving a symbolic-ref chain, is the later + * trusted repository/Git execution boundary's to establish; see * {@link readCanonicalBranchRef}. */ function mayDenoteSameBranchRef(left: string, right: string): boolean { @@ -755,7 +765,7 @@ export interface RepairJobAuthorization { * that the two names resolve to distinct targets in a repository — a canonical * repair ref that is symbolic to the parent is invisible to a pure string * boundary — which the later trusted repository/Git execution boundary must - * resolve or reject before any ref-mutating operation runs. See + * resolve or reject before it acts on any authority a permit records. See * {@link readCanonicalBranchRef}. */ readonly repairBranch: string; From afba343d1ca2bb8c27e50dd5e8964fc039b84034 Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Thu, 20 Aug 2026 14:37:40 +0200 Subject: [PATCH 09/15] docs: make protected parent role-aware --- docs/architecture/C1-repair-job-authority.md | 30 +++++++++++++++++--- src/domain/repair-job.ts | 17 ++++++----- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/docs/architecture/C1-repair-job-authority.md b/docs/architecture/C1-repair-job-authority.md index 957efeb..cca523e 100644 --- a/docs/architecture/C1-repair-job-authority.md +++ b/docs/architecture/C1-repair-job-authority.md @@ -225,7 +225,23 @@ an `ExecutionPermit` records — not only operations that write a ref — that boundary must satisfy the requirements below, and must **fail closed** — refuse the operation — wherever a required identity cannot be safely established, wherever resolution cycles or is otherwise indeterminate, or wherever an effective -identity is or dereferences to the protected parent. +identity is or dereferences to a ref the operand's role is not authorized to +denote. + +*The protected-parent rule is role-bound.* Protected-parent identity is forbidden +only where it is unauthorized for the operand's role — which is every role but +one. The effective mutation target of a `repair.commit` or a `repair.push`, and a +`repair.change_request` `sourceRef`, must each be the authorized repair ref, so +for all three an effective identity that is or dereferences to the protected +parent is a refusal. A `repair.change_request` `targetRef` is the single operand +whose *required* effective identity **is** the protected parent ref — the same +operand C1's string layer already singles out as the only one that may name it — +so for that role, and only that role, reaching the protected parent is the +authorized outcome and reaching anything else is the refusal. Stated role-blind +instead, the rule would forbid the one direction the quarantine depends on. No +role widens past this: an operand authorized to denote the protected parent as a +change-request *target* acquires no authority to denote it anywhere else, and the +exemption never reaches an operand that would mutate the parent. *Which identity is compared.* The isolation question is about the **effective ref-name referent** — the terminal ref reached by resolving a symbolic-ref chain — @@ -233,9 +249,15 @@ not about commit-object identity. A freshly created repair branch may legitimate point at the **same commit object** as the protected parent until its first repair commit, so distinct commit OIDs are neither necessary nor sufficient: two different branch refs may share one commit OID, and commit-object equality does not -make two refs the same authority target. The boundary must detect and reject a -symbolic or effective ref-name identity that aliases the protected parent, and -must not rest the check on whether two refs currently resolve to the same commit. +make two refs the same authority target. The boundary must therefore compare +effective ref-name referents, and must not rest the check on whether two refs +currently resolve to the same commit. What that comparison must *yield* is fixed +by the operand's role: for an operand whose required identity is the authorized +repair ref, a symbolic or effective ref-name identity that aliases the protected +parent must be detected and rejected; for the one operand whose required identity +is the protected parent ref — the `repair.change_request` `targetRef` — the alias +to detect and reject is the converse one, an effective identity that is not the +protected parent ref. *Binding the effective mutation target.* A resolved ref *name* is not the target a mutation will advance, and the boundary must bind the two before it acts: diff --git a/src/domain/repair-job.ts b/src/domain/repair-job.ts index bf64b8d..c8ffd1a 100644 --- a/src/domain/repair-job.ts +++ b/src/domain/repair-job.ts @@ -527,13 +527,16 @@ function endsWithDotLockSuffix(value: string, start: number, end: number): boole * protected parent's commit OID — and bind the effective mutation target it will * actually advance: the worktree's effective `HEAD` referent for a commit, and the * effective destination ref for a push. It must **fail closed** — refusing the - * operation — if that effective identity is or dereferences to the protected - * parent, if it changes between comparison and update, or if it cannot be safely - * established; a resolve-then-mutate pre-check is not itself atomic, so the - * invariant is enforced at the mutation/receiving boundary. Nothing here acquires - * git invocation, filesystem access, a subprocess, or network to decide it. See - * `docs/architecture/C1-repair-job-authority.md`, "What canonical ref names do - * and do not prove". + * operation — if that effective mutation target is or dereferences to the + * protected parent, if it changes between comparison and update, or if it cannot + * be safely established; a resolve-then-mutate pre-check is not itself atomic, so + * the invariant is enforced at the mutation/receiving boundary. That refusal is + * bound to the operand's role rather than being a blanket ban on the protected + * parent's identity: a `repair.change_request` `targetRef` is *required* to reach + * the protected parent ref, and fails closed when it reaches anything else. + * Nothing here acquires git invocation, filesystem access, a subprocess, or + * network to decide it. See `docs/architecture/C1-repair-job-authority.md`, + * "What canonical ref names do and do not prove". * * The value is returned exactly as supplied, or not at all. No normalisation, * no prefixing, no case folding: a boundary that repaired the spelling would be From bf78cf668e6b93701c83ea0a3537c185aaca40e8 Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Thu, 20 Aug 2026 18:38:31 +0200 Subject: [PATCH 10/15] docs: bind change requests at provider boundary Co-Authored-By: Claude Opus 4.8 --- docs/architecture/C1-repair-job-authority.md | 46 ++++++++++++++------ src/domain/repair-job.ts | 14 ++++-- 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/docs/architecture/C1-repair-job-authority.md b/docs/architecture/C1-repair-job-authority.md index cca523e..51b16c8 100644 --- a/docs/architecture/C1-repair-job-authority.md +++ b/docs/architecture/C1-repair-job-authority.md @@ -282,16 +282,34 @@ branch **to** the protected parent. The boundary must establish that the effecti source identity is the authorized repair ref and the effective target identity is the protected parent ref, reject a symbolic or effective alias that changes that direction, and fail closed if either effective identity cannot be safely -established. - -*Concurrency is not closed by a pre-check.* A resolve-then-check-then-mutate -sequence is **not** an atomic security guarantee: the effective ref or referent -can change between the comparison and the update, so a name observed as an -ordinary repair ref can become symbolic to the protected parent before the -mutation lands. The invariant must be enforced **at the actual mutation/receiving -boundary**, by a mechanism whose semantics prevent an unchecked identity change -between comparison and update — not by an earlier client-side observation this -boundary later trusts. +established. Because this operation performs no ref update to guard, the boundary +that consumes these identities is the change-request/provider creation — or +update — request itself, and the established source and target identities must be +**bound through to that provider request**: the provider must create the request +from exactly the authorized effective source and target, and must not +independently re-resolve the ref names, derive the source or target from ambient +repository state, or otherwise act on an identity that has changed since it was +established. If that authorized source-to-target relationship cannot be maintained +through to the provider request — because an effective identity has changed, or +cannot be safely re-established at that boundary — the boundary must fail closed +and create no change request. + +*Concurrency is not closed by a pre-check.* A resolve-then-check-then-act +sequence is **not** an atomic security guarantee: an effective ref or referent can +change between the comparison and the moment the identity is consumed, so a name +observed as an ordinary repair ref can become symbolic to the protected parent — +or a target can cease to denote it — after the check and before the act. The +invariant must be enforced **at the actual trusted execution boundary that +consumes each identity, not only where a ref is mutated**, by a mechanism whose +semantics prevent an unchecked identity change between the comparison and that +consumption — not by an earlier client-side observation the boundary later trusts. +That consuming boundary differs by operation and the obligation is identical at +each: for `repair.commit` it is the commit mutation boundary, for `repair.push` +the push receiving/mutation boundary, and for `repair.change_request` — which +mutates no ref — the change-request/provider creation boundary at which the source +and target identities are actually consumed. An operation whose authorized +effective-identity relationship cannot be held through to its consuming boundary +must fail closed. This document states the required invariant, not an implementation: it names no git command, lock, or transaction mechanism, and it claims no more atomicity than @@ -537,9 +555,11 @@ touch, about which ref a commit or push would actually advance, or about the effective direction of a change request, so a permit must never be read as proof that repository-level ref identity, the effective mutation target, or the change-request direction is safe. The trusted execution boundary that acts on a -permit binds the effective target, resolves the effective identity, enforces it at -the mutation/receiving boundary, and fails closed; see *What canonical ref names -do and do not prove* above. +permit binds the effective identity, resolves it, and enforces it at the boundary +that actually consumes that identity — the mutation/receiving boundary for a +`repair.commit` or `repair.push`, and the change-request/provider creation boundary +for a `repair.change_request` — and fails closed; see *What canonical ref names do +and do not prove* above. ### Single use diff --git a/src/domain/repair-job.ts b/src/domain/repair-job.ts index c8ffd1a..a1a5e31 100644 --- a/src/domain/repair-job.ts +++ b/src/domain/repair-job.ts @@ -529,11 +529,19 @@ function endsWithDotLockSuffix(value: string, start: number, end: number): boole * effective destination ref for a push. It must **fail closed** — refusing the * operation — if that effective mutation target is or dereferences to the * protected parent, if it changes between comparison and update, or if it cannot - * be safely established; a resolve-then-mutate pre-check is not itself atomic, so - * the invariant is enforced at the mutation/receiving boundary. That refusal is + * be safely established; a resolve-then-act pre-check is not itself atomic, so the + * invariant is enforced at the boundary that actually consumes each identity — the + * mutation/receiving boundary for a commit or push, and the + * change-request/provider creation boundary for a change request. That refusal is * bound to the operand's role rather than being a blanket ban on the protected * parent's identity: a `repair.change_request` `targetRef` is *required* to reach - * the protected parent ref, and fails closed when it reaches anything else. + * the protected parent ref, and fails closed when it reaches anything else. Because + * that operation mutates no ref, the identity it consumes is bound at its provider + * create/update request: the effective source must remain the authorized repair ref + * and the effective target the protected parent ref through to that request, the + * provider must not independently re-resolve those refs or derive them from ambient + * repository state, and the boundary must fail closed if that relationship cannot be + * maintained there. * Nothing here acquires git invocation, filesystem access, a subprocess, or * network to decide it. See `docs/architecture/C1-repair-job-authority.md`, * "What canonical ref names do and do not prove". From 7810d6e337e3b664526bb8017f0f80e12ad2b312 Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Thu, 20 Aug 2026 19:43:09 +0200 Subject: [PATCH 11/15] docs: allow identity-preserving provider resolution Co-Authored-By: Claude Opus 4.8 --- docs/architecture/C1-repair-job-authority.md | 20 +++++++++++++------- src/domain/repair-job.ts | 9 +++++---- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/docs/architecture/C1-repair-job-authority.md b/docs/architecture/C1-repair-job-authority.md index 51b16c8..7d07781 100644 --- a/docs/architecture/C1-repair-job-authority.md +++ b/docs/architecture/C1-repair-job-authority.md @@ -286,13 +286,19 @@ established. Because this operation performs no ref update to guard, the boundar that consumes these identities is the change-request/provider creation — or update — request itself, and the established source and target identities must be **bound through to that provider request**: the provider must create the request -from exactly the authorized effective source and target, and must not -independently re-resolve the ref names, derive the source or target from ambient -repository state, or otherwise act on an identity that has changed since it was -established. If that authorized source-to-target relationship cannot be maintained -through to the provider request — because an effective identity has changed, or -cannot be safely re-established at that boundary — the boundary must fail closed -and create no change request. +from exactly the authorized effective source and target. Provider-side resolution +of the supplied ref names is not itself forbidden — a create/update API may have +to resolve the source and target names against its own authoritative repository +state — but it must yield exactly those authorized effective identities: it must +not let re-resolution, ambient repository state, or any substitution cause the +request to be created from, or to consume, a **materially different** effective +source or target than the one authorized. Resolution that preserves the exact +authorized source-to-target relationship conforms; resolution that would consume +a materially different effective identity does not. If that authorized +source-to-target relationship cannot be maintained through to the provider +request — because an effective identity has changed, cannot be safely +re-established, or cannot be shown equivalent to the authorized one at that +boundary — the boundary must fail closed and create no change request. *Concurrency is not closed by a pre-check.* A resolve-then-check-then-act sequence is **not** an atomic security guarantee: an effective ref or referent can diff --git a/src/domain/repair-job.ts b/src/domain/repair-job.ts index a1a5e31..8287b5b 100644 --- a/src/domain/repair-job.ts +++ b/src/domain/repair-job.ts @@ -538,10 +538,11 @@ function endsWithDotLockSuffix(value: string, start: number, end: number): boole * the protected parent ref, and fails closed when it reaches anything else. Because * that operation mutates no ref, the identity it consumes is bound at its provider * create/update request: the effective source must remain the authorized repair ref - * and the effective target the protected parent ref through to that request, the - * provider must not independently re-resolve those refs or derive them from ambient - * repository state, and the boundary must fail closed if that relationship cannot be - * maintained there. + * and the effective target the protected parent ref through to that request. The + * provider may resolve those ref names at its own boundary, but must not let + * re-resolution or ambient repository state substitute a materially different + * effective identity for either end, and the boundary must fail closed if the + * authorized relationship cannot be maintained or shown equivalent there. * Nothing here acquires git invocation, filesystem access, a subprocess, or * network to decide it. See `docs/architecture/C1-repair-job-authority.md`, * "What canonical ref names do and do not prove". From 077933d3a3df574c8a40c8074e0c2d918445cced Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Thu, 20 Aug 2026 22:20:47 +0200 Subject: [PATCH 12/15] C1: bind repair.push source to authorized ref --- docs/architecture/C1-repair-job-authority.md | 22 ++++++++++++++------ src/domain/repair-job.ts | 6 ++++-- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/docs/architecture/C1-repair-job-authority.md b/docs/architecture/C1-repair-job-authority.md index 7d07781..bc9fabc 100644 --- a/docs/architecture/C1-repair-job-authority.md +++ b/docs/architecture/C1-repair-job-authority.md @@ -267,12 +267,22 @@ mutation will advance, and the boundary must bind the two before it acts: The boundary must bind that effective `HEAD` referent to the authorized repair ref and refuse to commit if the worktree is detached, attached to the protected parent, attached to any other ref, or its safe binding cannot be established. -- **`repair.push`.** The authorized repair ref identifies a source, not a - destination. The boundary must bind the push's **effective destination ref** to - the authorized repair ref and must not let a caller-selected destination refspec - redirect the push; the receiving/mutation side must fail closed if the effective - destination is the protected parent or cannot be proven to be the authorized - repair ref. +- **`repair.push`.** A push carries both a source and a destination ref, and the + authorized repair ref governs **both**. The boundary must bind the push's + **effective source ref** and its **effective destination ref** — each by its + effective ref-name referent, not by commit-object identity — to the authorized + repair ref, and must not let a caller-selected source or destination refspec + redirect either half. The source must be **present**: an absent source, the + deletion refspec `:refs/heads/…`, is not a `repair.push` at all but a + `branch.delete`, which is denied, so a destination that still names the repair + ref does not make it authorized. No alternate branch, tag, or commit-ish may + stand in for the authorized repair ref on either half. The receiving/mutation + side must fail closed if either effective half is the protected parent, is not + provably the authorized repair ref, or ceases to be between the check and the + push — the authorized source-to-destination relationship must hold through to + that consuming boundary, not only at an earlier pre-check. An ordinary + `refs/heads/repair:refs/heads/repair` push, whose effective source and + destination are both the authorized repair ref, remains authorized. *Operands that set direction without mutating a ref.* The obligation is not limited to ref-mutating operations. `repair.change_request` mutates no ref, but diff --git a/src/domain/repair-job.ts b/src/domain/repair-job.ts index 8287b5b..8c726d0 100644 --- a/src/domain/repair-job.ts +++ b/src/domain/repair-job.ts @@ -525,8 +525,10 @@ function endsWithDotLockSuffix(value: string, start: number, end: number): boole * actual repository — the terminal ref reached through a symbolic-ref chain, not * commit-object identity, since a fresh repair branch may legitimately share the * protected parent's commit OID — and bind the effective mutation target it will - * actually advance: the worktree's effective `HEAD` referent for a commit, and the - * effective destination ref for a push. It must **fail closed** — refusing the + * actually advance: the worktree's effective `HEAD` referent for a commit, and, + * for a push, both the effective source and destination refs — each the authorized + * repair ref, so an absent (deletion) or redirected source is refused. It must + * **fail closed** — refusing the * operation — if that effective mutation target is or dereferences to the * protected parent, if it changes between comparison and update, or if it cannot * be safely established; a resolve-then-act pre-check is not itself atomic, so the From 6812607a81dda46e3cf5ca3c3105b9bfe7fa6322 Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Thu, 20 Aug 2026 23:42:38 +0200 Subject: [PATCH 13/15] C1: distinguish repair.push source role Co-Authored-By: Claude Opus 4.8 --- src/domain/repair-job.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/domain/repair-job.ts b/src/domain/repair-job.ts index 8c726d0..272885b 100644 --- a/src/domain/repair-job.ts +++ b/src/domain/repair-job.ts @@ -524,12 +524,15 @@ function endsWithDotLockSuffix(value: string, start: number, end: number): boole * records, resolve the requested ref's effective ref-name referent against the * actual repository — the terminal ref reached through a symbolic-ref chain, not * commit-object identity, since a fresh repair branch may legitimately share the - * protected parent's commit OID — and bind the effective mutation target it will - * actually advance: the worktree's effective `HEAD` referent for a commit, and, - * for a push, both the effective source and destination refs — each the authorized + * protected parent's commit OID — and bind each effective identity the operation + * acts on to the authorized repair ref. A commit advances one mutation target: the + * worktree's effective `HEAD` referent, the ref it will actually advance. A push + * binds two effective identities in distinct roles — its destination ref, the + * receiving/mutation target the push advances, and its source ref, the input the + * push consumes to select what is sent — each of which must be the authorized * repair ref, so an absent (deletion) or redirected source is refused. It must * **fail closed** — refusing the - * operation — if that effective mutation target is or dereferences to the + * operation — if any such effective identity is or dereferences to the * protected parent, if it changes between comparison and update, or if it cannot * be safely established; a resolve-then-act pre-check is not itself atomic, so the * invariant is enforced at the boundary that actually consumes each identity — the From 467ccc39b7c497c904aa527f9d3d50940d2593c7 Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Fri, 21 Aug 2026 03:06:31 +0200 Subject: [PATCH 14/15] C1-A04 clarify effective repository ref identity Co-Authored-By: Claude Opus 4.8 --- docs/architecture/C1-repair-job-authority.md | 10 +++++++++- src/domain/repair-job.ts | 18 +++++++++++++----- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/docs/architecture/C1-repair-job-authority.md b/docs/architecture/C1-repair-job-authority.md index bc9fabc..523f8a3 100644 --- a/docs/architecture/C1-repair-job-authority.md +++ b/docs/architecture/C1-repair-job-authority.md @@ -251,7 +251,15 @@ commit, so distinct commit OIDs are neither necessary nor sufficient: two different branch refs may share one commit OID, and commit-object equality does not make two refs the same authority target. The boundary must therefore compare effective ref-name referents, and must not rest the check on whether two refs -currently resolve to the same commit. What that comparison must *yield* is fixed +currently resolve to the same commit. A terminal ref-name *spelling* is not by +itself repository ref identity: where a repository applies its own ref-identity +semantics — for instance a case-insensitive ref store under which +`refs/heads/Main` and `refs/heads/main` are one ref — two terminal names that are +not equal strings may still be the same repository ref, so the boundary must +decide whether two effective referents are the same or distinct under that +repository's actual ref-identity semantics rather than by terminal-name string +(in)equality alone, and must fail closed wherever the required distinctness cannot +be safely proven under those semantics. What that comparison must *yield* is fixed by the operand's role: for an operand whose required identity is the authorized repair ref, a symbolic or effective ref-name identity that aliases the protected parent must be detected and rejected; for the one operand whose required identity diff --git a/src/domain/repair-job.ts b/src/domain/repair-job.ts index 272885b..0705e34 100644 --- a/src/domain/repair-job.ts +++ b/src/domain/repair-job.ts @@ -525,7 +525,14 @@ function endsWithDotLockSuffix(value: string, start: number, end: number): boole * actual repository — the terminal ref reached through a symbolic-ref chain, not * commit-object identity, since a fresh repair branch may legitimately share the * protected parent's commit OID — and bind each effective identity the operation - * acts on to the authorized repair ref. A commit advances one mutation target: the + * acts on to the authorized repair ref. A terminal ref-name spelling is not by + * itself repository ref identity: where a repository applies its own ref-identity + * semantics — for instance a case-insensitive ref store treating `refs/heads/Main` + * and `refs/heads/main` as one ref — terminal names that are not equal strings may + * still be the same repository ref, so the boundary must judge sameness or + * distinctness under that repository's actual ref-identity semantics rather than by + * terminal-name string (in)equality alone, and fail closed where the required + * distinctness cannot be safely proven. A commit advances one mutation target: the * worktree's effective `HEAD` referent, the ref it will actually advance. A push * binds two effective identities in distinct roles — its destination ref, the * receiving/mutation target the push advances, and its source ref, the input the @@ -622,10 +629,11 @@ export function readCanonicalBranchRef(value: unknown): string | null { * compares strings and resolves nothing, so a canonical name that is a symbolic * ref to the other still answers `false` here, and — since two different branch * refs may legitimately share one commit object — commit-object equality is not - * the question either. Repository-resolved identity, meaning the effective - * ref-name referent reached by resolving a symbolic-ref chain, is the later - * trusted repository/Git execution boundary's to establish; see - * {@link readCanonicalBranchRef}. + * the question either. Repository-resolved identity — whether the effective + * referents reached by resolving symbolic-ref chains are the same or distinct + * under the repository's own ref-identity semantics, which a terminal ref-name + * spelling alone does not settle — is the later trusted repository/Git execution + * boundary's to establish; see {@link readCanonicalBranchRef}. */ function mayDenoteSameBranchRef(left: string, right: string): boolean { if (left === right) { From f8eb259664fd2ae15eb1792daedd41665e63ced7 Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Fri, 21 Aug 2026 13:51:44 +0200 Subject: [PATCH 15/15] fix(c1): snapshot trusted state before hostile reads --- src/domain/execution-permit.ts | 12 +++-- src/domain/job-authorization.ts | 7 ++- tests/domain/execution-permit.test.ts | 53 +++++++++++++++++++ .../job-authorization-invariants.test.ts | 30 +++++++++++ 4 files changed, 97 insertions(+), 5 deletions(-) diff --git a/src/domain/execution-permit.ts b/src/domain/execution-permit.ts index 4abfdb8..0c529a3 100644 --- a/src/domain/execution-permit.ts +++ b/src/domain/execution-permit.ts @@ -398,6 +398,14 @@ export function operatorMergeAuthorizes( return false; } + // The supplied target is captured *before* any candidate property is read. + // Reading the untrusted candidate runs its own getters and Proxy traps, which + // could otherwise mutate the still-live target before its fields are captured + // and make a stale candidate match a target it was rewritten to fit. + const targetRepositoryId = readExactIdentifier(readOwnProperty(targetRecord, 'repositoryId')); + const targetPullRequestId = readExactIdentifier(readOwnProperty(targetRecord, 'pullRequestId')); + const targetHeadSha = readExactIdentifier(readOwnProperty(targetRecord, 'currentHeadSha')); + const authorizationId = readExactIdentifier(readOwnProperty(record, 'authorizationId')); const operatorId = readExactIdentifier(readOwnProperty(record, 'operatorId')); const repositoryId = readExactIdentifier(readOwnProperty(record, 'repositoryId')); @@ -406,10 +414,6 @@ export function operatorMergeAuthorizes( const authorizedAt = readExactIdentifier(readOwnProperty(record, 'authorizedAt')); const singleUse = readOwnProperty(record, 'singleUse'); - const targetRepositoryId = readExactIdentifier(readOwnProperty(targetRecord, 'repositoryId')); - const targetPullRequestId = readExactIdentifier(readOwnProperty(targetRecord, 'pullRequestId')); - const targetHeadSha = readExactIdentifier(readOwnProperty(targetRecord, 'currentHeadSha')); - if ( authorizationId === null || operatorId === null || diff --git a/src/domain/job-authorization.ts b/src/domain/job-authorization.ts index dcb7e57..a55704e 100644 --- a/src/domain/job-authorization.ts +++ b/src/domain/job-authorization.ts @@ -349,8 +349,13 @@ export function authorizeJobOperation( job: RepairJobAuthorization, request: JobOperationRequest, ): JobAuthorizationDecision { - const operation = readJobOperation(request); + // The trusted job is snapshotted into a frozen copy *before* the untrusted + // request is read. Reading the request runs its own getters and Proxy traps, + // which could otherwise mutate the still-live trusted job before it is + // captured; taking the snapshot first means every later check reads only the + // frozen `snapshot`, never a value request-side code could still change. const jobRead = readRepairJobAuthorization(job); + const operation = readJobOperation(request); const snapshot = jobRead.snapshot; const kind: JobOperation = operation.operation; diff --git a/tests/domain/execution-permit.test.ts b/tests/domain/execution-permit.test.ts index fc63a6f..9333c61 100644 --- a/tests/domain/execution-permit.test.ts +++ b/tests/domain/execution-permit.test.ts @@ -3,9 +3,12 @@ import { describe, expect, it } from 'vitest'; import { authorizeJobOperation, JOB_AUTHORIZATION, + operatorMergeAuthorizes, permitAuthorizes, type ExecutionPermit, type JobOperationRequest, + type MergeTarget, + type OperatorMergeAuthorization, type RepairJobAuthorization, } from '../../src/domain/index.js'; import { @@ -14,13 +17,16 @@ import { buildJob, buildPush, buildRequest, + HEAD_A, HEAD_B, JOB_B, NON_OBJECTS, + PARENT_PR_A, PARENT_PR_B, PARENT_REF, REPAIR_BRANCH, REPAIR_WORKTREE, + REPO_A, REPO_B, SECOND_AUTHORIZED_PATH, throwingRecord, @@ -407,3 +413,50 @@ describe('permit identity cannot be collided by operand content', () => { expect(withStowaway.operands.commandClass).toBeNull(); }); }); + +describe('the merge target is captured before the candidate is read', () => { + const buildTarget = (): MergeTarget => ({ + repositoryId: REPO_A, + pullRequestId: PARENT_PR_A, + currentHeadSha: HEAD_A, + }); + const buildAuthorization = (): OperatorMergeAuthorization => ({ + authorizationId: 'auth-0001', + operatorId: 'operator-1', + repositoryId: REPO_A, + pullRequestId: PARENT_PR_A, + // A stale/wrong HEAD: does not match the target's authoritative SHA. + headSha: HEAD_B, + authorizedAt: '2026-01-01T00:00:00Z', + singleUse: true, + }); + + it('rejects a candidate whose HEAD SHA does not match the supplied target', () => { + expect(operatorMergeAuthorizes(buildAuthorization(), buildTarget())).toBe(false); + }); + + it('a candidate getter cannot rewrite the supplied target to match itself', () => { + // The candidate carries a getter that, while the candidate is being read, + // rewrites the still-live target's HEAD to the candidate's stale SHA. If the + // target were captured *after* the candidate were read, that mutation would + // turn a false into a true. + let getterRan = false; + const target = buildTarget(); // currentHeadSha === HEAD_A (authoritative) + const hostile = { + ...buildAuthorization(), // headSha === HEAD_B (stale) + get authorizationId() { + getterRan = true; + (target as { currentHeadSha: string }).currentHeadSha = HEAD_B; + return 'auth-0001'; + }, + } as unknown as OperatorMergeAuthorization; + + const result = operatorMergeAuthorizes(hostile, target); + + // The getter must actually have executed, or the test proves nothing. + expect(getterRan).toBe(true); + // The authoritative target SHA was already captured, so the stale candidate + // never matches. + expect(result).toBe(false); + }); +}); diff --git a/tests/domain/job-authorization-invariants.test.ts b/tests/domain/job-authorization-invariants.test.ts index 968bdbc..854c435 100644 --- a/tests/domain/job-authorization-invariants.test.ts +++ b/tests/domain/job-authorization-invariants.test.ts @@ -690,6 +690,36 @@ describe('a value read twice cannot differ between validation and use', () => { expect(after.decision).toBe(JOB_AUTHORIZATION.ALLOW_ONCE); expect(Object.isFrozen(before.invalidJobFields)).toBe(true); }); + + it('snapshots the trusted job before the hostile request is read', () => { + // A `repair.push` naming the protected parent ref. Honestly evaluated this + // is a protected-ref mutation and must be denied. The request carries a + // getter that, while the request is being read, mutates the still-live + // trusted job so the protected ref becomes the repair branch. If the job + // were snapshotted *after* the request were read, that mutation would flip + // DENY into ALLOW_ONCE and issue a permit to push the protected branch. + let getterRan = false; + const job = buildJob(); + const hostile = { + ...buildPush({ ref: PARENT_REF }), + get operation() { + getterRan = true; + (job as { repairBranch: string }).repairBranch = PARENT_REF; + (job as { protectedParentRef: string }).protectedParentRef = REPAIR_BRANCH; + return JOB_OPERATION.REPAIR_PUSH; + }, + } as unknown as JobOperationRequest; + + const decision = authorizeJobOperation(job, hostile); + + // The getter must actually have executed, or the test proves nothing. + expect(getterRan).toBe(true); + // The trusted job was already captured, so the mutation changed no authority. + expect(decision.decision).toBe(JOB_AUTHORIZATION.DENY); + expect(decision.reason).toBe(JOB_AUTHORIZATION_REASON.PROTECTED_REF_MUTATION); + expect(decision.permit).toBeNull(); + expect(decision.mayExecuteOnce).toBe(false); + }); }); describe('prototype pollution and inherited properties create no authority', () => {