Skip to content

feat(runner-policy): register link-check and pulumi runner-input contracts - #156

Merged
kyle-sexton merged 3 commits into
mainfrom
feat/runner-policy-register-reusable-runner-inputs
Jul 16, 2026
Merged

feat(runner-policy): register link-check and pulumi runner-input contracts#156
kyle-sexton merged 3 commits into
mainfrom
feat/runner-policy-register-reusable-runner-inputs

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Register runner-input contracts in components/runner-policy/policy.json for two floor-conversion reusables, both pinning ci-workflows PR #124's squash-merge commit 3dfb18452a8c6059a22e62456390d84feb10b42f (the reviewed source that carries the governed runner input):

  • link-check.ymlallowedInputs: ["runner", "args"], allowedCallerPermissions: {contents: read, issues: write}.
  • pulumi-version-drift-check.ymlallowedInputs: ["runner"], allowedCallerPermissions: {contents: read, issues: write}.

Each carries allowedCallerPermissions because its rolling tracking-issue lane writes issues through the caller GITHUB_TOKEN; the waiver is honored only while the call is genuinely selector-routed, and the reusables' own permissions are unchanged. This also corrects the now-false README claim that the Pulumi version-drift monitor "remains fixed to explicit GitHub-hosted images," adds a review note for both contracts, and extends the runner-policy golden fixture (runner-policy.test.mjs) to assert them. hostedExceptionReasons is unchanged — enum-tightening is the separate last wave and must not land before every consumer has dropped its floor exceptions.

Why

Wave 2 of the private-repo hosted-floor elimination (melodic-software/github-iac#78, decision of record 2026-07-16). ci-workflows PR #124 (Wave 1) added the governed runner input to these reusables' source; this PR registers the reviewed contracts so each private consumer (Wave 3) can add a select-runner job, route the reusable to the fleet, and drop its hosted-control-plane / privileged-control-plane exception. policy.json is the upstream source of truth synced to five consumers, so the contract belongs here.

Deferred — standards-sync

The third floor-conversion reusable, standards-sync.yml, is intentionally not registered here. Its caller GITHUB_TOKEN is read-only (sync.yml declares permissions: contents: read; all writes go through a minted App token), but a runner-input contract that passes secrets must carry allowedCallerPermissions, which the validator requires to include at least one write (runner-policy.mjs:186-198). There is no honest caller write to declare, so registering it would bake a least-privilege inaccuracy into the managed contract. This is a contract-model gap (the secrets waiver is coupled to a write requirement) routed to the runner-policy component owner for a "reviewed secrets + read-only caller" contract shape; standards-sync keeps its hosted-only contract until that lands.

Test plan

Related

No linked standards issue. Cross-repo references:

🤖 Generated with Claude Code

https://claude.ai/code/session_01K3QehVwmWzkBLpKokNCkkt

@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 24s —— View job


Code Review

  • Gather context from PR description and changed files
  • Review policy.json changes
  • Review runner-policy.test.mjs changes
  • Review README.md changes
  • Post final review

Overall: Clean, minimal, well-structured draft PR. The contracts are correctly shaped, the golden assertions match the policy exactly, and the README fix is accurate. The only blocker is the placeholder SHA, which is already tracked in the finalize step.


policy.json

link-check.yml@feedface… (L170–L179) and pulumi-version-drift-check.yml@feedface… (L180–L189):

  • Routing shape is correct for both: runner-input with runnerInput: "runner", empty allowedSecrets.
  • link-check correctly retains the existing args input alongside runner.
  • pulumi-version-drift-check takes only runner — correct, matches its existing zero-input hosted-only contract.
  • allowedCallerPermissions: { contents: "read", issues: "write" } satisfies the runtime constraint at runner-policy.mjs:186–191 (at least one write scope required).
  • The earlier hosted-only contracts for both workflows remain registered (L88–L98, L118–L123), as required for ordered consumer migration.
  • feedfacefeedfacefeedfacefeedfacefeedface is a syntactically valid 40-character hex SHA and passes format validation. Must be replaced before merge per the finalize step.

runner-policy.test.mjs

WAVE1_FLOOR_CONVERSION_SHA constant (L31–L36): The PLACEHOLDER comment is clear and the lockstep instruction ("Keep in lockstep with the matching keys in policy.json") is the right guidance.

Golden assertions (L1357–L1370): Both assert.deepEqual calls exactly match the corresponding policy.json entries, including field order. Placed in the right existing snapshot test.

One note on the comment at L31: it says "all three floor-conversion reusables share that single SHA" — this is accurate (referring to the single PR #124 merge SHA), though only two are registered here. The deferral of standards-sync is clearly explained in the PR body and doesn't need elaboration in the comment.

README.md

The removed lines at old L246–L249 previously claimed Pulumi version-drift monitor "remain[s] fixed to explicit GitHub-hosted images" — removing this is correct.

The new paragraph at L262–L271 accurately documents the floor conversion, the placeholder SHA, the allowedCallerPermissions rationale, and the migration order. Prose is consistent with the existing review-note style.


Pre-merge checklist (already documented in the PR body, confirming here):

  1. Replace feedfacefeedfacefeedfacefeedfacefeedface in all 4 occurrences: policy.json ×2, runner-policy.test.mjs constant, README.md review note.
  2. Re-run npm run test:runner-policy and npm run lint:runner-policy after replacement.

…racts

Register runner-input contracts for the link-check and Pulumi version-drift
reusables so strict private consumers can route these privileged scheduled
maintenance lanes through the governed selector instead of a fixed hosted
image. Each names an exact allowedCallerPermissions of contents:read plus
issues:write -- the narrow write-capable caller token its rolling
tracking-issue lane needs -- honored only while genuinely selector-routed;
the reusables' own permissions are unchanged.

Wave 2 of the private-repo hosted-floor elimination
(melodic-software/github-iac#78), consuming the reusable-source runner inputs
added in ci-workflows PR #124.

The contract SHAs are a `feedface...` placeholder pending that PR's
squash-merge. Finalize by replacing that value in policy.json, the
runner-policy test constant, and the README review note with the merged
main commit SHA (all three reusables share it).

standards-sync is intentionally deferred: its App-token caller is read-only,
so runner-input's write-requiring allowedCallerPermissions waiver does not fit
cleanly, and its registration awaits a contract-model decision.
hostedExceptionReasons is unchanged; enum-tightening remains the separate last
wave.

Update the runner-policy golden fixture to assert both new contracts and
adjust the README review note and fixed-hosted list.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K3QehVwmWzkBLpKokNCkkt
@kyle-sexton
kyle-sexton force-pushed the feat/runner-policy-register-reusable-runner-inputs branch from 74f99cf to d3692ba Compare July 16, 2026 20:13
@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 29s —— View job


Code Review

  • Gather context from PR description and changed files
  • Check for REVIEW.md criteria
  • Review policy.json changes
  • Review runner-policy.test.mjs changes
  • Review README.md changes
  • Post final review

Overall: Correct, minimal, and well-structured draft PR. All three files are internally consistent. No secrets, no injection risk, no tenant concerns. The only thing blocking merge is the placeholder SHA replacement already tracked in the PR's finalize step.


policy.jsonL170–L189

Contract shapes pass every validator gate in runner-policy.mjs:

  • allowedCallerPermissions contains issues: "write", satisfying the ≥1 write scope guard at L186–192.
  • allowedSecrets: {} means the per-secret format loop at L193–201 is a no-op — no violation.
  • runnerInput: "runner" appears in each contract's allowedInputs, satisfying the runnerInput-must-be-in-allowedInputs guard at L174–183.
  • link-check retains "args" in allowedInputs — correct alongside runner.
  • pulumi-version-drift-check takes only ["runner"] — matches its zero-input hosted-only contract.
  • feedfacefeedfacefeedfacefeedfacefeedface is 40 valid hex characters; format validation passes. Must be replaced before merge per the finalize step.
  • Prior hosted-only contracts for both workflows remain registered (L88–L98, L118–L123), preserving ordered consumer migration.

runner-policy.test.mjsL31–L35, L1356–L1369

Golden assertions operate against BASE_POLICY.approvedReusableWorkflowContracts (the raw JSON), which is the correct surface for pinning production shapes. Both assert.deepEqual calls exactly match the corresponding policy.json entries field-for-field.

The WAVE1_FLOOR_CONVERSION_SHA constant is used as both the JSON key suffix and the test lookup key, so the tests are structurally self-enforcing: replacing the SHA in policy.json without updating the constant (or vice versa) breaks the test immediately, making the lockstep safe without a process-only reminder.

🟡 Nit: the comment at L31–32 ("not a real commit SHA") accurately flags the placeholder but doesn't tell a reader what to do. A trailing // Replace with the ci-workflows PR #124 merge SHA before merge or a pointer to the finalize step would make it actionable without requiring the PR description. Low priority given that the PR body already documents this.

README.mdL244–L276

The removed sentence previously listed Windows Pester, Docker-dependent scans, and the Pulumi version-drift monitor as fixed-hosted. Removing Docker-dependent scans is also correct: osv-scanner.yml and zizmor.yml are already runner-input contracts (see policy.json L158–168 from prior commits). Only Windows Pester remains fixed.

The new paragraph accurately describes the floor-conversion, the placeholder SHA, the allowedCallerPermissions rationale, and migration order. Prose is consistent with the surrounding review-note style.


Pre-merge checklist (confirming what the PR already tracks):

  1. Replace feedfacefeedfacefeedfacefeedfacefeedface in all 4 occurrences: policy.json ×2, runner-policy.test.mjs constant, README.md review note.
  2. Re-run npm run test:runner-policy and npm run lint:runner-policy after replacement.

Replace the `feedface...` placeholder with ci-workflows PR #124's squash-merge
commit 3dfb18452a8c6059a22e62456390d84feb10b42f, which carries the governed
runner input for all three reusables, in policy.json (both contract keys), the
runner-policy golden-fixture constant, and the README review note. The
registered link-check and pulumi-version-drift-check runner-input contracts now
pin the reviewed reusable source.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K3QehVwmWzkBLpKokNCkkt
@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 47s —— View job


Code Review

  • Gather context from PR description and prior reviews
  • Get current diff (post-finalize SHA replacement)
  • Review policy.json — SHA consistency and contract correctness
  • Review runner-policy.test.mjs — golden assertions match updated policy
  • Review README.md — review note reflects real SHA
  • Post final review

Overall: The finalize commit correctly replaced all four feedface… placeholder occurrences with 3dfb18452a8c6059a22e62456390d84feb10b42f. The contracts are well-formed, the golden assertions are consistent with policy.json, and the SHA format is valid (40 hex chars). Two pieces of surrounding prose were not updated to match the now-finalized state.


policy.jsonL170–L189

SHA substitution is consistent in both keys. All validator gates remain satisfied:

  • allowedCallerPermissions carries issues: "write" (≥1 write scope guard at runner-policy.mjs:186–192).
  • runnerInput: "runner" appears in each contract's allowedInputs.
  • allowedSecrets: {} — no per-secret format loop to trip.
  • Prior hosted-only contracts remain registered at L88–98 and L118–123.

No issues.


runner-policy.test.mjsL31–33

The golden assertions at L1357–L1370 match the policy field-for-field. The SHA substitution in the WAVE1_FLOOR_CONVERSION_SHA constant is correct.

Stale comment (correctness): Lines 31–32 still read:

// Shared placeholder for the reviewed ci-workflows merge commit that carries
// these reusables' runner input; not a real commit SHA.

Both "placeholder" and "not a real commit SHA" are now false — the real squash-merge commit was substituted. A reader seeing the real 40-char SHA next to "not a real commit SHA" will be confused about whether this is final. Fix this →


README.mdL262–L265

Stale parenthetical (correctness): The prose at L263 reads:

`runner-input` at `3dfb18452a8c6059a22e62456390d84feb10b42f` (placeholder pending
the ci-workflows floor-conversion merge; Wave 1 of the private-repo hosted-floor

The finalize commit substituted the SHA but left "(placeholder pending the ci-workflows floor-conversion merge;" in place. The merge has happened — this parenthetical is now false. A reader following up on Wave 3 consumer migrations will see a real SHA alongside a claim that it is still a placeholder. Fix this →


Pre-merge: Both stale-prose items should be fixed before merge — neither blocks the policy from being functionally correct, but both will mislead the next engineer who reads the file after this lands.

@kyle-sexton
kyle-sexton marked this pull request as ready for review July 16, 2026 20:50
@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 41s —— View job


Code Review

  • Gather context from PR description and prior reviews
  • Read current state of changed files
  • Check whether previous review's stale-prose findings were addressed
  • Fix stale comment in runner-policy.test.mjs (L31–32) — fixed locally; see note below
  • Fix stale parenthetical in README.md (L263–264) — fixed locally; see note below
  • Review policy.json changes
  • Review runner-policy.test.mjs changes
  • Review README.md changes
  • Post final review

Overall: Functionally correct. The contracts are well-formed, golden assertions are consistent with policy.json, and the prior hosted-only contracts remain registered for ordered consumer migration. The two stale-prose items from the previous review are still present in the current HEAD — I prepared fixes for both but could not push (write-access restriction). Changes ready to apply are documented below.


policy.jsonL170–L189

Both new contracts are correct:

  • SHA 3dfb18452a8c6059a22e62456390d84feb10b42f is consistent in both keys — 40 valid hex chars, matches the WAVE1_FLOOR_CONVERSION_SHA constant in the test file.
  • allowedCallerPermissions: { contents: "read", issues: "write" } satisfies the ≥1 write-scope guard at runner-policy.mjs:186–192.
  • runnerInput: "runner" appears in each contract's allowedInputs, satisfying the runnerInput-must-be-in-allowedInputs guard at runner-policy.mjs:174–183.
  • link-check retains "args" alongside "runner" — correct for its existing callers.
  • pulumi-version-drift-check takes only ["runner"] — matches its zero-input hosted-only predecessor.
  • Prior hosted-only contracts remain at L88–98 and L118–123, preserving ordered consumer migration.

No issues.


runner-policy.test.mjsL31–35, L1356–L1369

Golden assertions exactly match the corresponding policy.json entries field-for-field. The WAVE1_FLOOR_CONVERSION_SHA constant is used as both the JSON key suffix and the test lookup key — a structurally self-enforcing lockstep.

Stale comment (correctness): Lines 31–32 still read:

// Shared placeholder for the reviewed ci-workflows merge commit that carries
// these reusables' runner input; not a real commit SHA.

3dfb18452a8c6059a22e62456390d84feb10b42f is the real squash-merge SHA — "not a real commit SHA" and "placeholder" are now false. Replace with:

// Squash-merge SHA of ci-workflows PR #124 (Wave 1 floor conversion); all three
// floor-conversion reusables share this single merge commit.

README.mdL262–L271

The removed paragraph correctly excises the now-false claim that the Pulumi version-drift monitor remains fixed to explicit GitHub-hosted images.

Stale parenthetical (correctness): L263–264 reads:

`runner-input` at `3dfb18452a8c6059a22e62456390d84feb10b42f` (placeholder pending
the ci-workflows floor-conversion merge; Wave 1 of the private-repo hosted-floor
elimination, melodic-software/github-iac#78)

The finalize commit substituted the SHA but left "placeholder pending the ci-workflows floor-conversion merge" in place. That merge landed; the parenthetical is now false and will mislead Wave 3 consumers. Replace with:

`runner-input` at `3dfb18452a8c6059a22e62456390d84feb10b42f` (ci-workflows PR #124,
squash-merged 2026-07-16; Wave 1 of the private-repo hosted-floor elimination,
melodic-software/github-iac#78)

Pre-merge: Apply the two prose fixes above. I prepared both as local edits (committed as docs(runner-policy): update stale prose after SHA finalization) but was unable to push due to a write-access restriction. The changes are ready to cherry-pick or re-apply manually.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 53a2e9bfd1

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread components/runner-policy/policy.json
The wave-1 merge SHA is now the reviewed ci-workflows floor-conversion commit,
not a pending placeholder. Correct the README review note and the runner-policy
fixture comment so neither describes 3dfb18452a8c6059a22e62456390d84feb10b42f as
an unresolved placeholder or a non-real SHA now that the contracts allowlist it
as production.

Addresses the Codex review on #156.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K3QehVwmWzkBLpKokNCkkt
@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 43s —— View job


Code Review

  • Gather context from PR description and prior reviews
  • Read current diff (post stale-prose fix commit)
  • Review runner-policy.test.mjs — stale comment resolved
  • Review README.md — stale parenthetical resolved
  • Confirm policy.json still correct
  • Post final review

Overall: The latest commit (2e82ac2 docs(runner-policy): drop stale placeholder wording from finalized SHA) resolves both stale-prose findings from the previous review. The PR is clean and ready to merge.


Latest commit — stale-prose fixes

runner-policy.test.mjs L31–32: Previous comment read "Shared placeholder … not a real commit SHA." Now reads:

// The reviewed ci-workflows floor-conversion merge commit that carries these
// reusables' runner input; shared by both runner-input contracts.

Accurate. ✅

README.md L262–265: Previous prose said "placeholder pending the ci-workflows floor-conversion merge." Now reads:

`runner-input` at `3dfb18452a8c6059a22e62456390d84feb10b42f`, the reviewed
ci-workflows floor-conversion merge (Wave 1 of the private-repo hosted-floor
elimination, melodic-software/github-iac#78), so strict consumers route these …

Accurate — the merge has landed and the SHA is finalized. ✅


Remaining files (unchanged from prior approval)

policy.json L170–189: Both contracts unchanged. SHA 3dfb18452a8c6059a22e62456390d84feb10b42f is consistent across both keys. All validator gates remain satisfied (≥1 write scope, runnerInput in allowedInputs, empty allowedSecrets). Prior hosted-only contracts remain registered at L88–98 and L118–123 for ordered consumer migration. ✅

runner-policy.test.mjs L1356–1369: Golden assertions match policy.json field-for-field. WAVE1_FLOOR_CONVERSION_SHA is used as both the key suffix and test lookup key — a self-enforcing lockstep. ✅


No outstanding findings. All three prior review rounds are resolved.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2e82ac21d9

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread components/runner-policy/policy.json
@kyle-sexton
kyle-sexton merged commit 0b0781e into main Jul 16, 2026
41 checks passed
@kyle-sexton
kyle-sexton deleted the feat/runner-policy-register-reusable-runner-inputs branch July 16, 2026 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant