Fix YAML timestamp handling in action scanner

[justin808]

Why

The repository action-pinning scanner used Psych safe loading without the two scalar classes Psych uses for valid YAML timestamps. On Ruby 3.4/Psych 5.4, an unquoted date or full timestamp could therefore raise before the scanner checked any action reference.

Closes #302.

What changed

• Keep YAML.safe_load_file and narrowly permit only Date and Time for scanner inputs.
• Route workflow and composite-action parsing through that single safe loader.
• Add regression coverage showing timestamp-bearing YAML still accepts an immutable full-SHA action reference and rejects a mutable tag.

Validation

• RED on main: Ruby 3.4.6/Psych 5.4.0 raised Psych::DisallowedClass for Date; a full timestamp reproduced the same boundary for Time.
• Focused timestamp regression: 1 run, 4 assertions, 0 failures.
• ruby bin/repository-security-policy-test.rb: 9 runs, 34 assertions, 0 failures.
• bin/validate under the host-compatible macOS Bash path: passed, including RuboCop on 112 files with 0 offenses.
• Independent QA/adversarial review: passed on exact head e55a6d2e895a80f9d2d15cdc4fc7e2abc80354ee, including base negative controls, parser red-team cases, and action-reference behavior.
• Hosted validate and claude-review: passed on the exact head; CodeRabbit was terminally unavailable because of its service rate limit.

Codex Decision Log

• Non-blocking: Which YAML object classes should the scanner permit?
  • Decision: Permit only Date and Time, the classes Psych materializes for valid date and timestamp scalars.
  • Why: This fixes the observed parser boundary while retaining safe loading and rejecting unrelated object deserialization.
  • Review later: None.

Review churn

• Pre-push review gate: maker TDD plus coordinator exact-diff review.
• Post-push review churn: 0 follow-up commits.
• Outcome: independent QA passed; the single non-blocking Claude consistency nit was declined with evidence and resolved because applying it would broaden parsing outside the scanner's scope.

Confidence note: High implementation confidence from red/green reproduction, fail-closed reference assertions, one-file scope, full local validation, independent QA, and current-head GitHub evidence.

QA Evidence

• Base negative control reproduced Psych::DisallowedClass independently for both Date and Time on Ruby 3.4/Psych 5.4.
• Candidate safe-loading matrix accepted only the intended timestamp classes and continued to reject arbitrary objects, symbols, and subclass hooks.
• Immutable full-SHA workflow and composite-action references remained accepted; mutable tags remained rejected.
• Focused regression: 1 run, 4 assertions; full security-policy test: 9 runs, 34 assertions.

Completed-batch audit

Status: Clean — no outstanding findings or follow-ups. Durable receipt.

[coderabbitai]

Warning

Review limit reached

@justin808, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 56 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8a2d59ec-006e-4e5f-8a77-1dc1b9beda6c

📥 Commits

Reviewing files that changed from the base of the PR and between 4ed056b and e55a6d2.

📒 Files selected for processing (1)

• bin/repository-security-policy-test.rb

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Review: Fix YAML timestamp handling in action scanner

Overview

Fixes #302: bin/repository-security-policy-test.rb's action-pinning scanner called YAML.safe_load_file(path, aliases: true) without any permitted_classes. On Ruby 3.4/Psych 5.4, an unquoted date (2026-08-02) or full timestamp (2026-08-02T12:34:56Z) scalar in a workflow/composite-action YAML file resolves to a Date/Time object, and Psych's safe loader raises Psych::DisallowedClass for any class not explicitly permitted — so the scanner would crash before checking any uses: reference at all, rather than just on the offending file.

Code quality

• Clean, minimal, single-file diff (37/-4). The fix is exactly as scoped as the issue: add Date/Time to permitted_classes and route both workflow and composite-action parsing through one load_yaml_file helper (DRY-ing up two previously-duplicated safe_load_file calls).
• require "date" is correctly added — needed since Date must be a loaded constant to appear in permitted_classes.
• The new regression test (test_action_reference_scanner_accepts_timestamp_scalars_and_still_rejects_mutable_uses) is well constructed: it exercises both a bare date scalar and a full ISO8601 timestamp scalar, in files that also contain SHA-pinned and tag-pinned uses: references, so it verifies both "doesn't crash on timestamps" and "still correctly flags mutable references" in one go — i.e., it's a fail-closed check, not just a crash-avoidance check.

Security considerations

• Explicitly allowlisting only Date and Time (rather than, say, widening to permitted_classes: true or disabling safe loading) preserves the safe-load boundary — these two classes can't be abused for arbitrary object deserialization/code execution the way !ruby/object or similar tags could. This is the correct, minimal-privilege fix for the Psych::DisallowedClass boundary.
• No behavior change for the actual security check itself (SHA-pinning enforcement via acceptable_action_reference?) — only the YAML loading step is touched.

Minor nit (left inline)

test_dependabot_proposes_pinned_action_updates_for_review still calls YAML.safe_load_file directly rather than through the new load_yaml_file helper. Not a bug (different file, unrelated to reference scanning), just a small consistency question — worth a quick look but not blocking.

Test coverage / validation

• PR description shows red/green reproduction on Ruby 3.4.6/Psych 5.4.0, plus full bin/repository-security-policy-test.rb suite (9 runs/34 assertions) and RuboCop passing with 0 offenses. Confirmed via static review that the current repo has no workflow/action files containing bare date/timestamp scalars today, so this is a forward-looking regression fix rather than one masking a currently-broken CI run — appropriate given it's meant to harden the scanner for future contributions.

Overall: a well-scoped, correctly-reasoned bug fix with solid regression coverage. No blocking issues found.

[justin808]

Address-review summary

Scan scope: full PR history; no previous address-review cutoff existed.

Mattered

• None. Both independent QA and the configured Claude review found no blocking or correctness issue.

Optional

• Declined the .github/dependabot.yml loader-consistency suggestion because that file is outside the scanner input surface and deliberately uses aliases: false; reusing the scanner helper would broaden parsing beyond Follow-up: accept YAML timestamp scalars in the action pinning scanner #302. Rationale was posted on the original thread and the completed conversation was resolved: Fix YAML timestamp handling in action scanner #305 (comment)

Skipped

• CodeRabbit reported its review-rate limit and supplied no code finding; treated as terminal reviewer-unavailable metadata under the configured reviewer policy.

No code changes or follow-up commits were required. Current candidate head: e55a6d2e895a80f9d2d15cdc4fc7e2abc80354ee.

Next default scan starts after this comment. Say check all reviews to rescan the full PR.

[justin808]

Completed-batch audit: replay evidence follows.

[justin808]

---

head_sha: e55a6d2
triggered_gates:

• security-auth-privacy
  rollback_disposition: code-only-rollback-established
  decision: approve
  approved_by: justin808
  source: direct-user-task
  evidence: Codex task 019fc164-0f1b-7970-b075-be9d78280dba direct-user approval in awb-issue302-maple on 2026-08-02
  ...

[justin808]

Completed-batch audit: replay evidence follows.