bug(hooks): get_staged_files ignored its file-type filter in CI, and a parse crash was reported as a Redis violation

Root-caused while diagnosing a red code-quality check on PR #13945. Fixed there; filing so the
scope of what was broken is on record, because it affected every hook using the shared helper.

The bug

autobot-infrastructure/shared/scripts/hooks/lib/_common.sh:42-52:

get_staged_files() {
    local pattern="$1"; shift
    if [ "$#" -gt 0 ]; then
        printf '%s\n' "$@"     # <-- pattern never applied
        return
    fi
    git diff --cached --name-only --diff-filter=ACMRT | grep -E "$pattern" || true
}

The argv branch returned the file list verbatim, without applying $pattern. Both invocation
paths are documented right above it (pre-commit passes no args; CI wrappers pass argv), so the
filter worked locally and was silently bypassed in CI — the one place nobody watches it.

What it produced

pre-commit-no-direct-redis declares get_staged_files '\.py$' "$@". In CI it therefore received
.vue files and handed them to a Python tokenizer. WorkflowCanvas.vue raised:

IndentationError: unindent does not match any outer indentation level
  VIOLATION autobot-frontend/src/components/workflow/WorkflowCanvas.vue:
COMMIT BLOCKED: 1 direct Redis connection(s) found

A Vue file with no Redis in it, reported as a direct Redis connection. The hook caught
tokenize.TokenError but IndentationError is a SyntaxError, so the exception escaped, the
interpreter exited non-zero, and the shell wrapper counted the crash as a violation.

Two independent defects, either of which alone is enough to mis-report:

The filter bypass — wrong files reach the checker.
The crash-as-violation — a parse failure is indistinguishable from a finding.

Fixed in PR #13945

_common.sh applies $pattern on both branches, so the two invocation paths agree.
The redis hook catches (TokenError, SyntaxError, ValueError) and prints SKIP <path>: not parseable as Python (<ExcType>) to stderr — visible, so a genuinely broken Python file is not
silently unscanned.

Verified both directions: a real redis.Redis( still blocks (exit 1), an unparseable file skips
without a violation, and the previously-red file set now exits 0.

Why this is worth its own issue

This is the fifth defect of the same shape found today: a check that reports something other
than what it measured. The others are the org chart's is_human branch (#13936), the Kanban board
(#13993), the canvas pan gesture (#13996), and the contact no-login test (#13969 review). Related:
#13200 already records that 3 of 4 pre-commit checkers are untested — this is what that costs.

Ask

Audit the other hooks calling get_staged_files for the same latent mis-scan (the filter bypass
was repo-wide; only the redis hook happened to crash loudly enough to notice).
Give the shared hook lib a test that pins the argv branch applies the pattern — the fix is one
line and will regress silently without one.

Delivered in #14883 as part of the CI-guard cluster (#14371, #13984, #12986, #14034). #14181 was measured and deliberately excluded — see the comment there for where it actually stands. Discovered and filed rather than dropped: #14884, #14885.

perf(ci): prefilter the staged-file scan too, so both entry points ag…

fix(hooks): make the argv and git file-type filters agree, and pin it (…

refactor(ci): one hardcoded-value detector carrying all three rule se…

perf(ci): prefilter the staged-file scan too, so both entry points ag…

refactor(ci): consolidate duplicated, drifted and never-run CI guards (…

Delivered and merged.

PR: refactor(ci): consolidate duplicated, drifted and never-run CI guards (#14371, #13984, #12986, #14034) #14883 — squash-merged as ac754f5e0
CI: 47 success / 9 skipped / 0 failures, deduped to the latest run per check name on head a9d3c9805, including all 12 python-suite shards. Up to date with base at merge.
Review: independent code-reviewer pass. It reconstructed all three pre-merge detectors from the rebase target and diffed them byte-for-byte rather than trusting the PR's union table.

Verified in base after merge:

scripts/lib/hardcoded-value-rules.sh          PRESENT
pipeline-scripts/check_baseline_no_growth.sh  PRESENT
pipeline-scripts/hardcoded_values_baseline.txt PRESENT
scripts/lib/git-scope.sh                      PRESENT

Union preserved — rules a survivor-pick would have deleted:

Rule	Existed only in	Now
VM IP, any fourth octet	CI scanner	kept wide; the hook's narrow `(19\|2[0-5])` did not win
Ports 8443 / 5432 / 9090	CI scanner's `PORT_PATTERN` — declared and never grep'd	wired in for the first time
Per-value SSOT key suggestions, generic model regex, hardcoded-URL rule	dormant fork	carried
Account identity (`User=`, `chown`, sudoers)	CI scanner	carried
`offset=0` as WARNING not VIOLATION	hook	severity preserved

scripts/lib/hardcoded_value_rules_test.py fails by name per former detector, so a substitution cannot pass as a merge.

One HIGH finding was raised in review and fixed before merge, not deferred. The baseline suppressing 1977 findings had no anti-growth check: a PR could hardcode a value and append a matching baseline line in the same commit, and ssot-coverage would report ssot_violations=0. check_baseline_no_growth.sh now diffs the baseline against the base ref and fails on any new key or increased count, failing closed on an unresolvable base, an unreadable baseline, or a parse error. Mutation-proved in a scratch repo both directions — growth red, shrinkage green.

A real credential exposure was found and fixed while doing it. Two postgresql://user:pass@host/db values were already sitting in the generated baseline, a tracked file that also reaches public CI output. One shared _hv_redact_userinfo now serves both the DSN and URL rules; scheme and host survive so findings stay actionable. Verified in base: zero user:pass@ patterns remain, and the baseline totals are unchanged at 1434 lines / 1977 — the redaction dropped no finding.

Also fixed: api-wiring — a required check — omitted autobot_shared/**/*.py from its path filter while audit_api_wiring.py imports two modules from that tree, so it reported green over an audit that never ran (#12986).

#14181 was deliberately not closed by this PR — measured at 8/10 on one criterion with the remainder blocked by epic #7442. Follow-ups filed: #14884, #14885, #14891.

Navigation Menu

Repository navigation

bug(hooks): get_staged_files ignored its file-type filter in CI, and a parse crash was reported as a Redis violation #14034

The bug

What it produced

Fixed in PR #13945

Why this is worth its own issue

Ask

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

bug(hooks): get_staged_files ignored its file-type filter in CI, and a parse crash was reported as a Redis violation #14034

Description

The bug

What it produced

Fixed in PR #13945

Why this is worth its own issue

Ask

Activity

mrveiss commented on Aug 23, 2026

mrveiss commented on Aug 23, 2026

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions