Skip to content

feat: support policy-only Pulumi deploy guard - #79

Merged
kyle-sexton merged 2 commits into
mainfrom
codex/pulumi-guard-policy-only
Jul 11, 2026
Merged

feat: support policy-only Pulumi deploy guard#79
kyle-sexton merged 2 commits into
mainfrom
codex/pulumi-guard-policy-only

Conversation

@kyle-sexton

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

Copy link
Copy Markdown
Contributor

Summary

  • allow the reusable Pulumi deploy guard to run with an explicit empty expected_runner_policies: []
  • keep OIDC subject/repository/environment validation, stack identity checks, and export-shape validation fail-closed
  • return deterministic empty policy/label outputs in policy-only mode
  • preserve the existing 1–32 policy behavior for all current callers
  • make immutable-pin tests acquire two deleted-branch fixtures by their exact 40-character SHAs

Why

The personal GitHub IaC rollout needs to establish its protected OIDC deployment environment before any self-hosted routing configuration exists. The guard previously coupled OIDC verification to the presence of routing policy entries, creating a bootstrap cycle. This adds a narrow policy-only mode; it does not activate runner routing or weaken identity validation.

The fixture fetch is separate test infrastructure: fetch-depth: 0 fetches branches and tags, not retained hidden PR refs after source-branch deletion. CI fetches only the two immutable commits, never checks them out or executes them, and does not change production pins or workflow permissions.

Validation

  • Pulumi guard Bash suite: 22/22
  • relevant Node contract tests: 50/50
  • full hosted CI and ci-status
  • hosted selector-conformance
  • actionlint
  • shellcheck
  • shfmt
  • Biome 2.5.1
  • git diff --check
  • signed commits verified
  • fresh credential-free shallow clone reproduced the missing objects and proved exact-SHA acquisition, object integrity, and no FETCH_HEAD mutation

The original independent review passed at 30367dbb8146aa4f4412fa28b3e83aa92e0a7e4a. The same independent reviewer rechecked final head c7d2546c26c4e98ce317d4185b6888dc42f61be6 and reported PASS with no findings. Both bot reviews also passed.

Rollout

The final squash SHA will be pinned by the separate personal OIDC-foundation change. No secrets, Apps, environments, or runner routing are changed here.


Note

Medium Risk
Touches the pre-apply OIDC and stack gate, but only adds a documented empty-URN bootstrap path with stricter JSON parsing and expanded tests; identity and policy checks remain fail-closed.

Overview
Adds an explicit policy-only path to the reusable Pulumi deploy guard by accepting operational-resource-urns-json: []. Callers can run the same OIDC policy check, stack identity validation, and stack export shape checks without listing state-adoption URNs; outputs are deterministic zeros and empty JSON/target lists.

Validation tightening: guard.sh now requires exactly one JSON document (jq -s), allows 0–32 URNs (was 1–32), and still rejects malformed JSON, non-arrays, and duplicate URNs. Docs and action.yml describe policy-only mode; bash and Node contract tests cover empty-array success and fail-closed cases (wrong policy, bad export, stack/org mismatch).

CI: ci.yml and selector-conformance.yml add a shallow git fetch of two pinned commits so immutable workflow pin tests still resolve after PR branch cleanup.

Reviewed by Cursor Bugbot for commit c7d2546. Bugbot is set up for automated code reviews on this repo. Configure here.

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown

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


Code Review — feat: support policy-only Pulumi deploy guard

  • Read changed files: guard.sh, action.yml, guard.test.sh, pulumi-deploy-guard.test.cjs, README.md
  • Analyze correctness, security, and CLAUDE.md alignment
  • Post review findings

Summary

The implementation is correct, minimal, and maintains all security invariants. The change is well-justified — it breaks the bootstrap cycle without widening any authentication or policy gate.


Core mechanism (guard.sh)

The key change: adding -s (slurp) to the initial jq validation and dropping length >= 1:

Before:

jq -e '
  type == "array" and
  length >= 1 and length <= 32 and
  ...
' "$requested_urns"

After:

jq -e -s '
  length == 1 and
  (.[0] |
    type == "array" and
    length <= 32 and
    ...
  )
'  "$requested_urns"

The -s flag collects all JSON documents in the file into a wrapper array. length == 1 then enforces exactly one JSON document was present (catching the []\n[] multi-document case). The inner length <= 32 drops the >= 1 lower bound, enabling empty arrays. This is a clean, well-reasoned approach. permalink

Downstream jq calls with [] all behave correctly:

  • The cross-stack prefix check (all(.[]; startswith($prefix))) is called without -s; all over an empty stream returns vacuously true in jq. ✓
  • --slurpfile requested "$requested_urns" produces $requested[0] = []; map(...) on empty gives []. ✓
  • jq -r '.[]' on [] emits nothing, giving an empty existing-targets file. ✓

Security properties preserved

All critical checks still run regardless of array length:

Check Policy-only mode
OIDC policy exact-match (cmp -s) ✓ still enforced
Stack export (Pulumi API call) ✓ still runs
Export shape validation ✓ still validated
Contract organization ↔ stack organization ✓ still enforced
Bundled contract wildcard rejection (*, ?, .) ✓ still enforced
Issuer deduplication ✓ still enforced

The only thing that changes at []: the resource-counting pass is vacuously empty and all five outputs are deterministic zeros/empty arrays/empty multiline block.


Deterministic empty output

The test pins the exact GITHUB_OUTPUT content for policy-only mode including the existing-targets multiline delimiter:

ci_runner_e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855

This is SHA-256 of the empty string — a well-known constant. The hardcoded hash is correct and will never change. permalink


Proactive bug fix in reset_valid_fixtures

requested_urns is now reset inside reset_valid_fixtures. Before this PR, no test set requested_urns='[]', so the omission was harmless. With the new policy-only tests setting it, omitting the reset would contaminate downstream test runs with [] where the two-URN default is expected. Adding the reset here is the correct fix. permalink


Test coverage

New tests in guard.test.sh provide complete coverage of the new mode:

Test Guard
empty array → deterministic empty outputs
policy-only mode still requires exact OIDC policy regression guard
policy-only mode still requires valid export shape regression guard
policy-only mode still validates stack identity regression guard
malformed JSON fails before auth
non-array JSON fails before auth
multiple JSON documents fail before auth

Existing test duplicate requested URNs fail before authentication now gets a grep -F 'operational-resource-urns-json is invalid' assertion on stderr — a minor improvement to the pre-existing test.

Node attestation tests verify the new description strings in action.yml and assert the old length >= 1 and length <= 32 pattern is absent from guard.sh. These tests are structural sanity checks and are correctly formed.


Findings

No correctness bugs, security issues, or CLAUDE.md violations found.

One observation worth noting (not a defect): the action.yml description for operational-resource-urns-json says "zero to 32" — this is accurate, but callers who inspect the description after the change will see the policy-only mode explicitly documented, which is desirable for discoverability. permalink


Verdict: PASS. The change is minimal, correctly implemented, fully tested with regression guards for all security-critical paths, and aligned with the repository's security ground rules. The bootstrap cycle motivation is sound and the narrow scope (no changes to runner routing, secrets, environments, or the contract itself) is appropriate.

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown

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


Review of updated PR (new commit: c7d2546 fix: make immutable-pin tests clone-independent)

  • Identify new changes since last review
  • Read changed files
  • Review for correctness, security, and CLAUDE.md alignment
  • Post findings

What changed in this sync

The previous review at 30367db gave a PASS. One new commit has since landed:

c7d2546  fix: make immutable-pin tests clone-independent

Three files touched:

File Change
.github/scripts/pulumi-deploy-guard.test.cjs Formatting only — collapses a 3-line assert.match to 1 line, no logic change
.github/workflows/ci.yml Adds a git fetch step before the Node test runner
.github/workflows/selector-conformance.yml Same git fetch step

New git fetch step

- name: Fetch immutable workflow pins under test
  # A full-history checkout excludes hidden PR refs after their source
  # branches are deleted, so acquire only the exact commits under test.
  run: |
    git fetch --no-tags --no-write-fetch-head --depth=1 origin \
      ca3c19cbd946db3d7d9bc0a3782497ed9cecacb4 \
      fdf0e6e5905e50be8e332f9bc522e24947a61b13

Why it's needed. The immutable-pin tests (*.test.cjs) verify that specific workflow files at pinned SHAs have expected content. When a PR's source branch is deleted post-merge, the refs/pull/N/head ref becomes hidden — those commits are unreachable via the regular git fetch --all or even fetch-depth: 0. Fetching by object SHA directly solves this without touching refs or branch state. permalink — ci.yml

Flags are correct:

  • --no-tags — doesn't pollute the tag namespace
  • --no-write-fetch-head — doesn't update FETCH_HEAD, cleaner for subsequent steps
  • --depth=1 — fetches only the exact commit objects needed, not history chains
  • Two hardcoded SHAs, not mutable refs — consistent with the repo-wide SHA-pin policy in CLAUDE.md

Security: This repo is public. git fetch origin works without stored credentials even though the checkout uses persist-credentials: false. The fetched commits come from the same origin repository. No secrets are involved. No checkout of untrusted code into the workspace root occurs. No CLAUDE.md rules are touched.

No findings. The step is minimal, well-commented, and correctly scoped.


Overall verdict: PASS

The new commit is a narrow, well-reasoned fix to a test-environment reliability issue. The two SHAs are fetched immutably from origin, the flags minimize side-effects, and the approach is consistent with the repository's security posture. No correctness bugs, security issues, or CLAUDE.md violations in either commit.

@kyle-sexton
kyle-sexton marked this pull request as ready for review July 11, 2026 14:28
@cursor

cursor Bot commented Jul 11, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_8d4fbeab-e0c2-4d68-ab9b-7f7ebdc3f6ff)

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown

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


PR Review — feat: support policy-only Pulumi deploy guard

  • Read CLAUDE.md guidelines
  • Read changed files (guard.sh, action.yml, guard.test.sh, pulumi-deploy-guard.test.cjs, ci.yml, selector-conformance.yml, README.md)
  • Analyze correctness, security, and CLAUDE.md alignment
  • Post review findings

Summary

The implementation is correct and maintains all security invariants. Two commits are in scope:

  • 30367db — core policy-only feature
  • c7d2546 — make immutable-pin tests clone-independent

Core mechanism (guard.sh)

Before:

jq -e '
  type == "array" and
  length >= 1 and length <= 32 and
  ...
' "$requested_urns"

After:

jq -e -s '
  length == 1 and
  (.[0] |
    type == "array" and
    length <= 32 and
    ...
  )
' "$requested_urns"

The -s (slurp) flag collects all JSON documents in the file into a wrapper array. This transforms the single-document validation into two distinct checks:

  • Outer length == 1 — enforces exactly one JSON document, catching the []\n[] multi-document injection path (test at L211)
  • Inner length <= 32 — drops the >= 1 lower bound, enabling the empty-array path

Each input shape is verified correct:

Input jq -s sees Outer check Inner check Result
[] [[]] length 1 ✓ [], length 0 ≤ 32 ✓ pass
[]\n[] [[], []] length 2 ✗ reject
not-json parse error error reject
{"urn":...} [{"urn":...}] length 1 ✓ type ≠ array ✗ reject

Downstream calls with [] all behave correctly:

  • Line 60: jq -e --arg prefix "..." 'all(.[]; startswith($prefix))' is called without -s. The file contains [] as a single JSON document (empty array); all over an empty iterator returns vacuously true in jq. ✓
  • Lines 161–167: --slurpfile requested "$requested_urns"$requested[0] = []; map(...) on empty gives []. ✓
  • Line 176: jq -r '.[]' "$existing_json" on [] emits nothing → zero-byte existing_targets file. ✓

Security properties preserved

All identity and policy gates still run regardless of array length:

Check Policy-only mode
Bundled contract wildcard rejection (*, ?, .) ✓ enforced pre-auth
OIDC issuer exact-match ✓ enforced
OIDC policy exact-match (cmp -s) ✓ enforced
Contract organization ↔ stack organization ✓ enforced
Stack export (Pulumi API call) ✓ still runs
Export shape validation ✓ still validated
Duplicate URN rejection ✓ still enforced (vacuously empty counts)

The only thing that changes at []: the resource-counting pass is vacuously empty and all five outputs are deterministic zeros/empty arrays/empty multiline block.


Deterministic empty output

The test pins the exact GITHUB_OUTPUT content for policy-only mode (L106–L115):

existing-count=0
missing-count=0
existing-urns-json=[]
missing-urns-json=[]
existing-targets<<ci_runner_e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
ci_runner_e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855

SHA-256 of the empty string is e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 — a fixed constant. The multiline delimiter format (name<<DELIM\nDELIM) matches what write_multiline_output produces for a zero-byte file. ✓


reset_valid_fixtures fix

requested_urns is now reset inside reset_valid_fixtures (L56). Before this PR, no test set requested_urns='[]', so the omission was harmless. With the new tests setting it, omitting the reset would contaminate downstream test runs with [] where the two-URN default is expected. The fix is correct.


Test coverage

New bash tests provide complete coverage of the new mode:

Test Guard
Empty array → deterministic empty outputs ✓ full cmp of GITHUB_OUTPUT
Policy-only mode still requires exact OIDC policy regression guard
Policy-only mode still requires valid export shape regression guard
Policy-only mode still validates stack identity regression guard
Malformed JSON fails before auth
Non-array JSON fails before auth
Multiple JSON documents fail before auth

The duplicate-URN test now gains a grep -F 'operational-resource-urns-json is invalid' stderr assertion — a minor tightening of a pre-existing test.

Node attestation tests (pulumi-deploy-guard.test.cjs L75–77) verify that length <= 32 is present, length == 1 and is present, and the old length >= 1 and length <= 32 pattern is absent from guard.sh. These are structural integrity checks that correctly constrain the live implementation.


git fetch steps (c7d2546)

Both ci.yml and selector-conformance.yml add a git fetch step that fetches two hardcoded SHAs from origin before running immutable-pin tests. This is a correct fix for tests that verify pinned-SHA workflow content: when a PR's source branch is deleted post-merge, those commits become unreachable via the standard fetch-depth: 0 checkout, but fetching by object SHA directly works reliably.

Flags are correct: --no-tags (no tag namespace pollution), --no-write-fetch-head (no FETCH_HEAD side-effect), --depth=1 (only the needed object). Both SHAs are hardcoded immutable refs, consistent with the CLAUDE.md SHA-pin policy. No checkout into the workspace root occurs; no secrets are involved.


Findings

No correctness bugs, security issues, or CLAUDE.md violations found.

Verdict: PASS. The change is minimal, correctly implemented, fully tested with regression guards on all security-critical paths, and aligned with the repository's security ground rules. The bootstrap-cycle motivation is sound and the narrow scope (no changes to runner routing, secrets, environments, or the contract itself) is appropriate.

@cursor cursor 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.

Risk: medium. Not approving: Cursor Bugbot completed as skipped (usage limit reached), so the required automated review signal is incomplete. Human review is needed before merge; no reviewers were assigned (no eligible reviewers besides the author).

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@kyle-sexton
kyle-sexton merged commit d87a590 into main Jul 11, 2026
38 of 40 checks passed
@kyle-sexton
kyle-sexton deleted the codex/pulumi-guard-policy-only branch July 11, 2026 14:32

@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: c7d2546c26

ℹ️ 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 on lines +51 to +52
type == "array" and
length <= 32 and

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Bind policy-only mode to the requested stack

When operational-resource-urns-json is [], the existing all(.[]; startswith($prefix)) stack/project check below becomes vacuous, and the remaining contract check only compares the organization. In the new policy-only bootstrap path, a same-org project/stack typo such as kyle-sexton/other/production can therefore pass the guard and proceed to the protected apply for that stack instead of failing before deployment. Please validate the exported stack resource against stack_project/stack_id or otherwise add an explicit stack binding for policy-only mode.

Useful? React with 👍 / 👎.

kyle-sexton added a commit that referenced this pull request Jul 13, 2026
)

Part of melodic-software/github-iac#79 (Epic
melodic-software/github-iac#78, Phase 1 W1).

## What

- **Liveness routing**: the `prefer-self-hosted` candidate test drops
`busy === false`; any matching managed-prefix runner with `status ==
"online"` routes the workflow self-hosted. GitHub natively queues jobs
targeting a busy fleet and only fails them after 24 hours queued
([routing precedence
docs](https://docs.github.com/en/actions/reference/runners/self-hosted-runners#routing-precedence-for-self-hosted-runners)).
Hosted fallback now happens only when the fleet is fully offline.
- **Delete the rerun→hosted hack**: removed the `run_attempt > 1 →
hosted` branch, the `RUN_ATTEMPT` input, and the workflow-level
`github.run_attempt == 1` token-mint guard. A re-run reuses the previous
attempt's successful selector output ([re-run
docs](https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs)),
so the branch only converted re-runs into paid hosted work jobs.
- **Honest naming**: `idle-runner-count` → `online-runner-count`;
reasons `idle` → `online`, `no-idle-runner` → `no-online-runner`. `busy`
is no longer consumed, so it left inventory validation.
- Vendored bundle regenerated (`render-select-runner-workflow.cjs
--check` green); README selector contract updated, including
one-preflight-job-per-workflow guidance.

## Not in this PR (sequenced follow-ups)

- `local-runner-canary.yml` / `production-ha-proof.yml` stay pinned to
the reviewed old selector SHA and stay self-consistent (they assert
`idle` semantics of that pinned revision). They migrate in their own
pin-bump PR once this merges.
- `standards` runner-policy SHA allowlist bump + consumer-repo fan-out
collapse land as separate PRs after merge (allowlist keys on this PR's
merge SHA).

## Verification

- `node --test .github/scripts/*.test.cjs`: 169/169 pass, including new
cases: busy online runner routes self-hosted; busy empty-label scale-set
runner keeps the inferred route; fully offline fleet routes hosted with
`no-online-runner`.
- Empirical routing acceptance (fleet online → `ci-runner-melo-*`, fleet
offline → hosted, re-run keeps route) runs on a routed private repo
after the consumer pin bump, per issue #79.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01EPDbXgonTuFwFwdTtHaCmw

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Changes adaptive CI runner routing for all `prefer-self-hosted`
consumers—busy fleets stay on self-hosted instead of falling back to
paid hosted minutes—while removing rerun-forced hosted routing; behavior
is well covered by unit tests but rollout affects org-wide workflow
execution.
> 
> **Overview**
> **`prefer-self-hosted` now keys off fleet liveness, not idle
capacity.** Matching managed-prefix runners with `status === "online"`
route self-hosted even when `busy` is true; hosted fallback applies only
when no eligible runner is online (`no-online-runner`), relying on
GitHub to queue jobs on a busy fleet.
> 
> The **rerun→hosted shortcut is removed**: no `run_attempt > 1`
preflight branch, no `RUN_ATTEMPT` env/input, and the observer-token
step no longer gates on `github.run_attempt == 1`. The **`rerun` reason
and idle-centric API** are gone—`idle-runner-count` / `idle` /
`no-idle-runner` become **`online-runner-count` / `online` /
`no-online-runner`**, and runner inventory validation no longer requires
`busy`.
> 
> **Tests, vendored `select-runner.yml`, `selector-conformance.yml`, and
README** are updated to match (including busy-online and fully-offline
cases, and public-guard expectations without attempt-2 `rerun`).
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
d1e14eb. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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