Fix trusted verifier and bootstrap authorization#124
Conversation
Traviinam
left a comment
There was a problem hiding this comment.
[echo-ui-loop-review bootstrap-pr124 round=1 head=10c62e812f2ae27983f786fa5fdbf2829d1ae058]
COMMENT — 2 findings
RVW-1-1-1 — P1 — High confidence
loops/issue-dev-loop/scripts/lib/github-identity.mjs:1992
Inline comment ID: null
Bootstrap preflight trusts git status plus working-tree lstat. An added symlink under an allowed control path can be committed, replaced locally with a regular file, and hidden using skip-worktree/assume-unchanged. Status is then clean, lstat sees a regular file, and the A-only diff bypasses forbiddenChanges, allowing the committed symlink to be pushed. The preflight also never verifies an isolated linked worktree.
Resolve by rejecting index concealment, verifying linked-worktree isolation, and validating every changed entry’s type/mode from the authorized head’s Git tree. Add concealment and primary-checkout regressions.
RVW-1-1-2 — P2 — High confidence
loops/issue-dev-loop/scripts/lib/bootstrap-authorization.mjs:168
Inline comment ID: null
The documented 24-hour maximum is extended by CLOCK_SKEW_MS. A comment created at 2026-07-27T00:00Z with expiry 2026-07-28T00:05Z is accepted—a 1,445-minute lifetime.
Keep clock skew for comparing GitHub time with local time, but enforce expiresAt <= createdAt + 24h exactly and add a boundary test.
Targeted suites passed: routing 49/49; runtime 58/58. Exact head remained pinned and clean.
|
[echo-ui-loop-review-response bootstrap-pr124 finding=RVW-1-1-1 head=e412fde0191d27f57dd7b4c989912de245b2b70a] Classification: accepted. Fixed in e412fde. Bootstrap preflight now requires a linked worktree, rejects non-H Git index states, validates every changed entry from the authorized head tree as a regular 100644/100755 blob, and verifies tracked filesystem contents and modes against HEAD. Added primary-checkout, skip-worktree symlink-replacement, and assume-unchanged regressions. Evidence: routing tests 49/49 passed; runtime tests 58/58 passed; full pnpm verify passed at the exact head. |
|
[echo-ui-loop-review-response bootstrap-pr124 finding=RVW-1-1-2 head=e412fde0191d27f57dd7b4c989912de245b2b70a] Classification: accepted. Fixed in e412fde. Authorization lifetime is now capped exactly at createdAt + 24 hours; the five-minute clock skew remains only for comparing GitHub creation time with the local current time. Added tests proving exactly 24 hours is accepted and 24 hours plus 1 ms is rejected. Evidence: runtime tests 58/58 passed; full pnpm verify passed at the exact head. |
Traviinam
left a comment
There was a problem hiding this comment.
[echo-ui-loop-review bootstrap-pr124 round=2 head=e412fde0191d27f57dd7b4c989912de245b2b70a]
COMMENT — 3 findings. Round-1 findings are resolved.
RVW-1-2-1 — P1 — High confidence
loops/issue-dev-loop/scripts/lib/github-identity.mjs:254-255,296-303
Inline comment ID: null
Bootstrap preflight validates the authorized SHA, but the eventual hardened push uses the mutable local branch as its source refspec:
refs/heads/<branch>:refs/heads/<branch>
A concurrent untrusted process can move that ref after preflight and before git push resolves it, pushing an unreviewed descendant containing product/control changes despite the exact-head authorization.
Expected resolution: construct the authenticated bootstrap push from authorization.bootstrap.headSha directly, and add a regression that moves the branch after preflight but proves only the authorized commit can be pushed.
RVW-1-2-2 — P2 — High confidence
.github/workflows/issue-dev-loop-evidence.yml:19-21
Inline comment ID: null
The bootstrap verification job still accepts only codex/issue-dev-loop PRs authored by codeacme17. The newly documented flow creates codex/bootstrap-* Draft PRs as Ethandasw; both workflow jobs therefore skip, so privileged control-plane bootstrap PRs receive no CI pnpm verify.
Expected resolution: gate bootstrap verification on the documented codex/bootstrap-*/automation-identity shape and add a structural workflow regression test.
RVW-1-2-3 — P2 — High confidence
loops/issue-dev-loop/scripts/lib/github-identity.mjs:1962-1968,898-912
Inline comment ID: null
Bootstrap mode is not mutation-exclusive. preflightBootstrapMutation ignores operations other than push/PR creation, while the generic API policy still permits arbitrary POSTs to the state-journal issue. Reproduction with bootstrap authorization: assertGitHubCliPolicy(...) accepts gh api repos/codeacme17/echo-ui/issues/105/comments -f body=arbitrary.
Expected resolution: while bootstrap authorization is active, reject every remote mutation except the exact authorized push and Draft-PR creation; retain read-only commands.
|
[echo-ui-loop-review-response bootstrap-pr124 finding=RVW-1-2-1 head=66fc518dad9d16f7595210116363021cefdc4781] Classification: accepted. Fixed in 66fc518. Bootstrap hardening now receives the verified authorization and rewrites the push source to the authorization-bound commit SHA, not the mutable local branch ref. The regression moves the branch after preflight and verifies the executed refspec still uses the authorized SHA. Evidence: routing tests 49/49 passed; full pnpm verify passed at the exact head. |
|
[echo-ui-loop-review-response bootstrap-pr124 finding=RVW-1-2-2 head=66fc518dad9d16f7595210116363021cefdc4781] Classification: accepted. Fixed in 66fc518. The bootstrap evidence job now matches same-repository codex/bootstrap-* PRs authored by Ethandasw and runs pnpm verify. Structural validation binds the configured automation login, branch prefix, same-repository gate, and verification step; negative workflow mutations are rejected. Evidence: runtime tests 58/58 passed; full pnpm verify passed at the exact head. The synchronize event now triggers bootstrap CI for this Draft PR. |
|
[echo-ui-loop-review-response bootstrap-pr124 finding=RVW-1-2-3 head=66fc518dad9d16f7595210116363021cefdc4781] Classification: accepted. Fixed in 66fc518. While bootstrap authorization is active, GitHub policy now retains read-only issue/PR/run/API operations but rejects every mutation except the exact authorized Draft PR creation; Git policy separately permits only the exact authorized branch push. Regression coverage rejects state-journal POST, PR comment, and PR edit mutations. Evidence: routing tests 49/49 passed; full pnpm verify passed at the exact head. |
Summary
Why
The currently installed control plane cannot publish a branch that introduces its own new authorization rule. This owner-authorized bootstrap closes that one-time gap while preserving exact branch, base, head, actor, path, expiry, and Draft-only constraints for future use.
Review repairs
Round 1
Round 2
Verification
Boundaries