ci(security): the "whole tree" secret scan is gated by a path filter, so a change outside backend Python is never scanned #16863

Problem

The secret-detection job in .github/workflows/security.yml scans every tracked file and carries this comment at :477-481:

the whole tracked tree, on EVERY event. There is deliberately no needs: changes gate. The committed Fernet key sat in an extensionless root dotfile that no path filter names, so a gate keyed on file types would skip exactly the change this job exists to catch.

That is true of the job and false of the workflow. on.pull_request.paths gates the entire run above it:

on:
  pull_request:
    branches: [ release, main ]
    paths:
      - 'autobot-backend/**/*.py'
      - 'autobot-slm-backend/**/*.py'
      - 'autobot_shared/**/*.py'
      - '.github/filters/backend-python-paths.yml'
      - '**/requirements*.txt'
      - 'requirements-ci/**'
      - 'pyproject.toml'
      - 'autobot_shared/pyproject.toml'
      - '**/package*.json'
      - '.github/workflows/security.yml'

A pull request touching none of those never triggers the workflow, so the whole-tree scan does not run. Removing needs: changes from the job bought nothing while the filter above it decides whether the job exists at all.

The reasoning in that comment is exactly right about why a path filter is the wrong shape for this scanner. The filter it warns about is the one sitting over it.

Evidence — this is not theoretical, it already happened

#16810 added pipeline-scripts/header_safe_secret.py and pipeline-scripts/header_safe_secret_test.py. pipeline-scripts/** is not in the list, so security.yml never ran: 65 checks reported on that PR and not one was Secret Detection (whole tree). The PR merged green.

The test file carries two GitHub-PAT-shaped fixtures. They are fabricated and harmless, but the scanner never got to say so — and once merged, every later PR whose tree carries the file fails the whole-tree scan, because that scan does run when the PR happens to touch backend Python. Surfaced on #16444 after a refresh onto current main; fixture fix in #16862.

So the gate has both failure directions at once:

• False clean on the PR that introduces the string, which is the only moment anyone can cheaply fix it.
• False blame on unrelated PRs later, which inherit a red they did not cause and cannot fix without touching someone else's file.

Uncovered surface

Anything outside that path list, including: pipeline-scripts/**, scripts/**, repo_tests/**, autobot-frontend/** (except package*.json), autobot-infrastructure/**, docs/**, .github/** other than security.yml itself, and every extensionless root dotfile — the exact case the job's comment cites as its reason for existing.

The daily schedule: trigger is not path-filtered, so a committed secret is caught within a day. That is a real backstop and it is why this is not critical — but it catches it after merge, on main, which is the expensive place.

Acceptance criteria

• A change to any tracked file runs the whole-tree secret scan on its pull request, or the job's comment is corrected to state the real coverage boundary and say which surfaces are only covered by the nightly run.
• A test or guard asserts the two cannot drift apart again — the existing pipeline-scripts/check_workflow_path_filters.py already fails the build when the backend-Python copy stops matching its canonical set, so the pattern exists.
• Verified against the known case: a PR touching only pipeline-scripts/ must show Secret Detection (whole tree) in its checks. security(ci): a malformed token cannot reach a CI log or a traceback (#15204) #16810 is the negative control — it did not.

Options

1. Split secret-detection into its own workflow with no paths: filter. Matches the job's stated intent exactly, costs one short job per PR, and leaves the existing gating for the expensive scanners untouched. My recommendation.
2. Drop paths: from security.yml. Simplest diff, but it also un-gates dependency-security and static-analysis, which the filter exists to keep off unrelated PRs — a real CI cost increase.
3. Add the missing paths. Cheapest, and it preserves the defect: the next directory nobody listed is uncovered again, which is what the job's own comment warns against.

This changes CI cost and behaviour for every PR, so it is an owner/coordinator call rather than mine. Happy to implement whichever is chosen.

Related

Fixture unblock: #16862. Surfaced on #16444. Same shape as #16793 and #16855 — a guard accurate about what it says and wrong about what it actually does.

🤖 Generated with Claude Code