Skip to content

[codex] Harden release security gates#177

Merged
project-navi-bot merged 4 commits into
mainfrom
codex/release-security-gate-hardening
Jun 9, 2026
Merged

[codex] Harden release security gates#177
project-navi-bot merged 4 commits into
mainfrom
codex/release-security-gate-hardening

Conversation

@Fieldnote-Echo

@Fieldnote-Echo Fieldnote-Echo commented Jun 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • require actionlint.yml and zizmor.yml to be green for the release SHA before a tag can publish
  • pin that release-gated workflow list in tests/release_publish_invariants.py
  • fail the release invariant if trusted-publishing-blocked triggers (pull_request_target, workflow_run) are added
  • fail if id-token: write is granted workflow-wide or outside known release signing/publishing jobs
  • fail closed on scalar/non-mapping permissions blocks such as permissions: write-all
  • document the expanded pre-tag gate in RELEASING.md

Basis

Current crates.io/GitHub OIDC guidance treats trusted publishing as bound to owner/repo/workflow/environment claims and requires id-token: write only for jobs that request OIDC tokens. crates.io also blocks pull_request_target and workflow_run for trusted publishing, so the repo-local release invariant now pins those constraints and rejects scalar permission shortcuts.

Validation

  • bash tests/release_publish_invariants.sh
  • bash tests/release_signed_release_invariants.sh
  • /tmp/ordvec-actionlint/actionlint .github/workflows/release.yml
  • python3 -m py_compile tests/release_publish_invariants.py
  • negative fixture: top-level permissions: write-all fails with workflow permissions must be an explicit mapping
  • negative fixture: job-level permissions: write-all fails with jobs.require-ci-green.permissions must be an explicit mapping
  • git diff --check
  • GitHub API sanity check: actionlint.yml and zizmor.yml are active workflows with recent successful push-to-main runs

Stacked on #176.

Copy link
Copy Markdown
Owner Author

/agentic_review

@qodo-code-review

qodo-code-review Bot commented Jun 4, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0)

Grey Divider


Action required

1. Gate accepts non-push runs 🐞 Bug ⛨ Security
Description
The require-ci-green gate in release.yml counts any successful run on main for the SHA without
restricting the triggering event. Because fuzz.yml executes different jobs for push vs
workflow_dispatch/schedule, a manual/scheduled run can satisfy the gate while skipping the intended
push smoke checks.
Code

.github/workflows/release.yml[R172-175]

+          for wf in ci.yml python.yml fuzz.yml codeql.yml actionlint.yml zizmor.yml; do
            ok="$(gh api \
              "repos/${REPO}/actions/workflows/${wf}/runs?head_sha=${SHA}&branch=main&status=success&per_page=20" \
              --jq '[.workflow_runs[] | select(.head_branch == "main" and .conclusion == "success")] | length')"
Evidence
release.yml’s GH API query filters by head_sha/branch/success but not by event, so any successful
run for that SHA on main can satisfy the gate. fuzz.yml supports workflow_dispatch and schedule, and
its jobs are gated by github.event_name such that dispatch/schedule runs can skip the push smoke
job; therefore the gate can be satisfied without running the intended push smoke checks.

.github/workflows/release.yml[162-180]
.github/workflows/fuzz.yml[22-29]
.github/workflows/fuzz.yml[46-52]
.github/workflows/fuzz.yml[77-81]
tests/release_publish_invariants.py[456-475]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`require-ci-green` currently accepts *any* successful workflow run for the release SHA on `main`, regardless of event type. This allows `workflow_dispatch` or `schedule` runs to satisfy the gate.

This is particularly problematic for `fuzz.yml`, which conditionally runs different jobs depending on `github.event_name` (push/PR smoke vs schedule/dispatch weekly). A successful dispatch run can therefore skip the push smoke job entirely while still being counted as “green”.

### Issue Context
The PR’s intent is to ensure the tag can only publish once the per-commit push-to-main gates are green. The gate should require a successful **push** run on `main` for the exact SHA.

### Fix Focus Areas
- .github/workflows/release.yml[162-180]

### Suggested implementation
- Add an explicit event filter to the GH API query, e.g. `...&event=push...`, and/or tighten the jq filter to select only runs where `.event == "push"`.
- Consider updating `tests/release_publish_invariants.py` to assert that the gate filters on push events (so future edits can’t reintroduce the bypass).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Permissions check bypass ✓ Resolved 🐞 Bug ⛨ Security
Description
check_release_security_gates only enforces the id-token: write restriction when permissions is
a dict; if permissions is any non-mapping YAML value, the check is skipped and the invariant can
miss workflow-wide or job-wide id-token grants. This undermines the PR’s stated goal of failing the
invariant when id-token: write is granted workflow-wide or outside approved release jobs.
Code

tests/release_publish_invariants.py[R452-498]

+    top_permissions = workflow.get("permissions")
+    if isinstance(top_permissions, dict) and top_permissions.get("id-token") == "write":
+        fail(f"{path}: id-token: write must be scoped to explicit signing/publishing jobs, not workflow-wide")
+
+    jobs = mapping(workflow.get("jobs"), f"{path}: jobs")
+    require_job = mapping(jobs.get("require-ci-green"), f"{path}: jobs.require-ci-green")
+    steps = sequence(require_job.get("steps"), f"{path}: jobs.require-ci-green.steps")
+    gated_workflows = ("ci.yml", "python.yml", "fuzz.yml", "codeql.yml", "actionlint.yml", "zizmor.yml")
+    found_loop: tuple[str, ...] | None = None
+    for index, raw_step in enumerate(steps):
+        step = mapping(raw_step, f"{path}: jobs.require-ci-green.steps[{index}]")
+        run = step.get("run")
+        if not isinstance(run, str):
+            continue
+        match = re.search(r"(?m)^\s*for\s+wf\s+in\s+(.+?);\s+do\s*$", run)
+        if match:
+            found_loop = tuple(shlex.split(match.group(1)))
+            break
+    if found_loop is None:
+        fail(f"{path}: require-ci-green must loop over the release-gated workflow filenames")
+    if found_loop != gated_workflows:
+        fail(
+            f"{path}: require-ci-green gates {found_loop!r}; expected {gated_workflows!r}"
+        )
+
+    allowed_id_token_jobs = {
+        "attest",
+        "provenance",
+        "publish-crate",
+        "attest-manifest",
+        "manifest-provenance",
+        "publish-manifest-crate",
+        "publish-pypi",
+    }
+    for job_name, raw_job in jobs.items():
+        if not isinstance(job_name, str):
+            continue
+        job = mapping(raw_job, f"{path}: jobs.{job_name}")
+        permissions = job.get("permissions")
+        if not isinstance(permissions, dict):
+            continue
+        id_token = permissions.get("id-token")
+        if id_token == "write" and job_name not in allowed_id_token_jobs:
+            fail(
+                f"{path}: jobs.{job_name} grants id-token: write but is not an allowed "
+                "release signing/publishing job"
+            )
Evidence
The invariant explicitly skips validation unless permissions is a dict, and the YAML loader does
not enforce types—so a non-dict permissions value will pass through without being checked for
id-token: write. This is directly visible in the new check_release_security_gates logic and in
the load_workflow parser.

tests/release_publish_invariants.py[31-57]
tests/release_publish_invariants.py[452-455]
tests/release_publish_invariants.py[489-498]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`check_release_security_gates()` only validates `permissions` when the parsed YAML value is a mapping (`dict`). If a workflow/job uses a non-mapping `permissions` value, the invariant silently skips validation, which can allow the intended `id-token: write` restriction to be bypassed.

## Issue Context
The workflow YAML is parsed with `yaml.safe_load()` (or `yq` JSON) without schema enforcement, so `permissions` can be any YAML type. The invariant should be robust against that and either (a) normalize supported non-mapping forms, or (b) fail closed when `permissions` is not a mapping so the security gate cannot be bypassed.

## Fix Focus Areas
- tests/release_publish_invariants.py[452-498]
- tests/release_publish_invariants.py[31-57]

### Implementation guidance
- Update the top-level `permissions` check to fail-closed when `permissions` is present but not a mapping (or explicitly recognize/normalize any supported scalar forms, then re-run the same `id-token` check).
- Update the per-job `permissions` loop similarly: if `permissions` is present but not a mapping, fail (or normalize) rather than `continue`, so jobs cannot evade the allowlist by changing the YAML shape.
- Keep error messages explicit (e.g., "permissions must be a mapping to validate id-token scoping").

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request updates the release documentation and introduces a new security gate validation check (check_release_security_gates) in the test suite to enforce security invariants on the release workflow, such as blocking unsafe triggers and restricting id-token: write permissions. The review feedback correctly identifies critical security bypasses where the workflow-wide or job-level permissions could be set to the string "write-all", which implicitly grants write access and would bypass the current dictionary-based checks. Implementing the suggested checks for "write-all" will make the security gates more robust.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread tests/release_publish_invariants.py
Comment thread tests/release_publish_invariants.py
Comment thread tests/release_publish_invariants.py
@codecov

codecov Bot commented Jun 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Fieldnote-Echo Fieldnote-Echo marked this pull request as ready for review June 4, 2026 14:41
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Harden release security gates with OIDC and workflow validation

✨ Enhancement 🧪 Tests

Grey Divider

Walkthroughs

Description
• Add comprehensive release security gate validation checks
  - Detect blocked triggers (pull_request_target, workflow_run)
  - Enforce id-token: write scoping to allowed jobs only
  - Validate release-gated workflow list in CI gate
  - Verify environment configuration for publishing jobs
• Expand release CI gate to include actionlint.yml and zizmor.yml
• Update documentation to reflect new security requirements
Diagram
flowchart LR
  A["Release Workflow"] -->|"Validate triggers"| B["Security Gate Checks"]
  B -->|"Check id-token scope"| C["Job Permissions"]
  B -->|"Verify gated workflows"| D["CI Gate List"]
  B -->|"Validate environments"| E["Publishing Jobs"]
  D -->|"Include"| F["actionlint.yml + zizmor.yml"]
  C -->|"Restrict to"| G["Allowed Signing/Publishing Jobs"]

Loading

Grey Divider

File Changes

1. tests/release_publish_invariants.py ✨ Enhancement +84/-0

Add comprehensive release security gate validation

• Add trigger_names() helper to extract trigger names from workflow on configuration
• Implement check_release_security_gates() function with multiple validation checks:
 - Reject workflows using pull_request_target or workflow_run triggers
 - Enforce id-token: write is not set workflow-wide
 - Validate require-ci-green job loops over exact gated workflow list
 - Restrict id-token: write to allowed signing/publishing jobs only
 - Verify publishing jobs use correct environments (crates-io, pypi)
• Integrate new security gate check into main() function

tests/release_publish_invariants.py


2. .github/workflows/release.yml ✨ Enhancement +5/-4

Expand CI gate to include security lint workflows

• Expand require-ci-green gate to include actionlint.yml and zizmor.yml workflows
• Update job documentation to reflect new security lint gates
• Update step name and comments to reference "release-gated workflows" generically
• Modify shell loop to iterate over six workflows instead of four

.github/workflows/release.yml


3. RELEASING.md 📝 Documentation +4/-3

Document expanded release CI gate requirements

• Update release process documentation to list actionlint.yml and zizmor.yml as required gates
• Clarify that require-ci-green checks all six gated workflows for successful completion
• Maintain consistency with workflow implementation changes

RELEASING.md


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Jun 4, 2026

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit b5d1643

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

ℹ️ 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 tests/release_publish_invariants.py

Copy link
Copy Markdown
Owner Author

Remediated the scalar permissions bypass in 8c054f9.

What changed:

  • Release publish invariants now reject non-mapping workflow-level permissions values, including scalar shortcuts such as permissions: write-all.
  • Job-level permissions values are also fail-closed when present but not explicit mappings.
  • Omitted job-level permissions still inherit the workflow map; the invariant only rejects malformed/scalar permission blocks.

Validation:

  • python3 -m py_compile tests/release_publish_invariants.py
  • bash tests/release_publish_invariants.sh
  • Negative fixture: top-level permissions: write-all fails with workflow permissions must be an explicit mapping.
  • Negative fixture: job-level permissions: write-all fails with jobs.require-ci-green.permissions must be an explicit mapping.
  • git diff --check

Comment thread .github/workflows/release.yml Outdated
@Fieldnote-Echo Fieldnote-Echo force-pushed the codex/release-security-gate-hardening branch from 8c054f9 to 71b7b5b Compare June 4, 2026 15:51

Copy link
Copy Markdown
Owner Author

Addressed the remaining release-gate bot finding in 024804c.

What changed:

  • require-ci-green now queries only successful event=push workflow runs on main for the release SHA.
  • The jq filter also requires .event == "push", so a manual/scheduled run cannot satisfy the release gate.
  • tests/release_publish_invariants.py now fails if the release gate drops the push-event filter.

Validation:

  • python3 -m py_compile tests/release_publish_invariants.py
  • bash tests/release_publish_invariants.sh
  • bash tests/release_signed_release_invariants.sh
  • git diff --check

@Fieldnote-Echo Fieldnote-Echo force-pushed the codex/api-parity-release-audit branch from 3321054 to 84c0b42 Compare June 9, 2026 14:31
Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
@Fieldnote-Echo Fieldnote-Echo force-pushed the codex/release-security-gate-hardening branch from 024804c to 4db1446 Compare June 9, 2026 14:31
Base automatically changed from codex/api-parity-release-audit to main June 9, 2026 15:55
…y-gate-hardening

# Conflicts:
#	RELEASING.md
#	tests/release_publish_invariants.py
@project-navi-bot project-navi-bot merged commit 6e49c90 into main Jun 9, 2026
30 checks passed
@project-navi-bot project-navi-bot deleted the codex/release-security-gate-hardening branch June 9, 2026 16:01
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.

2 participants