Skip to content

fix: clarify no-workflows audit warnings (#584) - #685

Open
JohnsonLyu wants to merge 3 commits into
darnitdevorg:mainfrom
JohnsonLyu:fix/no-workflows-warning-584
Open

JohnsonLyu wants to merge 3 commits into
darnitdevorg:mainfrom
JohnsonLyu:fix/no-workflows-warning-584

Conversation

@JohnsonLyu

@JohnsonLyu JohnsonLyu commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #584.

Improves audit warnings for repositories without GitHub Actions workflow files.

  • Added a baseline-specific github_workflows_present handler to detect missing workflow files before running external tools.
  • Updated OSPS-BR-01.01, OSPS-BR-01.02, and OSPS-AC-04.02 to report a descriptive WARN instead of misleading unexpected_exit warnings when there is nothing to audit.
  • Prevented unnecessary zizmor and grep execution when no relevant inputs exist.
  • Preserved zizmor scanning for standalone action.yml and action.yaml files in OSPS-BR-01.01, ensuring template-injection findings are not suppressed.
  • Added regression tests covering missing workflows, standalone actions, error classification, and existing control behavior.
  • Updated framework documentation and CHANGELOG.
  • Concluding WARN semantics: This introduces the first in-tree step type to conclude WARN under a {fail} ceiling. The SC-008 / FR-017 regression guard is narrowly adjusted to permit this outcome only for the three affected controls and fixtures with no relevant auditable inputs. Since this WARN is conclusive, the manual step no longer runs when there is nothing to audit; the descriptive message provides the operator guidance instead. The result remains non-compliant.

Type of Change

  • Bug fix (non-breaking change fixing an issue)
  • New feature (non-breaking change adding functionality)
  • Breaking change (fix or feature causing existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)

Framework Changes Checklist

If this PR modifies the darnit framework (packages/darnit/):

  • Updated framework spec (docs/architecture/framework-design.md) if behavior changed
  • Ran uv run python scripts/validate_sync.py --verbose and it passes

Control/TOML Changes Checklist

If this PR modifies controls or TOML configuration:

  • Control metadata defined in TOML (not Python code)
  • SARIF fields (description, severity, help_url) included where appropriate
  • Ran validation to confirm TOML schema compliance

Testing

  • Tests pass locally (uv run pytest tests/ -v)
  • Added tests for new functionality (if applicable)
  • Linting passes (uv run ruff check .)

Validation results:

  • Focused regression tests: 355 passed, 2 skipped (Windows symlink limitations).
  • uv run ruff check .: passed.
  • uv run python scripts/validate_sync.py --verbose: passed.
  • git diff --check: passed.
  • Full test suite was attempted but could not pass on Windows due to missing Unix utilities and platform-specific execution failures. Comparison against clean upstream confirmed the existing environmental failures.
  • Linux CI verification remains pending.

AI assistance

  • No AI assistance was used
  • AI assistance was used

Cursor with Claude Opus 5.5 assisted with code investigation, implementation, regression tests, documentation updates, and test execution. Changes and results were reviewed before submission.

Additional Notes

@Marc-cn Marc-cn left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Careful implementation: step type with a {fail} ceiling, first step of the three controls, symlink escapes handled, action.yml only where zizmor reads it, spec table and TOML updated, good unit coverage. Three things:

  1. This is the first handler that returns WARN as a concluding, dispositive result, and the corpus invariant test_warn_outcome_changes_no_existing_control (SC-008 / FR-017: no handler returns WARN) is relaxed to tolerate it. That's a framework decision rather than a bug fix and should be stated in the description; a concluded WARN also stops the control before its manual step, where it used to fall through. @mlieberman85, your call on that.
  2. Needs a rebase, and coordination with #656 on the BR-01.02 grep step.

Re-review after.

@mlieberman85 mlieberman85 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @JohnsonLyu. On Marco's first point, the framework decision: I'm fine with this being the first step that concludes WARN.

framework-design 3.0 has allowed a concluding WARN under a {fail} ceiling since feature 037: "the handler read the evidence, understood it, and determined it is insufficient to pass". It also says a handler must not return WARN to mean "I am unsure". "No workflow files, nothing to audit" is a determined answer, so it fits. It can't conclude PASS or FAIL, it counts as FAIL for compliance either way, and the message tells the person what to do next. The corpus guard (test_warn_outcome_changes_no_existing_control) was 037's check that adding the outcome changed nothing. Narrowing it to these three controls, this message and fixtures with nothing to audit is the right way to retire that assumption.

I'm also OK with the concluded WARN skipping the manual step on a repository with nothing to audit. The message covers what the manual step would have said.

Before this can merge:

  1. Please say in the description that this is the first in-tree step to conclude WARN, that it narrows the SC-008 / FR-017 guard, and that the manual step no longer runs on a repository without workflows. Reviewers and the changelog should see the precedent without reading the tests.
  2. Rebase onto main and coordinate with #656, which adds fail_exit_codes = [0] to the BR-01.02 grep step.
  3. We need a recorded audit of a real repository without workflows, before and after, since this changes audit results. I can run it after the rebase.

Drafted with AI assistance (Claude), reviewed and filed by me.

Assisted-by: Cursor:Claude-Opus-5.5
Signed-off-by: Johnson Lyu <cl7084@nyu.edu>
Assisted-by: Cursor:Claude-Opus-5.5
Signed-off-by: Johnson Lyu <cl7084@nyu.edu>
@JohnsonLyu
JohnsonLyu force-pushed the fix/no-workflows-warning-584 branch from a2d848f to b5937c4 Compare October 31, 2026 22:02
Assisted-by: Cursor:Claude-Opus-5.5
Signed-off-by: Johnson Lyu <cl7084@nyu.edu>
Marc-cn
Marc-cn previously approved these changes Oct 31, 2026

@Marc-cn Marc-cn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed on b5937c4. Rebased on current main; the BR-01.02 grep step keeps #656's fail_exit_codes = [0] with github_workflows_present in front of it. The SC-008 guard is narrowed exactly as far as it needs to be: the three controls, the exact "nothing to audit" message, fixtures with no workflow and no action.yml, with tests for both tolerated and rejected drift. On a repository with no workflows the three controls now read a concluded WARN with the reason, where main gave the #584 unexpected_exit errors. Tests, validate_sync and the corpus gate pass locally.

Still open from Mike's list: the description should say this is the first in-tree step to conclude WARN, that it narrows the SC-008 / FR-017 guard, and that the manual step no longer runs on a repository without workflows — please add that. Mike's before/after audit on a real repository is his to run.

@JohnsonLyu

Copy link
Copy Markdown
Contributor Author

Thanks @Marc-cn and @mlieberman85 for the reviews!

I've updated the PR description and CHANGELOG to clarify the concluding WARN semantics, the narrowed SC-008 / FR-017 guard, and the manual-step behavior.

The rebase and #656 integration are complete, and all CI checks are passing.

@mlieberman85, the PR is ready for the before/after audit on a real repository without workflows, as discussed.

Thanks!

@Marc-cn Marc-cn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approved on d268738: the only change since b5937c4 is the CHANGELOG, which now states the precedent (first in-tree step to conclude WARN, manual step skipped when there is nothing to audit, SC-008/FR-017 guard narrowed to these three controls). Mike's before/after audit on a real repository is the remaining item, then it can merge.

@mlieberman85 mlieberman85 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the before/after audit I said I'd run, on d268738 merged into main at 155dcc1. It changes the wording as intended and no status.

kelseyhightower/nocode has no workflows and no other CI, so it reaches the new step:

control main this PR
OSPS-BR-01.01 WARN, unexpected_exit, by none WARN, no error class, by github_workflows_present
OSPS-BR-01.02 WARN, unexpected_exit, by none WARN, no error class, by github_workflows_present
OSPS-AC-04.02 WARN, unexpected_exit, by none WARN, no error class, by github_workflows_present

No other control's status, details, concluded_by or error class changes, and the status counts are identical. With zizmor 1.30.1, main's three stderr warnings about exit codes 3 and 2 are gone on this PR.

Repositories that shouldn't change didn't:

  • sindresorhus/slash has workflows: no differences.
  • jonschlinkert/is-number and jonschlinkert/is-odd have .travis.yml, so these controls are N/A: no differences.
  • mafintosh/pump and feross/safe-buffer also have .travis.yml: no differences in those fields.

The full suite passes on the merged tree (5359 passed, 11 skipped), and so do ruff and validate_sync.

It now conflicts with main in CHANGELOG.md only, since #684 and #670 merged. Could you rebase? The rebase will dismiss the approvals, and we'll re-approve once only the CHANGELOG has changed.


Drafted with AI assistance (Claude), reviewed and filed by me. Measurements quoted were executed and verified.

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.

No-workflows repositories: three controls report unexpected_exit instead of saying there is nothing to audit

3 participants