Repository navigation
tech-debt: three separate hardcoded-value detectors have drifted (pipeline-scripts, autobot-infrastructure, pre-commit hook) #14371
Description
Activity
👋 Auto-triage could not confidently classify this issue (prose matched 5 skill areas; too ambiguous to label). Could a maintainer add the appropriate labels (
frontend,backend,infrastructure,docs,testingandgood-first-issue,intermediate,advanced)?Delivered in #14883 as part of the CI-guard cluster.
The three detectors are collapsed into one rule set (
scripts/lib/hardcoded-value-rules.sh) carrying the union of all three — the PR body has the full per-rule table showing which canonical rule now covers each, including detector 1'sPORT_PATTERN, which was declared and never referenced by any scan function. The dormantautobot-infrastructure/shared/scripts/detect-hardcoded-values.shis retired: nothing invoked it, and its only references were inHARDCODING_PREVENTION.mdunder a path prefix that never existed. Docs repointed.Two thin entry points read the shared rules — the CI tree scanner and the pre-commit hook — differing only in where the lines come from and what they do with the verdict.
Measurement worth recording: running the merged rules over the tree found 1977 findings where the CI detector reported 0. Every one was already true; the rules that would have found them lived in a fork nothing invoked, or in a hook that only ever sees staged files. They are recorded in
pipeline-scripts/hardcoded_values_baseline.txt, which only ever shrinks and is audited by a new blocking CI step.Discovered while doing it and filed rather than dropped: #14884 (8 hook tests that cannot run), #14885 (
verify-generated-typesself-skipping onautobot_shared-only changes).- added 6 commits that reference this issue
on Aug 23, 2026 Rebased onto
d70a1f99eafter #14877 (35f48ba92) touched the one overlapping file. Resolved as a union of capabilities: the fail-loud abort idiom travels to where the retired detector's logic went, while thessot-config.shdependency deliberately does not — itset -a-loads the master.envand exportsAUTOBOT_REDIS_PASSWORD, and the consolidated detector prints matched source lines into a public CI log. Reasoning recorded inscripts/lib/hardcoded-value-rules.sh, not left to be re-derived.Reading #14877's three new guards turned up a reach gap, filed as #14891:
shell_lib_sources_resolve_test.pywalks*.shonly, so all 16 extensionless hooks underautobot-infrastructure/shared/scripts/hooks/sit outside it, two of them with live|| true-class defects.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 12python-suiteshards. Up to date with base at merge. - Review: independent
code-reviewerpass. 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 PRESENTUnion 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 winPorts 8443 / 5432 / 9090 CI scanner's PORT_PATTERN— declared and never grep'dwired 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=0as WARNING not VIOLATIONhook severity preserved scripts/lib/hardcoded_value_rules_test.pyfails 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-coveragewould reportssot_violations=0.check_baseline_no_growth.shnow 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/dbvalues were already sitting in the generated baseline, a tracked file that also reaches public CI output. One shared_hv_redact_userinfonow serves both the DSN and URL rules; scheme and host survive so findings stay actionable. Verified in base: zerouser: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 — omittedautobot_shared/**/*.pyfrom its path filter whileaudit_api_wiring.pyimports 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.
- PR: refactor(ci): consolidate duplicated, drifted and never-run CI guards (#14371, #13984, #12986, #14034) #14883 — squash-merged as
Post-merge follow-up: the blocking stale-baseline audit shipped in #14883 red-lined
Dev_new_guion the very next merge (#14890). Symptom fixed by #14910; root cause and a concrete removal-only--prune-baselineproposal filed as #14912. My design decision, measured at ~8% of the repo (776 of 9510 tracked files) able to trigger it.
Found while working #14316 (
pipeline-scripts/detect-hardcoded-values.shnever scanned shell scripts).Problem
There are three independent implementations of "detect hardcoded values" in this repo:
pipeline-scripts/detect-hardcoded-values.sh— used by.github/workflows/ssot-coverage.yml(CI, non-required check). Fixed in bug(ci): the hardcoded-value detector never scans shell scripts, which is where hardcoded accounts and paths actually live #14316 to also scan*.sh/*.yml/*.yamland to broaden its account rule.autobot-infrastructure/shared/scripts/detect-hardcoded-values.sh— a second copy with its own SSOT IP/port/model mapping tables, its own exclude list, and its own scan functions (scan_ssot_ips,scan_ssot_ports,scan_ssot_models,scan_urls). It references$PROJECT_ROOT/srcand$PROJECT_ROOT/backend, directory names that do not exist in this repo (the real trees areautobot-backend,autobot-frontend/src, etc.), and it is not invoked by any workflow or pre-commit hook — dormant and, on the evidence of the path mismatch, has been dormant long enough to drift out of sync with the repo layout. bug(ci): the hardcoded-value detector never scans shell scripts, which is where hardcoded accounts and paths actually live #14316 added it topipeline-scripts/detect-hardcoded-values.sh's exclude list (as an SSOT-definition file, matching howssot_mappings.pyis already excluded) so its own literal IP table isn't flagged as a violation — that is the only change made to it.autobot-infrastructure/shared/scripts/hooks/pre-commit-hardcoded-values— a THIRD implementation, wired as the actualdetect-hardcoded-valuespre-commit hook (.pre-commit-config.yaml,types: [python, ts, vue]). This is the one that actually blocks commits today.Three forks of the same concept is exactly the pattern
feedback_consolidate_never_forkexists to catch. Each has its own pattern list, its own exclude list, and (based on #14316's findings) each has independently missed things the others catch.Suggested next step
Measure which of the three is closest to canonical (the pre-commit hook is the one with actual enforcement power, so it's the strongest candidate), then either:
Do not delete #2 without first confirming (measure, don't assume) that nothing depends on it being present on disk even though nothing invokes it — e.g. documentation referencing its path.
Refs #14316