feat: Security-First Linting — opt-in, isolated, verified tool provisioning - #11
Merged
Merged
Conversation
…ng foundation
Security-First Linting groundwork (design + the security-critical core), per the
synthesis of the ChatGPT (honest positioning) + Qwen (security-first linting /
auto-install) analyses. This commit lands ONLY the foundation; tools are wired in
follow-up waves.
- docs/security-first-provisioning-design.md: the brainstorm/design (safety
model, verified official sources, research caveats, implementation order).
- src/provision.js: ensureTool() fail-open cascade (PATH -> isolated cache ->
opt-in download -> "unavailable"), with the security guarantees that make
auto-download safe for a security tool:
* OPT-IN: nothing downloads unless `provision: true` (CLI will gate this behind
--provision-tools + informed consent).
* PINNED OFFICIAL HOSTS: assertOfficialUrl() requires HTTPS + an allow-listed
official host; URLs are derived only from a tool's pinned official repo.
* INTEGRITY: SHA-256 verified against the vendor checksums file BEFORE the
artifact is installed/executed; mismatch or non-official URL => rejected,
never executed.
* ISOLATED + FAIL-OPEN: installs only into a caller-provided cache
(<project>/.csreview/bin, gitignored), never global/sudo; degrades to lower
confidence rather than crashing.
All network/fs/exec is injected. TOOL_REGISTRY pins gitleaks/gosec/trivy to
their official repos (gitleaks/gitleaks, securego/gosec, aquasecurity/trivy).
tests: test/provision.test.js (13) — host pin, checksum verify, platform asset
matching, and every cascade branch incl. checksum-mismatch + non-official-URL
rejection (install never called).
Gate: 139/139 tests, lint clean, typecheck exit 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…/Trivy Pure normalizers over each tool''s JSON output -> canonical Finding schema (confidence TOOL-ONLY; engine dedup promotes to CONFIRMED when a tool corroborates the heuristic detector -> cuts false positives). Secret values are ALWAYS redacted (Gitleaks Secret/Match, Trivy Secrets.Match, and Bandit B105-B107 / gosec G101 code lines) so CSReview never copies a raw secret into its report. runSecurityTool() takes an injected exec (no process spawned in tests) and runs each tool read-only with a SECURITY-ONLY profile + JSON output; the audited rootDir is passed only as an argv element (no shell interpolation). Fail-open. tests: test/security-tools.test.js (8) — mapping + CWE + redaction (raw secret never present) for all four tools, empty/garbage tolerance, runner fail-open, and the no-shell-interpolation guard. Gate: 146/146 tests, lint clean, typecheck exit 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…roboration -> CONFIRMED) - selectSecurityTools(): picks candidates by stack (always gitleaks + trivy; bandit for Python; gosec for Go). - gatherSecurityToolFindings(): orchestrates ensure(provision/resolve) + run per candidate, aggregating findings; fully fail-open per tool (injected ensure/run). - runAnalysis(): new optional options.gatherSecurityTools hook. When provided (the CLI composes the real one behind --provision-tools), its findings are merged into the dedup so a tool secret/finding at the same file:line:cwe as the heuristic detector is promoted to CONFIRMED — the false-positive-reduction mechanism. Absent => no-op, default scan unchanged. result.securityTools lists per-tool availability/source. tests: selectSecurityTools, gather aggregation + fail-open, and a runAnalysis integration proving detector+gitleaks corroboration yields CONFIRMED. Gate: 150/150 tests, lint clean, typecheck exit 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ime I/O src/provisionRuntime.js: the real (network/fs/exec) I/O for ensureTool, all composable + injectable — probeOnPath, cacheLookup, fetchLatestRelease (official GitHub API), downloadBuffer/downloadText (assertOfficialUrl enforced before fetch), installFromArchiveAsync (extract -> isolated .csreview/bin -> chmod, writes only under cacheDir + a temp dir), and makeSecurityToolGatherer() composing ensureTool + runSecurityTool for runAnalysis. CLI: --provision-tools is the opt-in gate. When set, CSReview prints an INFORMED CONSENT banner (which tools, the official sources, SHA-256 verification, isolated .csreview/bin, never global) before anything downloads, then wires the gatherer into runAnalysis and reports per-tool ran/skipped status. Without the flag the suite is off and the default scan is unchanged. provision.js awaits onPath/cache so real async probes work (no-op for sync test mocks). tests: test/provision-runtime.test.js (6) — version probe, cache lookup, release mapping/fail-open, non-official-host refusal before fetch, isolated install, and a gatherer run-on-PATH with provision:false never hitting the API. Gate: 156/156 tests, lint clean, typecheck exit 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nding (+ SKILL honesty) Owner requirement: the GitHub landing must tell users these tools are needed and that CSReview can install them (opt-in) for a faithful, low-false-positive result. - README: new prominent "Security tooling CSReview relies on (and can install for you)" section ABOVE the SKILL mirror (renders first on GitHub), listing every tool with its OFFICIAL source, the opt-in (--provision-tools) install behavior (official source + SHA-256 verify + isolated gitignored .csreview/bin + fail open), how to decline, and the honest positioning (local security alignment, not a pentest; read-only on source). - SKILL.md: new "Opt-in security-tool provisioning" paragraph; install policy refined so provisioning into an isolated, gitignored .csreview/ is explicitly allowed with consent while never installing as a project dependency / globally / modifying the audited source. README mirror re-synced (equality test green). - scanner.js: ignore **/.csreview/** so a scanned project's provisioned tool cache is never itself scanned. .gitignore: add .csreview/. Gate: 156/156 tests, lint clean, typecheck exit 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 33603145 | Triggered | Generic Password | e9975dc | csreview/test/security-tools.test.js | View secret |
| 33594677 | Triggered | Generic Password | 9b9deb7 | csreview/test/security-tools.test.js | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
…ing path
Adversarial review verdict was FIX-FIRST (no Critical; core verify-before-exec,
opt-in gating, and host-pin confirmed sound). Remediated:
H1 (zip-slip / arbitrary write via assetName, Windows): installFromArchive used
assetName.split('/').pop() which kept backslashes, letting a hostile official
asset name escape the temp dir on Windows. Now uses path.basename (strips both
separators) + asserts the archive path stays inside the temp dir. Temp dir is now
mkdtemp (unpredictable) instead of a guessable name (N2).
H2 (PowerShell command injection in defaultExtract): the Windows .zip branch
interpolated the archive path into a PowerShell -Command string. Replaced with a
pure-argv `tar -xf` (Windows 10+ bsdtar extracts zip) — no shell, no
interpolation anywhere on the extraction path.
M1 (host pin not re-asserted across redirects): downloads now follow redirects
MANUALLY and re-run assertOfficialUrl on EVERY hop (bounded), so an official host
cannot 3xx to an arbitrary host. + size cap (L2, MAX_DOWNLOAD_BYTES).
M2 (bandit advertised but unreachable): bandit added to the registry as a
PATH-only tool (PyPI, not a GitHub single binary) — used if already installed,
never auto-downloaded; ensureTool returns a `pip install bandit` hint otherwise.
CLI banner clarified (auto-installed: Gitleaks/Trivy/gosec; used-if-installed:
Bandit). README/design doc aligned.
L1 (secret in tool free-text): gitleaks/trivy-secret name/description now run
through scrubSecrets() so a raw secret can never reach a finding field even if a
tool echoes it into rule text. N3: findBinary uses lstat + skips symlinked
archive members. N4: provisioning writes .csreview/.gitignore so the audited
project's cache is gitignored. L3: design-doc consent claim softened to match the
implemented banner.
tests: +4 (H1 containment, M1 redirect re-assert, M2 bandit pathOnly, L1 scrub).
Gate: 160/160 tests, lint clean, typecheck exit 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 3, 2026
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.
Security-First Linting: opt-in, isolated, integrity-verified tool provisioning
Synthesizes the three parallel analyses (ChatGPT honest positioning + Qwen Security-First Linting + Qwen "Trio de Ouro" auto-install). CSReview's fidelity — results faithful to the objective, far fewer false positives — comes from running real best-in-class security tools and corroborating them with the heuristic detector (tool + detector ⇒
CONFIRMED). So CSReview can now provision the tools it needs, made safe.Design doc:
csreview/docs/security-first-provisioning-design.md.Safety model (auto-download is the highest-risk capability — designed so it can't be one)
--provision-tools; the CLI first prints what it will install, from where, and that it's isolated.assertOfficialUrl(HTTPS + allow-listed host); URLs derived only from each tool's pinned official repo..csreview/bin/; never global, neversudo, never a project dependency, never modifies the audited source.What's included
src/provision.js— registry (gitleaks/gosec/trivy pinned to official repos) +ensureToolcascade + checksum/host guards (all I/O injected).src/securityTools.js— normalizers (Gitleaks/Bandit/gosec/Trivy → canonical Finding, secrets always redacted) + injectable runners +gatherSecurityToolFindings.src/provisionRuntime.js— real network/fs/exec I/O +makeSecurityToolGatherer.runAnalysisopt-ingatherSecurityToolshook → findings merged into dedup ⇒ corroborationCONFIRMED.--provision-toolsflag + consent banner + per-tool ran/skipped reporting..csreview/.Verified official sources (researched 2026-06-03)
semgrep/semgrep · google/osv-scanner · gitleaks/gitleaks · aquasecurity/trivy · PyCQA/bandit · securego/gosec.
Caveats acted on:
fabpot/local-php-security-checkeris archived → prefercomposer audit;eslint-plugin-securityis FP-prone → weighted lower / supplementary (not in the first wave).Tests
+30 tests (provision foundation, normalizers + redaction, runtime I/O host-pin, gather fail-open, runAnalysis corroboration ⇒ CONFIRMED). 156/156 pass · lint clean · typecheck exit 0. Real downloads are never performed in tests (all I/O injected); the real path runs only on user opt-in.
Not in this PR (honest scope)
First wave wires Gitleaks/Bandit/gosec/Trivy; eslint-plugin-security / composer audit / brakeman / ruff remain in the recommendation matrix on the same foundation. Do not merge before review (security-sensitive capability).
🤖 Generated with Claude Code