Skip to content

Disable checkout credential persistence and scope Pages permissions - #2838

Open
Chris0Jeky wants to merge 2 commits into
mainfrom
issue-2335/checkout-least-privilege
Open

Disable checkout credential persistence and scope Pages permissions#2838
Chris0Jeky wants to merge 2 commits into
mainfrom
issue-2335/checkout-least-privilege

Conversation

@Chris0Jeky

Copy link
Copy Markdown
Owner

Disable credential persistence on the33 remaining checkout steps across29 workflows. These jobs use explicit API/action credentials when publishing and perform no authenticated Git fetch or push after checkout. Restrict Pages write/OIDC permissions to deployment; its build retains contents/read and Pages/read for the existing-site metadata lookup.

Add hosted self-tests for checkout credentials and the reviewed publishing write-grant sites. Update the parked staging workflow digest only for its two credential inputs; triggers, commands and its non-deploying handoff remain unchanged.

Refs #2335.

Validation at f076baa, base edf654e:

  • New contract red:2 failed/2 passed before the workflow fixes. Full Smart CI suite:161 passed afterward.
  • Governance check and27 governance mutation tests passed; action-pin inventory156/156 pinned; diff check passed.
  • Independent Terra high review:SHIP at768bb5852, no HIGH/CRITICAL. The main merge contributes only Chat files; all CI/control files remain byte-identical to that reviewed/tested commit.
  • Broad local node --test scripts/ci/*.test.mjs:343 passed,11 failed,2 skipped. Four failures require absent Actionlint/Bash tool paths; six release-test failures came from selecting the Windows WSL Bash bridge. With BASH_BIN=C:\Program Files\Git\bin\bash.exe, all87 release tests pass. One launcher late-marker cleanup left a synthetic helper alive; its isolated rerun passed1/1. That fixture failure is retained as a separate launcher residual, not called a product regression or dismissed as flaky. No broad-green claim.
  • Required exact-head hosted CI and hosted Actionlint remain pending. No Pages deployment, GitHub Release, GHCR publish or live credential test was manually dispatched.

The Pages read grant follows the pinned configure-pages action's default enablement:false and GitHub's Pages metadata API permission. The static contract intentionally supports the repository's block-mapping YAML shape; it is not a general YAML security parser.

No repository settings, CodeQL enrollment, required-context, runner or canonical-document changes. OUTSTANDING_TASKS.md human decisions remain unchanged. Local raw logs and synthetic failure fixture are preserved in the owned .tmp/issue-2335 pending copy-out before plain worktree removal.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-09-09T17:32:13.206571Z f076baa PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@Chris0Jeky Chris0Jeky moved this from Pending to Review in Taskdeck Execution Sep 9, 2026
@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Evidence cleanup completed:31 files, including raw test logs, pin inventory and the21-file synthetic launcher failure fixture, were copied and SHA-256 verified in .claude/local/milestone-20260909/preserved-2838/. The original owned helper was stopped and its temporary fixture removed after preservation. The clean pushed source worktree was removed with plain git worktree remove; primary unrelated changes remain untouched.

Independent Terra high review is SHIP. The launcher failure and isolated1/1 rerun are tracked on #2561 comment5606088804; Actionlint setup omissions and the87/87 Git-Bash release-test proof remain explicit in the PR. Required34383468694 is still pending; no hosted or deployment success is claimed yet.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

This source head is preserved unchanged in integration PR #2840 (f6a2bc9), together with #2836, #2838 and #2839. Each complete changed subtree matches its reviewed source exactly; composition review is SHIP. Source runs 34382773101 and 34383468694 failed before browser execution because Google's Chrome APT index had a Hash Sum mismatch. The integration's own required hosted gate must pass before merge. This source PR remains open until verified integration delivery; its owning issue is not automatically closed.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

The failed E2E job stopped before tests because Google's Chrome APT index had a Hash Sum mismatch. At 2026-09-09T18:10:42Z, a fresh read of InRelease and Packages.gz returned matching SHA256 233e56de019b57db89238fa7bcc3647718dbbea3a40c2dc1c633a8c8952aa9e9 (1405 bytes). Evidence is in .claude/local/milestone-20260909/apt-index-probe. This establishes the upstream index is now consistent at the observed endpoint; it is not a passing APT or browser check.

With source head and main unchanged, the coordinator requested one failed-job-only retry of this PR's required run. Normal APT integrity checks remain intact. The combined integration2840 separately has an unresolved transcript-source review finding; this source subtree is independent and its original qualifying gate remains required.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Coordination note — restart contract, so this PR is not carrying an implicit "someone will get to it".

Where it stands. Head f076baab1, opened 2026-09-09T17:31Z, base edf654ef9. All 35 checks green at that head; independent Terra high review SHIP at 768bb5852; Codex's Code Review on f076baa came back Failed, which is the review job erroring, not a finding. Its content is not on main: only 9 workflows carry persist-credentials: false there against the 33 checkout steps across 29 workflows this PR touches.

Why it is still open. Integration PR #2840 originally bundled #2836, #2838 and #2839; #2840 was closed and the replacement #2851 landed #2836, #2839 and #2843 on 2026-09-09T22:43Z without this one. So this is the piece of that batch left behind, not a duplicate of anything merged. Its gate is OUTSTANDING_TASKS.md §J.2, which names "the #2335 control-trust test PR" as awaiting the maintainer's review of its final head.

What has to happen before it can merge, in order.

  1. gh pr update-branch — the base has moved a long way (edf654ef9, and main is now past a1f797913). A base change counts as a head change.
  2. One fresh hosted required run at the new head. The 2026-09-09 E2E failure was Google's Chrome APT index Hash Sum mismatch, an upstream condition that a fresh run re-tests on its own.
  3. One fresh-context re-review against the moved base, scoped to whatever main has touched since 768bb5852 in this PR's own files or in the workflows they read — ci-required.yml, reusable-frontend-unit.yml, ci/policy.v1.json and .github/workflows/ci-continuation-observe.yml all changed on 2026-09-10.
  4. The maintainer's review under the ADR-0066 amendment, or an explicit delegation of this PR.

Step 4 is the open one, and the standing rule behind it is now itself a question: OUTSTANDING_TASKS.md §J.3 (added today) discloses that seven ci/continuation-* PRs merged on 2026-09-10 against the same declared gate, and asks the maintainer both to review those post hoc and to settle what binds the next control-plane PR. Whatever that answer is, steps 1 to 3 are owed here regardless, and nothing in this note infers approval or changes this PR's authority.

Not verified by this note: no command was run against this branch, no hosted run was dispatched, and the review at 768bb5852 was not re-read.

Chris0Jeky added a commit that referenced this pull request Sep 10, 2026
…6-09-06

Found by the fresh-context review of #2916, which noticed the same two
stale facts it was correcting in .codex/memories/00_ACTIVE.md also
survive here, in another coordinator-owned agent-facing view.

SC-9 closed on the 2026-09-06 walkthrough q-4 = A; the connector was
reviewing normally when last observed on 2026-09-10. SC-10 closed the
same day with all twelve merged, and the row now carries the distinction
that matters: that delegation covered twelve named PRs only, the
amendment still binds a new control-plane PR, and whether it should is
OUTSTANDING_TASKS section J.3. Parked control-plane PRs are recorded on
J.2, which holds #2838.

The section 1 provenance row for the human-gate table is split so it no
longer dates these two rows to 2026-09-05.
Chris0Jeky added a commit that referenced this pull request Sep 10, 2026
Codex P2 on #2919, and correct on the operational point: a coordinator
reading this row as 'Closed 2026-09-06' could skip open human actions.
SC-10's row is ticked, but section J.1 and J.2 are open rows carrying the
#2772 and #2787 post-hoc merges, J.2 also holds parked #2838, and J.3
carries the 2026-09-08 to 2026-09-10 disclosure. The row now names all
three and says the tick is not the end of it.

The other half of the finding is a cross-PR artifact rather than a defect:
J.3 and J.2's #2838 entry live in #2916, which is unmerged, so a review
against main sees references to sections that do not exist yet. That is
an ordering constraint, not a text problem - #2916 merges first.
@Chris0Jeky

Chris0Jeky commented Sep 10, 2026

Copy link
Copy Markdown
Owner Author

Read-only restart revalidation on 2026-09-10:

Do not run the recorded update-branch step until #2931 lands or releases the path. After it lands, update #2838 to current main and verify that both checkout steps retain persist-credentials: false while the mutation activation smoke remains before Stryker. Then re-run the exact-head hosted gate, fresh control-path review, and ADR-0066 maintainer review.

No #2838 branch or workflow state was changed. Project V2 status was not mutated because Project GraphQL is unavailable in this turn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Review

Development

Successfully merging this pull request may close these issues.

1 participant