feat(enrichment): flag insecure HTTP security-header settings in iac-misconfig#3370
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Caution 🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥 🛑 Gittensory review result - reject/close recommendedReview updated: 2026-07-05 05:20:29 UTC
🛑 Suggested Action - Reject/Close
Review summary Blockers
Nits — 5 non-blocking
Why this is blocked
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
|
Gittensory is closing this pull request on the maintainer's behalf (AI reviewers agree on a likely critical defect: review-enrichment/src/analyzers/iac-misconfig.ts:132 and review-enrichment/src/analyzers/iac-misconfig.ts:529 flag `unsafe-inline`/`unsafe-eval` anywhere in an admitted config file, so an added non-CSP line such as `+ unsafe-inline = false` or a documented allowlist value produces a `csp-*` finding even though no `Content-Security-Policy` is being configured; change the regex/check to require the CSP header token on the same parsed line, or give me a concrete reason this analyzer intentionally treats the token alone as CSP context, e.g. `const CSP_UNSAFE_INLINE_RE = /\bContent-Security-Policy\b[^\n]*\bunsafe-inline\b/i;` and add the matching negative test.). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed. |
Summary
The IaC-misconfig analyzer already covers Kubernetes, Dockerfile, Docker Compose, and TLS-bypass
misconfigurations. This adds 5 HTTP security-header rules for insecure header values that appear in the
config files the analyzer already scans (
nginx*.conf,.conf, YAML/Helm ingress annotations,.toml,.json):csp-unsafe-inline'unsafe-inline'in a Content-Security-Policycsp-unsafe-eval'unsafe-eval'in a Content-Security-Policyeval()(weakens XSS defense)hsts-disabledStrict-Transport-Security … max-age=0referrer-policy-leakReferrer-Policy: unsafe-urlcookie-not-httponlyhttpOnly: falseWhy these are false-positive-safe: in every rule the matched value is the weakening itself — there is no
"safe value" form of the same line.
unsafe-inline/unsafe-evalare Content-Security-Policy-exclusive keywords(a
unsafe_inlineidentifier with an underscore does not match). The HSTS and Referrer-Policy rules requiretheir own header token to be present on the line, so a normal
Cache-Control: max-age=0caching directive isnot matched by the HSTS rule (asserted in the negative test). The secure value of each setting uses a
different token the regex never matches (
'self',max-age=31536000,strict-origin-when-cross-origin,httpOnly: true), which the negative test asserts produces no finding.No existing rule is modified, and the analyzer's finding schema is
{file, line, kind}— thekindunion isnot part of the analyzer descriptor, so
analyzer-metadata.jsonand the generated UI mirror are unchanged.The render-brief switch is TypeScript-exhaustive, so each new kind is compiler-forced to have a public-safe
explanation.
No linked issue: additive detection-coverage that extends an existing multi-domain analyzer along its own
established lines; each rule is a self-evident, named HTTP-hardening check (OWASP Secure Headers) with no public
API/schema/deploy surface change — fits the repo's
preferred(not required) linked-issue policy.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run typechecknpm run rees:test— the review-enrichment build + analyzer suite (see note below)npm run test:coverage(N/A — this analyzer is inreview-enrichment/, outside the rootsrc/**Codecov scope)npm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
git diff --check(clean), the review-enrichment TypeScript build (exit 0 — which proves therender switch is exhaustive over the 5 new kinds), and the analyzer suite via
node --test. Theiac-misconfig file passes 21/21: a table test asserting each of the 5 settings produces exactly one finding
of its own kind, and a negative test asserting the secure counterpart of each — including a normal
Cache-Control: max-age=0(which must NOT fire the HSTS rule) — produces none. The fullnode --testrun'sonly failures are the two
upload-sourcemapstests (they shell out to the Sentry CLI, absent on this devbox), which fail identically on unmodified
main.metadata:checkstep ofrees:test. This change adds onlyfinding kinds and rules, not any analyzer descriptor field, so the committed
analyzer-metadata.json/ UImirror are unchanged (a local regeneration produces a zero-content diff) and
metadata:checkpasses on CI(Linux). On this Windows dev box
metadata:checkreports a spurious line-ending difference; it failsidentically on unmodified
main.analyzer-metadata.jsonwas NOT modified.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.Notes
threshold, or descriptor changed, so current findings and
analyzer-metadata.jsonare unaffected. Each newkind reports only
file:line+ the public-safe kind, never the matched line content.same value (e.g.
Cache-Control: max-age=0) is never flagged.