Repository navigation
fix(rules): WH013 reads steps with the shared workflow reader - #1006
Conversation
WH013's masked-write check carried its own step splitter, which missed three valid shapes Codex reported on #950: - a literal `steps:` inside an earlier step's `run: |` script reset the splitter, so the real sibling steps after it were never seen; - an indentless sequence (`steps:` with `- ` items at the key's column) ended the steps value before its first item; - a step opened by `-` and a comment took the comment's column as the step's key column, so `continue-on-error: true` never matched. WH013 now reads jobs and steps through Hypatia.WorkflowYaml (comment lines dropped, aliases expanded, steps only under the job's own `steps:` key), so the first is gone by construction. The other two are fixed in the reader, which RE001 shares: its indentless `steps:` items are now read (a harden-runner step there counts), and a comment-opened step keeps its `run:`. Tests: 7 new (4 WH013, 3 RE001); 6 fail on main, one per gap, and the seventh guards the sibling-step rule in the indentless shape. WH013 and RE001 findings on the hypatia, echidna and standards trees are identical before and after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CYR5bf5FBLS7cXQK72aVNM
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 35 minutes. View limit details
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. |
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. |
…ager-wright-zwwbe2-wh013-reader
|
This PR's red Merge-order note: #1002 adds WH014 code that calls Generated by Claude Code |
![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: b0658ca684
ℹ️ 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".
The shared reader splits jobs under `jobs:`, `"jobs":` or `'jobs':`, but workflow_preamble/1 still split only on a bare `jobs:`. With a quoted key the "preamble" was the whole file, so one job's PAT marked every writing job as credentialed and hid a starved GITHUB_TOKEN push. The boundary now accepts the same three forms. (Codex, #1006.) The new test passes on main (whose own job splitter did not split a quoted key either), fails without this fix, and passes with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CYR5bf5FBLS7cXQK72aVNM
|
Add Carrot credits or activate Agent usage billing to use Autofix |
…ager-wright-zwwbe2-wh013-reader
#1002 added WH014, which split a job body into steps with WorkflowHardening.step_bodies/1. This branch replaces that splitter with Hypatia.WorkflowYaml, so after merging main WH014 no longer compiled. WH014 now gets each job's step texts from WorkflowYaml.job_steps/1 (only the job's own `steps:` key, comment lines dropped), the same reader WH013 and RE001 use. blank_or_comment?/1, which WH014 also calls, is restored. All 220 RE and WH tests pass (including #1002's WH014 suite), and WH013, WH014 and RE001 give identical results to main on the hypatia, echidna and standards trees. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CYR5bf5FBLS7cXQK72aVNM
…ager-wright-zwwbe2-wh013-reader
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 602241520e
ℹ️ 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".
…H014 Two places still recognised fewer YAML spellings than the shared reader (Codex, #1006): - `jobs: &all_jobs` (an anchor or tag on the jobs key) did not start the jobs mapping in WorkflowYaml.workflow_job_lines/1, so WH013 fell back to one body with no steps and reported a write masked only by its step's continue-on-error as an ordinary starved write. The old splitter found it, so this was a regression. The key now accepts node properties. - WH014 split a job's preamble only on a bare `steps:`, so with a quoted `"steps":` a step-level working-directory was read as the job default and the SARIF path no longer matched. The split now accepts the same three spellings as the reader (pre-existing on main, exposed here). Two tests, one per gap; both fail without the fix. All 222 RE and WH tests pass, and WH013, WH014 and RE001 are unchanged from main on the hypatia, echidna and standards trees. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CYR5bf5FBLS7cXQK72aVNM
…ager-wright-zwwbe2-wh013-reader
|
|
Replying here because GitHub is refusing review-thread replies on this PR right now (422 "abuse" on both threads). Both Codex P2s on
All 222 RE and WH tests pass. WH013, WH014 and RE001 give the same results as Generated by Claude Code |
Summary
Step 2 of 2, stacked on #1005. Merge that first; this PR's base is its branch.
WH013's masked-write check (
continue-on-error: trueon the step that writes) used its own step splitter. Codex found three valid YAML shapes it missed on #950 (threads 4326052772, 4326052783, 4326052793):steps:line inside an earlier step'srun: |script (a heredoc that writes a workflow)steps:keysteps:with its-items at the key's column-and a comment (- # publication step)-WH013 now reads jobs and steps through
Hypatia.WorkflowYaml, which drops comment lines, expands aliases and finds steps by the job's field column. That removes the first gap by construction. The other two are fixed in the shared reader, so RE001 gets them too:steps:sequence is read, so a harden-runner step there now counts;run:, so its exposures are seen.WH013's own
step_bodiesparser is deleted.masked_write?/1stays public; its steps now come from the shared reader.Type of change
How has this been verified?
--warnings-as-errors). hex.pm is unreachable here.research_extensions_test+workflow_hardening_test: 195/195. That is the 188 existing tests plus 7 new ones.main:ResearchExtensions.scan/1give identical results before and after on the hypatia, echidna and standards trees. The fix closes the gaps without new noise.mix format --check-formatted: clean.Notes for reviewers
read_workflow/1, so a KYAML workflow is read as block layout too. Block YAML is returned byte-identical.continue-on-error. That is the same safe direction as before.WorkflowYaml's internals and leave WH013 and RE001 alone.Checklist
masked_write?/1doc).Generated by Claude Code