feat(runner-policy): classify packages-only write jobs as publication

[kyle-sexton]

No linked issue.

The Wave 4/5 enum tighten (melodic-software/github-iac#157) removes privileged-control-plane, but the validator's category rule required exactly that reason for ANY write-scoped token — leaving a hosted packages: write publication job homeless: its only admissible post-tighten path would move the registry token onto fleet containers, trading hosted artifact provenance for no security gain.

• Rule: permissionHostedRequirement now maps a write map whose only writable scope is packages to the durable publication category. Any additional write scope keeps privileged-control-plane. README documents the boundary.
• First consumer, same change: publish-packages.yml#publish re-reasons privileged-control-plane → publication and stays hosted. sync.yml#sync remains the sole privileged exception here, converting later in the standards-sync runner-input lane.
• Deviation from the chore: retire the canary managed runner label #157 plan comment recorded there next: publish-packages converts by re-reason, not grant.

Verification

• npm run test:runner-policy — 210/210 (added: publication-category pass, wrong-reason fail with the new message, mixed packages+issues stays privileged)
• GITHUB_REPOSITORY=melodic-software/standards npm run lint:runner-policy — passes re-reasoned; the old reason now fails hosted-exception-category
• npm run lint:md — 0 errors

Related

• melodic-software/github-iac#157 (conversion plan), melodic-software/github-iac#78 (epic)

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 298f1a696a

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

[claude]

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

---

PR Review

• Gather context (PR description, changed files)
• Read REVIEW.md for review criteria
• Review runner-policy.mjs changes
• Review runner-policy.test.mjs changes
• Review runner-policy.json changes
• Review README.md changes
• Evaluate Codex P2 finding
• Post final review

---

Summary

The semantic claim is sound — packages: write is registry-publication authority, not repository/org state — and the rule, README, and runner-policy.json changes are well-executed. However, there is a correctness bug in the implementation that the Codex P2 finding correctly identifies, and the missing test coverage for that gap confirms it is undetected.

---

🔴 Important — Early return bypasses environment and credential checks

File: components/runner-policy/runner-policy.mjs

permissionHostedRequirement now returns { reason: "publication" } for packages-only jobs. Inside privilegedHostedRequirement, that result causes an early return at line 2750, before the environment check (lines 2774–2785) and before the localCredentialRequirement / credentialActionUses checks (lines 2787–2820).

Before this PR, the early return was always privileged-control-plane — the most restrictive category — so skipping the downstream checks was harmless: nothing could elevate beyond it. Now that publication can emerge from the permission check, any additional privileged surface (an environment: key, an unapproved secret expression, or a credential-minting action like actions/create-github-app-token) is silently ignored.

Concrete exploit path — this job passes with only a publication exception:

jobs:
  publish:
    permissions:
      packages: write
    environment: production       # should require privileged-control-plane
    runs-on: ubuntu-24.04
    steps:
      - uses: actions/create-github-app-token@v1   # credential-minting; also skipped

The environment check at line 2775 would have returned { reason: "privileged-control-plane" }, but is never reached.

Fix: After permissionHostedRequirement returns publication, do not return immediately. Carry permissionRequirement as a candidate and let the environment/credential checks run. If any of those checks fire (returning privileged-control-plane), return that instead; otherwise fall through and return the publication candidate. Alternatively, perform the environment/credential checks first and only apply the packages: write → publication downgrade if none fired.

Fix this →

---

🔴 Important — Missing tests for the exploitable surface combinations

File: components/runner-policy/runner-policy.test.mjs

The three new tests cover the happy path, the wrong-reason rejection, and the mixed-scopes case. None of them combine packages: write with:

• an environment: declaration (expected: hosted-exception-category with privileged-control-plane)
• an unapproved secret expression in env: or with: (expected: hosted-exception-category)
• a credential-minting action (expected: hosted-exception-category)

Without these, the early-return bug above is invisible to CI.

Fix this →

---

🟡 Nit — README boundary statement is incomplete

File: components/runner-policy/README.md

Any additional write scope keeps the job privileged.

The documented boundary mentions only additional write scope, not environment: declarations or credential-minting actions, which also keep the job in privileged-control-plane. Once the code bug is fixed, the README should state the full boundary: "Any additional write scope, environment: declaration, or credential expression keeps the job privileged."

---

What's working well

• The semantic model is correct and the justification is precise — packages: write is genuinely registry-publication authority.
• runner-policy.json justification is meaningfully improved over the previous boilerplate.
• The mixed-scopes test (packages: write + issues: write stays privileged) is exactly the right boundary guard for the write-scope dimension.
• Commit follows Conventional Commits; the PR description links the upstream conversion plan and records the deviation.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 123d5d513d

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 96ba6ee289

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 616fc7df33

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

[claude]

Claude finished @kyle-sexton's task in 3m 57s —— View job

---

PR Review

• Gather context (PR description, changed files, prior review thread)
• Read REVIEW.md for review criteria
• Read privilegedHostedRequirement and surrounding changes (commits 123d5d5, 96ba6ee, 616fc7d)
• Read auditLocalPermissionFlow — evaluate latest Codex P2 finding
• Review test additions
• Post final review

---

Prior findings resolved

The three findings from the earlier review pass on the current HEAD:

• Early return bypass (🔴 Important, commit 123d5d5): publicationRequirement is now held and only returned after the environment, credential-expression, and credential-minting-action checks all pass. The new tests for environment, credential-minting action, named-secret expression, and GITHUB_TOKEN admission confirm the surfaces are all exercised.
• Missing tests: 7 new tests cover the happy path, wrong-reason rejection, mixed-scopes boundary, every privileged surface that should escalate to privileged-control-plane, and the GITHUB_TOKEN admission. 214/214.
• README gaps: Commits 96ba6ee and 616fc7d propagate the carveout to both later rules (the write-permission rule and the GITHUB_TOKEN step env/with exception) and make the normative boundary self-consistent.

---

🔴 Important — auditLocalPermissionFlow still requires privileged-control-plane for packages-only hosted jobs in local reusables

File: components/runner-policy/runner-policy.mjs

auditLocalPermissionFlow hard-codes privileged-control-plane as the only valid exception reason at line 783:

if (exception.reason !== "privileged-control-plane") {
  // → hosted-exception-category finding
}

permissionCapability (the function it uses to compute the job's effective write-capability) returns only "read-only" or "may-write" — it has no packages-only resolution. So when a fixed-hosted job inside a local reusable declares permissions: { packages: write }, capability is "may-write" and the hostedExecution && capability !== "read-only" branch fires, requiring privileged-control-plane.

This creates an unsatisfiable contradiction for any packages-only publisher that is structured as a local-reusable-internal job whose caller inherits write permissions:

• With publication exception → passes privilegedHostedRequirement (direct audit of the reusable file), fails auditLocalPermissionFlow with hosted-exception-category
• With privileged-control-plane exception → passes auditLocalPermissionFlow, fails privilegedHostedRequirement with hosted-exception-category (requires publication)

Neither exception can satisfy both passes, so the publisher is homeless in the local-reusable case. The README's normative boundary (which says publication is the required category for packages-only write) is accurate for the direct-job path, but contradicts what auditLocalPermissionFlow enforces in the local-reusable path.

The publish-packages.yml#publish consumer is a direct job and is unaffected. But the inconsistency is introduced by this PR (before it, both paths uniformly required privileged-control-plane), and any future publisher structured as local-reusable → fixed-hosted would encounter it.

Fix: auditLocalPermissionFlow needs a packages-only resolution alongside may-write. One approach — a narrower capability value (e.g. "packages-only") returned by permissionCapability when the effective permission map is packages-only write — would let the function accept publication exception for that case and privileged-control-plane for broader write. Alternatively, compute the fine-grained permission set inside auditLocalPermissionFlow before reaching the exception-reason check, mirroring the permissionHostedRequirement logic there.

A test for the local-reusable case (caller with write permissions → local reusable → packages-only fixed-hosted job with publication exception) would need to accompany the fix.

Fix this →

---

What's working well

The fix for the early-return bypass (123d5d5) is clean: publicationRequirement as a held candidate rather than an immediate return is exactly the right pattern, and admitGitHubToken threading it into localCredentialRequirement is the minimal surface to admit the GITHUB_TOKEN while keeping named secrets and credential-minting actions escalatory. The README reconciliation across all three rule paragraphs (510, 589, 612) is thorough. The test suite expansion from 210 to 214 covers all the surfaces the prior review identified.

[claude]

Claude finished @kyle-sexton's task in 3m 39s —— View job

---

PR Review

• Gather context (PR description, changed files, prior review thread)
• Read REVIEW.md for review criteria
• Read runner-policy.mjs — privilegedHostedRequirement, auditLocalPermissionFlow, and related paths
• Evaluate Codex P2 finding on local-reusable carveout
• Post final review

---

Summary

All three prior review findings (early-return bypass, missing GitHub-token README carve-out, inconsistent later workload rules) were correctly addressed in commits 123d5d5, 96ba6ee, and 616fc7d. The implementation and tests are sound for the covered paths.

The latest Codex P2 finding about auditLocalPermissionFlow is valid but architectural — it describes a design gap that is out of scope for this PR. One documentation nit remains to prevent downstream consumer confusion.

---

🟡 Nit — Local-reusable section does not explicitly scope out the packages-only carve-out

File: components/runner-policy/README.md line 598–606

The general workload-job rule (lines 590–596) says a write permission map can use publication when the only write scope is packages. The local-reusable section (lines 598–606) then says "A fixed hosted called job may inherit write only with its own privileged-control-plane exception" — with no explicit statement that the packages-only carve-out does NOT apply here. A downstream consumer trying to put a packages-only publisher inside a local reusable will follow the general rule, choose publication, and encounter a hosted-exception-category failure that the README doesn't predict.

A single clarifying phrase — e.g. "A fixed hosted called job may inherit write only with its own privileged-control-plane exception; the packages-only publication carve-out applies only to direct workload job permissions, not to inherited write capability" — would align the documentation with the code without requiring any code change.

---

On the Codex P2 finding (auditLocalPermissionFlow carryover)

The finding is architecturally correct: auditLocalPermissionFlow at line 783 hard-codes a privileged-control-plane requirement for any inherited write-capable call, while the new permissionHostedRequirement path allows publication for packages-only direct jobs. This creates a genuine inconsistency.

However, fixing it is not a one-liner. The auditLocalPermissionFlow path receives only the abstract capability ("read-only" / "may-write") from permissionCapability (line 677) — it does not carry the specific write scopes through the call chain. Extending it to grant packages-only inheritance the same publication treatment would require threading scope-specific permission information through the inherited parameter and updating permissionCapability accordingly. That is a non-trivial, self-contained architectural change and a reasonable follow-up.

Critically, the inconsistency is not a security regression: the local-reusable path remains more restrictive than the direct path, not less. No current consumer is affected — publish-packages.yml#publish is a direct job, not a local-reusable called job. Tracking this as a follow-up when privileged-control-plane retirement makes it unavoidable is the appropriate disposition.

---

What's working well

• The early-return fix (123d5d5) correctly defers the publication requirement as a candidate and lets environment/credential checks override it — admitGitHubToken wiring is precise.
• The README additions in 96ba6ee and 616fc7d are accurate within each section and cross-reference the packages-only rule consistently.
• 214 tests with clean dogfood coverage of this repository's own workflows is strong signal the boundary is correctly drawn for the direct-job path.

[claude]

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

---

PR Review

• Gather context (PR description, changed files, prior review thread)
• Read REVIEW.md for review criteria
• Read changed files at HEAD (616fc7d)
• Evaluate outstanding nit from prior review (README local-reusable scoping)
• Evaluate Codex P2 finding on auditLocalPermissionFlow
• Post final review

---

Prior 🔴 findings — resolved

The early-return bypass (round 1) and the README carve-out gaps (rounds 2–3) are correctly addressed in commits 123d5d5, 96ba6ee, and 616fc7d. The implementation is sound for the direct-job path.

---

🟡 Nit — README local-reusable section still missing the carve-out scope

File: components/runner-policy/README.md lines 598–606

This finding was identified in the previous review round and remains open at HEAD. Lines 590–596 now correctly say publication is valid for a packages-only write permission map, but the local-reusable paragraph immediately following (lines 598–606) still says only:

A fixed hosted called job may inherit write only with its own privileged-control-plane exception.

It does not say that the packages-only publication carve-out does not extend here. A consumer who reads the general rule, sets a publication exception on a packages-only publisher inside a local reusable, and then hits auditLocalPermissionFlow's hard-coded privileged-control-plane requirement will get a hosted-exception-category failure with no README explanation.

A single appended clause is sufficient:

A fixed hosted called job may inherit write only with its own privileged-control-plane exception; the packages-only publication carve-out applies only to direct workload job permissions, not to inherited write capability.

Fix this →

---

On the Codex P2 finding (auditLocalPermissionFlow)

No change from the prior review. auditLocalPermissionFlow at line 783 still hard-codes privileged-control-plane, creating a genuine inconsistency for packages-only publishers inside local reusables. This is not a security regression — the local-reusable path remains more restrictive than before, not less — and fixing it requires threading scope-specific permission data through permissionCapability and auditLocalPermissionFlow, which is architectural work beyond this PR's scope. No current consumer is affected. Appropriate as a tracked follow-up when privileged-control-plane retirement forces resolution.

---

What's working well

• The publicationRequirement held-candidate pattern (123d5d5) is clean and correctly defers until all downstream checks pass.
• admitGitHubToken threading is precisely scoped: GITHUB_TOKEN admitted, named secrets and credential-minting actions still escalate.
• .github/runner-policy.json justification is substantively better — provenance reasoning is precise.
• 214/214 tests with dogfood coverage of this repository's own workflows.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dcd56bb538

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

[claude]

Claude finished @kyle-sexton's task in 5m 18s —— View job

---

PR Review

• Gather context (PR description, changed files, prior review thread)
• Read REVIEW.md for review criteria
• Read runner-policy.mjs — privilegedHostedRequirement, auditLocalPermissionFlow, hosted-reusable path
• Evaluate Codex P2 finding: hosted-reusable credential scan bypass (runner-policy.mjs:2796)
• Evaluate Codex P2 finding: README local-reusable guidance reconciliation
• Post final review

---

Prior findings — resolved

All findings from rounds 1–4 are correctly addressed:

• Early-return bypass (round 1 🔴): publicationRequirement held as candidate, all surface checks still run.
• README GITHUB_TOKEN carve-out (round 2 🔴): documented in the publication rule paragraph.
• Later workload rules (round 3 🔴): both the write-permission sentence and the GITHUB_TOKEN step env/with exception carry the packages-only carve-out.
• auditLocalPermissionFlow inconsistency (round 4 🔴): commit dcd56bb now calls privilegedHostedRequirement directly to determine the required reason, so the local-flow pass and the direct audit agree on one exception key.

---

On the Codex P2 finding — hosted-reusable credential scan (runner-policy.mjs:2796)

The finding correctly observes that return publicationRequirement at line 2796 fires before the credential scan. However, this early return for target?.kind === "hosted-reusable" predates this PR and applies to all hosted-reusable jobs, not only packages-only ones — the comment at lines 2791–2794 records why: exact hosted-only secret mappings are governed by approvedReusableWorkflowContracts, not localCredentialRequirement. The only change this PR introduces is that a packages-only write job now returns publication from that path instead of triggering the else if (permissionRequirement) branch at line 2787 (which previously returned privileged-control-plane before ever reaching line 2795). The with input value check gap the finding describes was always present for every hosted-reusable exception level, not something newly bypassed. Not a regression.

---

🟡 Nit — README local-reusable paragraph now contradicts the code

File: components/runner-policy/README.md lines 602–605

After dcd56bb, auditLocalPermissionFlow calls privilegedHostedRequirement to derive the required reason, so a packages-only write job inside a local reusable now needs a publication exception (not privileged-control-plane). The paragraph still reads:

A fixed hosted called job may inherit write only with its own privileged-control-plane exception.

A consumer following this sentence would declare privileged-control-plane and receive hosted-exception-category. The sentence needs the same packages-only qualifier the surrounding paragraphs already carry:

A fixed hosted called job may inherit write only with its own privileged-control-plane exception — or publication when the only write scope is packages, per the packages-only rule above.

Fix this →

I prepared this one-line fix as a local commit (98ef1ca) but could not push due to write-access restrictions.

---

What's working well

• auditLocalPermissionFlow calling privilegedHostedRequirement (dcd56bb) is the right pattern: single source of truth for which exception category applies, no duplication of the packages-only detection logic.
• 216/216 tests with clean dogfood coverage; the two new tests (local-reusable publication round-trip and undeclared-map guard) confirm both sides of the boundary.
• The GITHUB_TOKEN admission wiring (admitGitHubToken: publicationRequirement !== undefined) is precisely scoped throughout the call chain.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fe09647c06

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

[claude]

Claude finished @kyle-sexton's task in 4m 23s —— View job

---

PR Review

• Gather context (PR description, changed files, prior review thread)
• Read REVIEW.md for review criteria
• Read runner-policy.mjs — runner-input fixed-literal path through privilegedHostedRequirement
• Read README.md — evaluate Codex P2 finding at line 514 (runner-input paragraph)
• Evaluate all prior findings at HEAD fe09647
• Post final review

---

Prior findings — resolved

All findings from rounds 1–5 are correctly addressed at HEAD (fe09647):

• Early-return bypass (round 1 🔴): publicationRequirement is held as a candidate; environment, credential-expression, and credential-minting-action checks all run before returning it.
• README GITHUB_TOKEN carve-out (round 2 🔴): documented in the publication rule paragraph.
• Later workload rules (round 3 🔴): the write-permission sentence (line 592) and the GITHUB_TOKEN step env/with exception (line 617) both carry the packages-only qualifier.
• auditLocalPermissionFlow inconsistency (round 4 🔴): dcd56bb calls privilegedHostedRequirement to derive the required reason, so the flow pass and the direct audit agree on one exception key.
• README local-reusable paragraph (round 5 🟡): lines 603–605 now read "A fixed hosted called job may inherit write only with its own privileged-control-plane exception — or publication when the called job declares its own packages-only write map with no other privileged surface, per the packages-only rule above."
• Hosted-reusable credential scan (round 5 🔴): fe09647 runs localCredentialRequirement over the caller before returning the held publication requirement, so credential expressions smuggled through with: values escalate to privileged-control-plane.

---

🟡 Nit — Runner-input paragraph still missing the packages-only qualifier

File: components/runner-policy/README.md lines 187–190

The runner-input section says:

a caller that instead pins a fixed hosted runner literal through the same input gets no permission waiver and still needs proven hosted execution under the privileged-control-plane exception category like any other write- or id-token-capable job.

In privilegedHostedRequirement, the permission check at lines 2778–2788 runs for any call where reviewedCallerPermissions is false (i.e., not genuinely selector-routed). A runner-input caller that pins ubuntu-24.04 as the literal has target.kind === "hosted-literal", so reviewedCallerPermissions is false and permissionHostedRequirement fires — which now returns publication for packages-only write maps. That publicationRequirement then survives the environment, credential-expression, and credential-minting-action checks and is returned at line 2907. So the code already applies the publication category to this caller, but the README still says privileged-control-plane.

A consumer following this sentence would declare privileged-control-plane and receive a hosted-exception-category failure with no README explanation.

Suggested fix — append the qualifier to the sentence:

a caller that instead pins a fixed hosted runner literal through the same input gets no permission waiver and still needs proven hosted execution under the privileged-control-plane exception category — or publication when the only write scope is packages, per the packages-only rule — like any other write- or id-token-capable job.

Fix this →

---

What's working well

• The publicationRequirement held-candidate pattern throughout privilegedHostedRequirement is clean and correctly defers until all downstream checks pass — the hosted-reusable path (fe09647) applies the same credential scan before returning.
• admitGitHubToken: publicationRequirement !== undefined is precisely scoped at every call site: localCredentialRequirement (line 2859), the new hosted-reusable path (line 2810), and auditLocalPermissionFlow's delegated call through privilegedHostedRequirement (line 776).
• All five README sections that could state the write-permission rule now consistently carry the packages-only qualifier — only this one sentence in the runner-input subsection remains.
• 217/217 tests with clean dogfood coverage; the two fe09647 additions (secrets expression in with.prompt stays privileged; literal input passes clean) confirm the hosted-reusable credential boundary is correctly drawn.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 168294fa4d

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

[claude]

Claude finished @kyle-sexton's task in 3m 53s —— View job

---

PR Review

• Gather context (PR description, changed files, prior review thread)
• Read REVIEW.md for review criteria
• Read runner-policy.mjs — evaluate Codex P2 finding at line 2907 (structural hosted checks)
• Read auditRepository to understand the privilegedHosted ?? structuralHostedRequirement pattern
• Evaluate all prior findings at HEAD (168294f)
• Post final review

---

Prior findings — resolved

All findings from rounds 1–6 are correctly addressed at HEAD (168294f):

• Early-return bypass (round 1 🔴): publicationRequirement held as candidate; environment, credential-expression, and credential-minting-action checks all run before returning it.
• README GITHUB_TOKEN carve-out (round 2 🔴): documented in the publication rule paragraph.
• Later workload rules (round 3 🔴): write-permission sentence and GITHUB_TOKEN step env/with exception both carry the packages-only qualifier.
• auditLocalPermissionFlow inconsistency (round 4 🔴): dcd56bb calls privilegedHostedRequirement to derive the required reason; the flow pass and direct audit agree on one exception key.
• README local-reusable paragraph (round 5 🟡): lines 605–608 now read "or publication when the called job declares its own packages-only write map with no other privileged surface."
• Hosted-reusable credential scan (round 5 🔴): fe09647 runs localCredentialRequirement over the caller before returning the held publication requirement.
• Runner-input paragraph (round 6 🟡): lines 189–192 now carry the packages-only carve-out.

---

🔴 Important — Container and service execution bypass the publication escalation

File: components/runner-policy/runner-policy.mjs line 2907

privilegedHostedRequirement returns publicationRequirement at line 2907 without checking whether the job has a container: or services: key. In auditRepository, the result feeds into:

const hostedRequirement = privilegedHosted ?? structuralHostedRequirement(job);

Because publicationRequirement is non-null, the ?? short-circuits and structuralHostedRequirement never runs. A packages-only write job with a container or services therefore needs only a publication exception, even though the README at lines 527–532 says container jobs require job-container and service-only jobs require service-container.

Before this PR, every write-capable job returned privileged-control-plane from permissionHostedRequirement, so the ?? always short-circuited to privileged-control-plane — the most restrictive category, which implicitly covered the container surface. Now that publication (a lighter category) can emerge from that path, the container goes uninventoried.

Concrete exploit path — this job passes with only a publication exception:

jobs:
  publish:
    permissions:
      packages: write
    container:
      image: ghcr.io/attacker/builder:latest   # arbitrary container code
    runs-on: ubuntu-24.04
    steps:
      - run: npm publish

The structuralHostedRequirement call would have returned { reason: "job-container" }, requiring a job-container exception — but it is never reached.

Fix: Before return publicationRequirement at line 2907, check structuralHostedRequirement(job) and escalate if it fires:

if (structuralHostedRequirement(job)) {
  return {
    reason: "privileged-control-plane",
    description: "containerized execution with packages-only write permissions",
    rule: "privileged-hosted-only",
  };
}
return publicationRequirement;

This preserves the pre-PR behavior: write-capable + container was always privileged-control-plane, never job-container.

Fix this →

---

🔴 Important — Missing tests for container + packages-only combination

File: components/runner-policy/runner-policy.test.mjs

The publication test block covers environment, credential expressions, credential-minting actions, named secrets, GITHUB_TOKEN admission, local-reusable round-trip, and hosted-reusable credential smuggling. None of the new tests combine packages: write with container: or services:. Without them, the code bug above is invisible to CI.

Fix this →

---

🟡 Nit — README publication rule omits container/services as escalating surfaces

File: components/runner-policy/README.md lines 520–525

Any additional write scope — or any other privileged surface on the job, such as a deployment environment, a credential expression, or a credential-minting action — keeps the job privileged.

The sentence lists three escalating surfaces. Once the code fix above lands, a container: or services: declaration will also escalate to privileged-control-plane for packages-only write jobs. The sentence should name them alongside the others so downstream consumers know to expect privileged-control-plane when they add a job container to a publication job.

---

On the Codex P2 finding

The finding correctly identifies the structural-hosted bypass. My analysis confirms it as a regression introduced by this PR (pre-PR: write + container always privileged-control-plane; post-PR: packages-only write + container escapes to publication). The fix is the one-line structural check above, not an architectural change.

---

What's working well

• The publicationRequirement held-candidate pattern is clean and correctly defers for all surfaces tested so far.
• admitGitHubToken: publicationRequirement !== undefined is precisely scoped at every call site.
• All six README sections that state the write-permission rule now consistently carry the packages-only qualifier.
• 217/217 tests with dogfood coverage of this repository's own workflows.

[claude]

Claude finished @kyle-sexton's task in 3m 51s —— View job

---

PR Review

• Gather context (PR description, changed files, prior review thread)
• Read REVIEW.md for review criteria
• Read changed files at HEAD (f2e375a)
• Evaluate round 7 finding: container/services bypass + tests + README omission
• Check for any new issues at HEAD
• Post final review

---

Prior findings — all resolved

All findings from rounds 1–7 are correctly addressed at HEAD (f2e375a):

• Early-return bypass (round 1 🔴): publicationRequirement held as candidate; environment, credential-expression, and credential-minting-action checks all run before returning it.
• README GITHUB_TOKEN carve-out (round 2 🔴): documented in the publication rule paragraph.
• Later workload rules (round 3 🔴): write-permission sentence and GITHUB_TOKEN step env/with exception both carry the packages-only qualifier.
• auditLocalPermissionFlow inconsistency (round 4 🔴): dcd56bb delegates to privilegedHostedRequirement; the flow pass and direct audit agree on one exception key.
• README local-reusable paragraph (round 5 🟡): lines 605–608 carry the packages-only carve-out.
• Hosted-reusable credential scan (round 5 🔴): fe09647 runs localCredentialRequirement over the caller before returning the held publication requirement.
• Runner-input paragraph (round 6 🟡): lines 189–192 carry the packages-only qualifier.
• Container/services structural bypass (round 7 🔴): f2e375a changes auditRepository's hostedRequirement resolution to structuralHosted ?? privilegedHosted when privilegedHosted.reason === "publication", so a containerized packages-only publisher is correctly placed into job-container/service-container inventory rather than bypassing to publication.

---

On the round 7 🟡 Nit (README missing container/services as escalating surfaces)

The round 7 review assumed the container fix would escalate to privileged-control-plane. The actual implementation uses structuralHosted ?? privilegedHosted to yield job-container/service-container — which is more semantically correct. The publication paragraph's phrase "any other privileged surface on the job, such as a deployment environment, a credential expression, or a credential-minting action — keeps the job privileged" remains accurate: container: does keep the job out of publication, and the immediately following paragraph (lines 527–532) states the job-container/service-container requirement unconditionally. No README change is needed.

---

🟡 Nit — Services-only path missing a test in the publication test block

File: components/runner-policy/runner-policy.test.mjs

The container fix test at line 2179 covers container: + publication → requires job-container. The symmetric case — services: (no container) + publication → requires service-container — is not tested in the publication block. The code path is structurally identical (same ternary, structuralHostedRequirement returns service-container) and is almost certainly correct, but a test matching the container test would close the coverage gap.

---

What's working well

• The placement of the fix in auditRepository (structuralHosted ?? privilegedHosted when reason === "publication") is architecturally sound: it keeps structuralHostedRequirement as the single source of truth for container-category logic rather than duplicating it inside privilegedHostedRequirement.
• The ternary is clean and its asymmetry is documented inline: privileged requirements keep their ordinary precedence over structural ones; only the publication downgrade gives structural priority.
• The admitGitHubToken: publicationRequirement !== undefined wiring is precisely scoped at every call site throughout the full function.
• 218/218 tests with dogfood coverage; every 🔴 finding across eight review iterations has a corresponding test verifying the fix.
• README is self-consistent across all six passages that state the write-permission rule.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f2e375a635

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

[claude]

Claude finished @kyle-sexton's task in 3m 48s —— View job

---

PR Review

• Gather context (PR description, changed files, prior review thread)
• Read REVIEW.md for review criteria
• Read runner-policy.mjs — evaluate commit 429a4fc (local reusable structural precedence)
• Read runner-policy.test.mjs — evaluate new tests
• Evaluate all prior findings at HEAD
• Post final review

---

Prior findings — all resolved

All findings from rounds 1–8 are correctly addressed at HEAD (429a4fc):

• Early-return bypass (round 1 🔴): publicationRequirement held as candidate; environment, credential-expression, and credential-minting-action checks all run before returning it.
• README GITHUB_TOKEN carve-out (round 2 🔴): documented in the publication rule paragraph.
• Later workload rules (round 3 🔴): write-permission sentence and GITHUB_TOKEN step env/with exception both carry the packages-only qualifier.
• auditLocalPermissionFlow inconsistency (round 4 🔴): dcd56bb delegates to privilegedHostedRequirement; the flow pass and direct audit agree on one exception key.
• README local-reusable paragraph (round 5 🟡): lines 605–608 carry the packages-only carve-out.
• Hosted-reusable credential scan (round 5 🔴): fe09647 runs localCredentialRequirement over the caller before returning the held publication requirement.
• Runner-input paragraph (round 6 🟡): lines 189–192 carry the packages-only qualifier.
• Container/services structural bypass (round 7 🔴): f2e375a's structuralHosted ?? privilegedHosted ternary when reason === "publication" ensures a containerized packages-only publisher lands in job-container/service-container inventory.
• Local reusable structural precedence (round 8 🔴): 429a4fc mirrors this in auditLocalPermissionFlow — when declaredRequirement.reason === "publication", structuralHostedRequirement(job) runs and its reason takes precedence. A containerized packages-only called job now demands job-container in both the direct audit and the flow pass; the new round-trip test (lines 2207–2240) confirms a single job-container exception satisfies both.
• Round 8 🟡 (services-only direct-audit test): not yet addressed — carried below.

---

🟡 Nit — Services-only path still untested for both the direct and local-reusable cases

File: components/runner-policy/runner-policy.test.mjs

The round 8 nit identified that the direct-audit services-only case has no test. Commit 429a4fc added the container local-reusable round-trip (line 2207) but did not add the services-only counterparts: neither the direct-audit case (packages: write + services: + publication exception → hosted-exception-category requiring service-container) nor the local-reusable round-trip case (packages: write + services: inside a local reusable with service-container exception → passes both audits).

The code handles these paths correctly — structuralHostedRequirement(job) returns { reason: "service-container" } for services-only jobs, and the ternary in auditRepository (line 3239) and the new flow-pass guard (line 789) both give that precedence over publication — but the coverage gap leaves the services branch unexercised in the publication test block and in the local-reusable context.

Fix this →

---

What's working well

• The fix in 429a4fc is minimal and precise: rather than duplicating the auditRepository ternary logic, it makes a targeted call to structuralHostedRequirement(job) exactly when the declared requirement is publication and gives structural precedence. The structural check and the publication check remain independent, consistent with the rest of the function.
• The round-trip test (lines 2207–2240) is the right kind of integration assertion — it validates that the exception key satisfies both audits simultaneously rather than testing each path in isolation.
• permissionCapability correctly returns "may-write" for the called job in the local-reusable test (the job-level permissions: { packages: write } declaration triggers the flow-pass check), so the test exercises the full code path under review.
• All nine prior 🔴 findings have corresponding tests. The implementation is sound for all covered paths.