feat(runner-policy): register wave-3 runner-input contracts

[kyle-sexton]

Registers two reusable-workflow contracts required by the Wave 3 per-repo floor conversion (melodic-software/github-iac#78):

• do-not-merge-gate.yml@8853021… (feat: add do-not-merge-gate reusable workflow ci-workflows#108) and pr-issue-linkage.yml@f7e94a8… (feat: add pr-issue-linkage reusable workflow ci-workflows#118) — both ship the runner/prerequisite-result shape; registration unblocks the fleet-routed caller rollouts (Roll out do-not-merge-gate caller to all requires-ci repos ci-workflows#120 / fix(runner-policy): require a paired sentinel for required selector input #121).

The contract set is fail-closed: an unregistered reference fails every caller's runner-policy lane, so registration must precede the Wave 3 caller PRs.

A standards-sync.yml@3dfb1845… registration was included originally and removed after Codex review: a secret-bearing runner-input contract without allowedCallerPermissions still fails privilegedHostedRequirement(), and the write-permission rule forbids a read-only allowedCallerPermissions map — sync-lane fleet routing is deferred behind a reviewed secret-capable waiver (tracked on melodic-software/github-iac#78).

No linked issue.

Verification

• Contract entries mirror the registered semantic-pr.yml@51012e2c… runner-input shape (selectorResultInput: prerequisite-result); allowedCallerPermissions omitted per the validator's write-permission rule (both are read-only).
• allowedInputs/allowedSecrets transcribed from each reusable at its pinned SHA.
• Full component suite passes (181/181).

Related

• melodic-software/github-iac#78 (epic — Wave 3)
• Roll out do-not-merge-gate caller to all requires-ci repos ci-workflows#120
• Roll out pr-issue-linkage caller to all requires-ci repos ci-workflows#121

🤖 Generated with Claude Code

https://claude.ai/code/session_01K3QehVwmWzkBLpKokNCkkt

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 903586892b

ℹ️ 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 42s —— View job

---

Review: feat(runner-policy): register wave-3 runner-input contracts

• Gather context (PR diff, changed files)
• Understand the runner-policy contract structure
• Evaluate the Codex concern about standards-sync.yml secret handling
• Cross-check contract entries against stated claims in the PR body
• Synthesize findings and post review

---

Summary

Two of the three contracts (do-not-merge-gate.yml and pr-issue-linkage.yml) are correctly formed and consistent with the semantic-pr.yml@51012e2c… precedent. The third — standards-sync.yml@3dfb18452a… — has a correctness problem that the Codex review identified accurately. The registered contract cannot be used for selector-output routing while the caller passes the app secrets, which is exactly the scenario the Wave 3 exception drop requires.

---

do-not-merge-gate.yml@885302176… and pr-issue-linkage.yml@f7e94a802… — ✅ Correct

Both are clean. They mirror semantic-pr.yml@51012e2c… structurally (routing: runner-input, selectorResultInput: prerequisite-result, allowedSecrets: {}), and the label input on do-not-merge-gate.yml is properly included in allowedInputs. The omission of allowedCallerPermissions is correct for both: empty allowedSecrets means no credential expressions appear in the caller's job body, so privilegedHostedRequirement() never triggers on them.

---

standards-sync.yml@3dfb18452a… — ❌ Blocked by credential-detection dead-end

The Codex finding is confirmed. I traced the enforcement path in runner-policy.mjs and the issue is precise.

How the failure happens:

privilegedHostedRequirement() (lines 2477–2531) has exactly one bypass for a caller's secrets field. It applies only when reviewedCallerPermissions is true:

const reviewedCallerPermissions =
  target?.kind === "selector-output" &&
  reusable.approved &&
  reusable.contract.allowedCallerPermissions !== undefined;   // ← key gate

const credentialJob = reviewedCallerPermissions
  ? Object.fromEntries(Object.entries(job).filter(([name]) => name !== "secrets"))
  : job;                                                       // secrets stay in job
const credentialRequirement = localCredentialRequirement(workflow, credentialJob);

When allowedCallerPermissions is absent (as in the registered standards-sync.yml@3dfb… contract), credentialJob === job and localCredentialRequirement() receives the full job including:

secrets:
  app-client-id: ${{ secrets.STANDARDS_SYNC_APP_CLIENT_ID }}
  app-private-key: ${{ secrets.STANDARDS_SYNC_APP_PRIVATE_KEY }}

localCredentialRequirement() (lines 2396–2443) destructures { steps, if: jobCondition, ...jobWithoutSteps } = job and checks containsCredentialExpression(jobWithoutSteps). Because secrets is not excluded from jobWithoutSteps, the scanner finds secrets.STANDARDS_SYNC_APP_CLIENT_ID and returns "an unapproved or transformed credential expression". That produces:

reason: "privileged-control-plane"
rule:   "privileged-hosted-only"

The privileged-hosted-only finding fires for any target kind that isn't hosted-literal, hosted-matrix, hosted-reusable, or hosted-local-reusable — and selector-output is none of those (lines 2907–2923). There is no exception path around privileged-hosted-only; exceptions only cover hosted-exception-required. So the caller cannot satisfy the policy regardless of what's in its runner-policy.json.

Why allowedCallerPermissions can't be added straightforwardly:

The validator enforces (lines 185–201 of runner-policy.mjs):

if (
  Object.hasOwn(contract, "allowedCallerPermissions") &&
  !Object.values(contract.allowedCallerPermissions).includes("write")
) {
  throw new ConfigurationError(
    `...allowedCallerPermissions must include at least one write permission`,
  );
}

If sync callers truly have only read-only GITHUB_TOKEN permissions, allowedCallerPermissions can't be set without also changing the callers' permission grants — and that would itself require the same reviewed contract update to list those write scopes.

Options to unblock:

1. Determine whether sync callers actually need a write scope (e.g. contents: write to commit synced files, or issues: write for sync-status annotations). If so, add allowedCallerPermissions to the contract with those scopes, and ensure callers declare exactly those permissions. This is the path taken by link-check.yml@3dfb… and pulumi-version-drift-check.yml@3dfb….

2. If sync callers are genuinely read-only, the credentials bypass is architecturally unavailable without a policy extension. In that case the sync lane cannot drop its hosted exception by this route, and a different mechanism (possibly keeping standards-sync.yml@3dfb… as a hosted-only contract with an updated input surface) would be needed.

3. Keep the existing hosted-only entry for exception-bearing callers and register the runner-input entry only when the caller-permission picture is resolved.

The 181/181 test pass is not a contradiction: the component suite validates policy schema structure, not the runtime outcome for a specific caller workflow. A caller in another repo attempting selector-output routing to this contract would surface the privileged-hosted-only finding at audit time, not at policy registration time.

---

Minor notes (non-blocking)

• The allowedSecrets format on standards-sync.yml@3dfb… uses caller-side names (STANDARDS_SYNC_APP_CLIENT_ID) consistent with the existing hosted-only entries. The strict ${{ secrets.<name> }} format check (lines 193–201 in the validator) only applies when allowedCallerPermissions is present, so this is fine for a contract without it.
• Adding manifest and standards-ref to allowedInputs compared to the hosted-only entries is correct if the new workflow version exposes those inputs.

---

Verdict: The Codex P2 finding (comments on policy.json:198) is confirmed by code analysis. The standards-sync.yml@3dfb… contract cannot achieve the Wave 3 exception-drop goal as written. The other two contracts are ready to merge; the blocker is specific to standards-sync.yml. I'd recommend either splitting this into two PRs (the two clean contracts now; standards-sync.yml once the caller-permission question is resolved) or addressing the credential bypass question in this PR before merging.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 24a47a09c7

ℹ️ 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 49s —— View job

---

• Gather PR context and understand current state
• Read policy.json and runner-policy.mjs to trace the enforcement path
• Evaluate new Codex P2 finding on do-not-merge-gate.yml selector-result condition
• Cross-check pr-issue-linkage.yml for same issue
• Synthesize findings and post review

---

Review: feat(runner-policy): register wave-3 runner-input contracts

Current state: After 24a47a0 dropped the problematic standards-sync.yml@3dfb… entry, this PR now registers exactly two contracts. A second Codex P2 review targets do-not-merge-gate.yml's selectorResultInput condition requirement.

---

New Codex P2 finding — do-not-merge-gate.yml selector-result condition: ❎ False positive

Codex claim: Wave 3 callers will use if: ${{ !cancelled() }}, but failClosedSelectorConditionStatus() requires exactly if: ${{ always() }}, so the registration "does not unblock that rollout."

Why the claim is incorrect:

The premise conflates regular runner-input routing (no selectorResultInput) with fail-closed reporter routing (with selectorResultInput). These two patterns are intentionally distinct, and the selectorResultInput field is the mechanism that switches between them.

In runner-policy.mjs:1818–1822:

const condition = usesRequiredInput
  ? selfHostedSelectorConditionStatus(job, selectorId)
  : reusableContract?.selectorResultInput
    ? failClosedSelectorConditionStatus(...)   // requires if: ${{ always() }}
    : cancellationSafeConditionStatus(job.if); // requires if: ${{ !cancelled() }}

• Without selectorResultInput: callers use if: ${{ !cancelled() }} — the reusable runs when not cancelled, result is not forwarded.
• With selectorResultInput: callers must use if: ${{ always() }} — the reusable is a required-check reporter that must fire on every outcome (success, failure, and cancellation) to guarantee the check context always materialises.

failClosedSelectorConditionStatus is not a constraint to work around; it is the contract. Its error message (line 1846) explains the semantic directly:

"fail-closed selector-result reporters must declare exactly if: ${{ always() }} so every prerequisite outcome materializes the required check"

A do-not-merge gate that skips on cancellation is not fail-closed — it can silently omit the required check context, defeating its purpose. The same logic applies to pr-issue-linkage.yml. Both workflows are required-check reporters that need to fire unconditionally.

Evidence this pattern is established: semantic-pr.yml@51012e2c… already carries selectorResultInput: prerequisite-result and its callers are expected to use if: ${{ always() }}. The two new contracts mirror that exact shape. The Wave 3 caller PRs (ci-workflows#120, #121) will likewise need if: ${{ always() }} — that is by design, not a bug in the registration.

---

do-not-merge-gate.yml@885302176… and pr-issue-linkage.yml@f7e94a802… — ✅ Correct

Both contracts remain clean:

• routing: runner-input, runnerInput: runner, selectorResultInput: prerequisite-result — consistent with semantic-pr.yml@51012e2c… precedent.
• allowedSecrets: {} — omission of allowedCallerPermissions is correct; no credential expressions in caller job body, so privilegedHostedRequirement() does not trigger.
• do-not-merge-gate.yml includes label in allowedInputs — correct for that workflow's optional label override.
• Auto-approval is correctly blocked for both by runner-policy.mjs:1620–1626 (selectorResultInput contracts require human review on SHA bumps because the policy can't observe whether the reusable still honors the fail-closed guarantee — this is not a concern for the initial registration here).

---

Verdict

The Codex P2 finding on the second review is a false positive. The selectorResultInput registration correctly mandates if: ${{ always() }} for callers — that is the design intent for fail-closed required-check reporters, not a policy gap to patch. Both remaining contracts are correctly formed. The PR is ready to merge.