Conversation
…1003) Fixes#949. ## What happened `_code_view()` (`spec/audit.py`) stripped comment-only lines but did not strip YAML block-scalar (`|`, `>`, and their chomp-indicator variants) body content. Every `requireTokensInJob`/`forbidTokensInJob` check in `check_interface()` is a plain substring search over that view, so a job whose `name:` (or any other string-valued key) is written as a block scalar containing text that happens to match a required or forbidden token, at a matching indent, could satisfy or trip the check without the actual YAML structure the token is meant to verify existing at all. ## Fix The issue's own targeted-mitigation shape: strip a block scalar's body lines (more indented than its own `key:` line, until the first line at or below that indent), keeping only the `key:` line itself, the same bounded heuristic `_code_view()` already uses for comments. Not a general YAML parser, since the issue notes that's likely overkill unless the false-positive/negative surface turns out to be broader than block scalars alone. ## Tests Extended `_selftest()` (what CI runs offline, no network, per OPERATIONS.md "Local Verification") with: - The exact adversarial construction from the issue: the `deploy-site.yml` caller-stub fixture with its real `with:`/`environment:` mapping replaced by a block-scalar `name:` containing that text, still reporting the token missing. - Three direct `_code_view()` fixture cases: body dropped/key kept, a dedented sibling key ending the block scalar body correctly, and a folded scalar with a chomp indicator plus a trailing comment still matching the key-line pattern. ## Verification Ran locally from the repo root: `python3 spec/audit.py --selftest` (SELFTEST PASS, including the new cases), `uvx ruff@latest check`/`format --check`, `uvx mypy@latest`, `python3 scripts/prose_lint.py` (default set), `python3 scripts/repo_gate.py`. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved workflow validation by ignoring YAML block-scalar content while preserving relevant scalar key lines. * Added support for literal and folded scalars, indentation and chomping indicators, anchors, comments, sequence items, and dedented sibling keys. * Prevented tokens within scalar bodies from being incorrectly interpreted as workflow configuration. * **Tests** * Expanded coverage for scalar-body exclusion and supported YAML formatting variations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: ptr727-codegen[bot] <275599072+ptr727-codegen[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: OpenAI Codex <codex@openai.com>
Warning Review limit reachedNext included review available in 1 minute. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Comment |
PR Summary by QodoIgnore YAML block-scalar bodies in audit token checks
AI Description
Diagram
High-Level Assessment
Files changed (1) |
Code Review by Qodo
1. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Follow-up to #1003 (issue #949), found by qodo-code-review on the develop -> main promotion PR #1004, which cannot carry the fix directly since its diff is develop against main. ## What happened _code_view()'s block-scalar recognition required a mapping key before the `|`/`>` indicator, so a keyed step (`- run: |`) was recognized but a bare sequence item with no key at all (`- |`, e.g. a matrix string) was not, leaving its body searchable and the requireTokensInJob/forbidTokensInJob mitigation bypassable the same way the original issue describes. ## Fix Recognizes a bare `- |`/`- >` too. Its boundary is the dash's own column rather than a key past it, since there is no key: verified against a real YAML parser (content one column past the dash parses, content at or before the dash's column is a parse error). ## Verification Ran locally from the repo root: `python3 spec/audit.py --selftest` (SELFTEST PASS, including the new case), `uvx ruff@latest check`/`format --check`, `uvx mypy@latest`, `python3 scripts/prose_lint.py` (default set), `python3 scripts/repo_gate.py`. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved code view filtering for bare block scalars in sequence items, including literal and folded formats. * Correctly removes multiline scalar content while preserving indentation behavior for keyed sequence items. * **Tests** * Added coverage for filtering matrix-style block scalars and validating indentation boundaries. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Uh oh!
There was an error while loading. Please reload this page.
Promotes #1003 (issue #949) to
main.What
spec/audit.py:_code_view()drops YAML block-scalar bodies (key: |/key: >,including a step's
- run: |and an anchoredkey: &label |) from the interfacetoken-matching view, so a crafted or ordinary block scalar can no longer satisfy or
trip a
requireTokensInJob/forbidTokensInJobcheck as though it were real YAMLstructure (check_interface()'s token matching reads YAML block-scalar content as structure #949).
Review
PR #1003 review loop: CodeRabbit found one real bug (a sequence-item's block-scalar
boundary used the dash's column instead of the key's, dropping a genuine sibling key
alongside the body) and qodo found a second real bug (an anchor property before the
indicator was not recognized) plus several comment-style findings, all fixed and
resolved. One qodo informational finding (task-specific issue-number references in
comments) was declined with in-file precedent (spec/audit.py:13, :281, :2764 already
cite issue numbers the same way). Copilot's own review account is in the fleet's
known repo-wide quota-exhausted state (confirmed live on every request this round),
so this proceeded on CodeRabbit's and qodo's coverage per standing precedent.