fix(rules): hardcoded_tmp stops teaching its own false positive; corpus + inline directives honoured - #881
Merged
Conversation
Contributor
|
Warning Review limit reachedNext included review available in 17 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (5)
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. Comment |
…us + inline directives honoured The rule's remediation said "use mktemp", yet `mktemp -d /tmp/x.XXXXXX` fired the rule, and mktemp is the wrong cure for a pid file the next run must re-find. Now: * reason text distinguishes pid/state files (per-user XDG ladder) from scratch files (mktemp -d, no /tmp template, trap cleanup); * skip_comment_lines + new skip_if_line_matches (~r/\bmktemp\b/) guard; * "content_patterns" joins @default_exemptions for the training corpus. The key is the EMITTED rule_module (cli.ex normalises content findings to "content_patterns"); a "cicd_rules" key would be vacuous; * `hypatia: allow <module>/<rule>` is now honoured by the content engine under either module spelling (was silently inert; only the bare `hypatia:ignore <rule>` needle worked). Rule ids are atoms, so the id is stringified before inline_allowed?/4; * line 1 no longer reads the LAST line as its "previous line" (Enum.at(lines, -1) wraparound); * .hypatia-ignore header and CLAUDE.md document the two-spelling trap. test/hardcoded_tmp_pipeline_test.exs runs the real pipeline (collect_findings -> normalise -> suppress). Four mutants each killed: exemption key renamed (4 red), mktemp skip removed (1 red), n>1 guard reverted (1 red), inline_allowed? clause removed (3 red). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK
…anner matches fragments The two readers of this file ask different questions. The gate is the stricter one on purpose (per-path ledger), so the header now says so instead of letting a directory fragment quiet only half the pipeline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK
hyperpolymath
force-pushed
the
fix/hardcoded-tmp-rule-cure
branch
from
September 30, 2026 10:08
05ade6d to
07806f8
Compare
hyperpolymath
changed the base branch from
fix/pin-integrity-latent-logic
to
main
September 30, 2026 10:08
hyperpolymath
enabled auto-merge (squash)
September 30, 2026 10:11
hyperpolymath
added a commit
that referenced
this pull request
Sep 30, 2026
Resolves conflicts after #875 squash-merged and #881/#882 landed: cicd_rules hardcoded_tmp takes main's mktemp skip; workflow_hardening keeps with_local_scripts; scanner_suppression test takes main's fixture-based form. mix test: 1713 tests, 0 failures. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65
hyperpolymath
added a commit
to hyperpolymath/standards
that referenced
this pull request
Sep 30, 2026
…1076) Part C of the launcher `/tmp` cure (origin: a Hypatia `content_patterns/hardcoded_tmp` alert on launch-scaffolder#46). ## launcher-standard — deed + adoc in lock-step - `:pid-file-pattern` → `${XDG_RUNTIME_DIR:-${XDG_STATE_HOME:-$HOME/.local/state}}/launch-scaffolder/{app-name}/server.pid` - `:log-file-pattern` → `${XDG_STATE_HOME:-$HOME/.local/state}/launch-scaffolder/{app-name}/server.log` The runtime hunk is copied **verbatim** from launch-scaffolder `main`'s baked `standards/launcher-standard_praxis.deed` (#54/#58/#62). Its generator already emits this ladder. The old `${TMPDIR:-/tmp}` last resort was the CWE-377 target named by the standard's own rationale: a predictable name in a world-writable dir lets another user choose which PID `stop` kills. The adoc template, the `--disinteg` list, the debugging checklist, logging and Security sections all move with it. **No `:standard-version` bump, deliberately.** launch-scaffolder already bakes this ladder at 0.4.0. A bump would desync the two copies and make `check-launcher-standard-currency.sh` fail every 0.4.0 claim. Launchers on the old ladder are now non-conforming by design; the A8 re-mint sweep reaches them. **Out of scope, noted:** the baked copy has also grown `platforms`, `lifecycle-phases` and `metadata-block.encoding` clauses that this canonical file lacks. That is a separate reconciliation. ## QUICKSTART It taught `PID_FILE="/tmp/myapp-server.pid"` and friends in seven places, contradicting the standard in the same directory. This is the likely seed of the ~17 shipped `/tmp` launchers. All are replaced; the `hardcoded_tmp` regex `["'/]tmp/` now matches nothing in the file. ## EXEMPTION-MECHANISMS The document said `.hypatia-ignore` is *"never read by anything"*, and told reviewers to reject it. It also said Hypatia *"ignores comments"*. Both are false. The new **Layer 2a** covers: - the two consumers and how each matches (scanner: substring fragment; governance gate: whole-line exact path); - why the gate is deliberately the stricter one; - the emitted-module trap (`content_patterns/…`, not `cicd_rules/…`); - the suppression ladder. The anti-pattern list now rejects directory, wildcard and wrong-module lines, and invented pragmas, instead of the file itself. Why the gate matcher is **not** loosened to substring matching: gate ⊂ scanner, so the mismatch can only false-*block*, never false-pass. Containment would let a `…:src/` line absorb every future banned file. A census of 266 local `.hypatia-ignore` files found **0** directory, wildcard or `/`-terminated lines for the two gate rules, so no repo is currently hit by the divergence. ⚠ Inline `hypatia: allow` on content-pattern rules needs hyperpolymath/hypatia#881 (armed). ### Added after review (e0c96f7, a530b71) QUICKSTART Step 1 copies `comprehensive-launcher-template.sh`, which still wrote pid/log to `/tmp` and created no directory — so the doc and the file it hands the reader disagreed, and the XDG ladder would fail on the first `nohup … > "$LOG_FILE"`. The comprehensive, dustfile and e-grade templates now use the ladder and `mkdir -p -m 0700` the leaf directory before first write (SC2174 is intended: only the per-app leaf needs 0700). README checklist and a soft-attach.sh usage example no longer teach `/tmp`. After this, the only `/tmp` mentions under `docs/UX-standards` and `launcher/` are launcher-standard.adoc’s explicit prohibitions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A Hypatia
hardcoded_tmpalert on launch-scaffolder#46 exposed a real CWE-377 defect in the launcher generator. That was fixed upstream in launch-scaffolder #54/#58/#62. The rule itself was part of the problem:PID_FILE="/tmp/x.pid"d=$(mktemp -d /tmp/foo.XXXXXX), the rule's own advice# comment mentioning "/tmp/"/tmpline undertests/fixtures/,test/,lib/rules/,scripts/fix-scripts/# hypatia: allow content_patterns/hardcoded_tmp -- …Changes
${XDG_RUNTIME_DIR:-${XDG_STATE_HOME:-$HOME/.local/state}}/<app>/), from scratch files (mktemp -dwith no/tmptemplate, plustrap … EXIT). The old text said "use mktemp", which is the wrong cure for a pid file.skip_comment_lines: true, plus a new generic rule keyskip_if_line_matches(here~r/\bmktemp\b/)."content_patterns", the key the findings are emitted under incli.ex. A"cicd_rules"key would be vacuous.hypatia: allow <module>/<rule>is now honoured by the content engine under either module spelling. Rule ids are atoms, so they are stringified beforeinline_allowed?/4.Enum.at(lines, -1)wraps around)..hypatia-ignoreheader and.claude/CLAUDE.mdnow document the two-spelling trap and the directive behaviour.Not changed, deliberately
applies_tostays["*.sh"]. Widening it to*.rswould flag#[cfg(test)]literals undersrc/, and cicd_rules has no Rust comment stripping.hypatia: allowform is still not wired for content patterns. CLAUDE.md now says so.Trade-off, stated plainly
After this PR, fixtures under
tests/are no longer scanned for this rule. On the launch-scaffolder side, the generator's own literal-pinned tests (DEFAULT_PID_LINE) are now the detector for a/tmpregression.Verification
test/hardcoded_tmp_pipeline_test.exshas 16 tests running the real pipeline (collect_findings→ normalise → suppress), never a hand-built finding.Four mutants were each killed:
n > 1guard revertedinline_allowed?clause removedFull suite: 1689 tests, 0 failures.
mix compile --warnings-as-errors --forceis clean.🤖 Generated with Claude Code
https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK