Repository navigation
fix(ci): inbox-steward-intake publishes through PRs and fails loudly - #943
hyperpolymath merged 2 commits into
Conversation
Four defects in .github/workflows/inbox-steward-intake.yml: - update-ruleset appended " :: TODO: Apply rule update: …" text lines to .hypatia-baseline.json for every critical/high suggestion, corrupting the JSON the Governance baseline gate reads. It now only surfaces them as ::notice annotations; the suggestions file (applied: false) is unchanged. - Every commit and push to main was masked (`2>/dev/null || true`, `|| echo "::warning::…"`), so a refused write stayed green. Writes now go through scripts/open-data-pr.sh: a branch, a PR and a squash auto-merge (the estate-rescan.yml pattern), with every failure exiting non-zero and an empty change skipped by `git diff --cached --quiet`. - record-report committed only when inputs.dry_run == "false", but the input is empty on repository_dispatch, so every real dispatch ran as a dry run. Only an explicit "true" is dry now, matching update-ruleset. - The .git-private-farm dispatch discarded stderr and turned a refusal into a warning. It now fails the job. FARM_PAT needs Contents and Pull requests write on this repository, and auto-merge must be enabled. Previously the pushes went straight to main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CYR5bf5FBLS7cXQK72aVNM
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
6ab2046
into
claude/eager-wright-zwwbe2-wh013
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aecf943c5e
ℹ️ 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".
|
|
||
| # --auto queues the merge until the base-branch policy allows it; a bare | ||
| # merge right after creation is refused before the policy settles. | ||
| gh pr merge "${pr_url##*/}" --repo "$GITHUB_REPOSITORY" --squash --auto --delete-branch |
There was a problem hiding this comment.
When required checks are pending, gh pr merge --help states that --auto only enables a later merge after requirements are met, so a zero exit here does not mean the data reached main. The helper therefore returns success immediately, allowing dispatch-to-farm in .github/workflows/inbox-steward-intake.yml to publish paths that do not exist on main; if CI later fails, the intake run still remains green. Poll the PR state and fail unless it reaches MERGED, as the repository's estate-rescan.yml publisher already does.
Useful? React with 👍 / 👎.
| exit 0 | ||
| fi | ||
|
|
||
| branch="inbox-steward/${GITHUB_RUN_ID}-${suffix}" |
There was a problem hiding this comment.
If an attempt fails after pushing this branch but before completing the PR/merge operation, rerunning the same workflow uses the unchanged GITHUB_RUN_ID and reconstructs the same branch name. The new commit is created independently from main, so the push is normally rejected as non-fast-forward (or PR creation collides with the existing PR), preventing the rerun from recovering from the original transient failure. Include GITHUB_RUN_ATTEMPT or explicitly resume/clean up the existing branch.
Useful? React with 👍 / 👎.
| fi | ||
|
|
||
| branch="inbox-steward/${GITHUB_RUN_ID}-${suffix}" | ||
| git switch -c "$branch" |
There was a problem hiding this comment.
When a maintainer starts the workflow_dispatch from a non-main branch or tag with dry_run false, actions/checkout checks out that selected ref and this command creates the publication branch directly from it. The resulting PR targets main, so it includes every commit unique to the selected ref in addition to the generated data file. Create the branch from the fetched origin/main ref, or restrict writable dispatches to main, before committing the generated path.
Useful? React with 👍 / 👎.
) ## Summary PR 4 of the baseline-gate stack, stacked on #942 (which now also carries #943). It clears all 3 RE005 findings at their source: one false positive in the rule, and two masks removed from workflows. | Finding | Verdict | Fix | |---|---|---| | `tests.yml:763` `trap 'kill "$STRESS_PID" 2>/dev/null \|\| true' EXIT` | **Rule false positive.** The EXIT trap runs after the step's status is decided. Under `set -e`, an *unmasked* failing `kill` turns a **passing** step red (measured: rc 1). The `\|\| true` is what keeps it honest. | RE005 skips `trap …` lines (`lib/rules/research_extensions.ex`). The same false positive is in echidna `server-boot-gate.yml:40`. | | `tests.yml:192` `$(find … -exec grep … \| grep -v … \|\| true)` | **Real, small mask.** It also swallowed grep's exit **2**: with a missing directory the old step reports PASS. | Rewritten as `grep -r --include … --exclude-dir=rules` with an explicit status test (1 = no match = pass, >1 fails). Comment lines are removed with `sed`, which can't exit 1 on "all filtered". | | `build-gossamer-gui.yml:189` `ls … 2>/dev/null \|\| true` right before `exit 1` | The mask did nothing except hide the diagnosis | Mask dropped | ## Type of change - [x] 🐛 Bug fix (a rule false positive, plus two CI masks) ## How has this been verified? - **Rule:** I compiled `research_extensions.ex` with `scanner_suppression.ex`, `cicd_rules.ex` and `security_errors.ex` standalone (Elixir 1.14; hex.pm is unreachable here) and ran `test/research_extensions_test.exs`: - **42/42** pass; - control: the new trap test fails against the previous module, and the "a trap does not hide a real swallow" test passes on both; - `mix format --check-formatted` passes. - **Blast radius (`re005_test_swallows_exit/1`, old → new):** - hypatia: 3 → **0**; - standards: 0 → 0; - echidna: 2 → 1 (the trap in `server-boot-gate.yml` clears; `live-provers.yml:475` is a real finding and remains). - **The `tests.yml` step**, extracted with `yq` and run on this tree: - clean → rc 0 PASS; - planted `believe_me` in `src/` → rc 1 FAIL; - planted in `lib/rules/` → PASS (excluded, as before); - `-- believe_me` comment → PASS; - a missing search directory → **rc 2 FAIL**, where the old step reports PASS. - `actionlint` 1.7.7: identical message sets before and after on both files (6 pre-existing in `tests.yml`, 0 in `build-gossamer-gui.yml`). ## Checklist - [ ] My commits are **signed**. The session's signing shim is platform-mediated. - [x] I ran the project's own checks/tests locally and they pass (scoped as above). - [x] New files carry the correct SPDX (none added). - [x] Docs are updated (the RE005 `@doc` states the trap rule). - [x] I have not introduced a soundness hole. Only whole `trap …` lines are skipped, and a real swallow elsewhere in the same step still fires (test). --- _Generated by [Claude Code](https://claude.ai/code/session_01CYR5bf5FBLS7cXQK72aVNM)_ Co-authored-by: Claude <noreply@anthropic.com>
## Summary This is the last step of the baseline-gate stack. `governance.yml` and `hypatia-scan.yml` pass `hypatia-ref: aa34a4c` (#940), which predates the rule fixes that clear this repo's own false positives. This PR moves both to **hypatia `main` at `23dc74d`**, which contains: - #938: ActionsLock accepts v0.0.3 locks (already in `aa34a4c`); - #942: WH013 judges writes per job and skips PAT/app-token writes. This clears `estate-rescan.yml`; `inbox-steward-intake.yml` was also repaired in #943; - #947: RE005 skips `trap` handlers, and job extraction survives comments between jobs. `uses:` refs are unchanged, so `actions.lock` is untouched. ## What this PR's governance run will show This PR's own `governance / Validate Hypatia Baseline` run is the measurement: it builds the scanner at `23dc74d`. The jump from `aa34a4c` also brings #931's new rules (WF026, WF027, PIN-003), which haven't been measured on this repo yet. hex.pm is blocked in my session, so I can't build the escript to pre-run them. **Expected remaining findings, by class:** - **RE001 (harden-runner):** most of the original 18 jobs, including the 2 dispatched label workflows, which must stay action-free. - These need an owner decision; the harden-runner steps were removed again in #948 because the lock has no record. - **WH013, RE004, RE005, `invalid_actions_lock`:** expected 0. I'll post the measured list here once the run completes. ## Type of change - [x] 🔧 Build / CI / tooling ## How has this been verified? - `actionlint` 1.7.7 is clean on both files. - `23dc74d` is `origin/main` at the time of writing. `git log aa34a4c..23dc74d` contains #942, #947, #945, #931, #941 and #936. ## Checklist - [ ] My commits are **signed**. The session's signing shim is platform-mediated. - [x] I ran the project's own checks/tests locally and they pass. - [x] New files carry the correct SPDX (none added). - [x] Docs are updated (the pin comment names what it carries). - [x] I have not introduced a soundness hole. --- _Generated by [Claude Code](https://claude.ai/code/session_01CYR5bf5FBLS7cXQK72aVNM)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
2 participants
Summary
PR 3 of the baseline-gate stack, stacked on #942. Fixes the true positive left after WH013's precision fix: four real defects in
.github/workflows/inbox-steward-intake.yml.update-rulesetappended:: TODO: Apply rule update: …lines to.hypatia-baseline.json::noticeannotations only. Thedata/ruleset-updates/*.jsonrecord (applied: false) is unchanged.git commit/git push origin HEADwas masked (2>/dev/null || true,|| echo "::warning::…") and went straight to mainscripts/open-data-pr.sh: branch, PR, squash auto-merge (theestate-rescan.ymlpattern). Every failure exits non-zero. An empty change is skipped withgit diff --cached --quiet.record-reportcommitted only wheninputs.dry_run == "false"; the input is empty onrepository_dispatchupdate-ruleset(!= 'true') did write'true'is a dry run, matchingupdate-ruleset..git-private-farmdispatch discarded stderr and turned a refusal into a warningContext: none of this has produced data.
data/inbox-steward-reports/anddata/ruleset-updates/don't exist on main, and the last 17 runs (to 2026-07-05) all failed.Type of change
How has this been verified?
wh013_permission_starved_write/1(with fix(rules): judge WH013 per job, and skip writes made with their own credential #942) on this tree returns[], down from 1:warnunmask-credentialed-writefinding.actionlint1.7.7: the same 14 messages before and after (diffed). All 14 are pre-existing and sit in thesummaryjob; see notes.shellcheck scripts/open-data-pr.sh: clean.open-data-pr.shagainst a bare local remote with a stubgh:inbox-steward/123-reportand callsgh pr create … --base mainthengh pr merge 77 --squash --auto --delete-branch(rc 0);Checklist
scripts/open-data-pr.sh: MPL-2.0).Notes for reviewers
FARM_PATwith Contents and Pull requests write on this repo, plus Allow auto-merge enabled. If either is missing, the job now fails visibly instead of warning.summaryjob builds its step summary with${{ … && echo '…' || echo '…' }}. That is shell inside an expression, and actionlint rejects it ("parser did not reach end of input"). Left out to keep this PR to the write path.Generated by Claude Code