Skip to content

feat(enrichment): flag insecure HTTP security-header settings in iac-misconfig#3380

Closed
luciferlive112116 wants to merge 1 commit into
JSONbored:mainfrom
luciferlive112116:feat/enrichment-iac-http-security-headers-v2
Closed

feat(enrichment): flag insecure HTTP security-header settings in iac-misconfig#3380
luciferlive112116 wants to merge 1 commit into
JSONbored:mainfrom
luciferlive112116:feat/enrichment-iac-http-security-headers-v2

Conversation

@luciferlive112116

Copy link
Copy Markdown
Contributor

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):

kind fires on risk
csp-unsafe-inline 'unsafe-inline' in a Content-Security-Policy permits inline scripts/styles (weakens XSS defense)
csp-unsafe-eval 'unsafe-eval' in a Content-Security-Policy permits eval() (weakens XSS defense)
hsts-disabled Strict-Transport-Security … max-age=0 disables HSTS (browsers stop enforcing HTTPS)
referrer-policy-leak Referrer-Policy: unsafe-url leaks the full URL (path + query) cross-origin
cookie-not-httponly httpOnly: false exposes the cookie to JavaScript (XSS can read it)

Supersedes the closed #3370, whose CSP rules matched unsafe-inline/unsafe-eval anywhere and so could flag
a non-CSP line such as unsafe-inline = false. This version anchors every rule to its own header token on
the same line (the CSP rules now require Content-Security-Policy), with negative tests for a bare
unsafe-inline/unsafe-eval line.

Why these are false-positive-safe: each rule requires its own header token on the same line as the
weakening value, so an unrelated line that merely contains the value is not flagged: a normal
Cache-Control: max-age=0 caching directive does not fire the HSTS rule, and a stray unsafe-inline = false
config key does not fire the CSP rule (both asserted in the negative test). Within a header that IS being set,
the matched value is the weakening itself — the secure value uses a different token the regex never matches
('self', max-age=31536000, strict-origin-when-cross-origin, httpOnly: true), also asserted to produce
no finding.

No existing rule is modified, and the analyzer's finding schema is {file, line, kind} — the kind union is
not part of the analyzer descriptor, so analyzer-metadata.json and 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

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

  • git diff --check
  • npm run typecheck
  • npm run rees:test — the review-enrichment build + analyzer suite (see note below)
  • npm run test:coverage (N/A — this analyzer is in review-enrichment/, outside the root src/** Codecov scope)
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • Ran locally: git diff --check (clean), the review-enrichment TypeScript build (exit 0 — which proves the
    render switch is exhaustive over the 5 new kinds), and the analyzer suite via node --test. The
    iac-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 full node --test run's
    only failures are the two upload-sourcemaps tests (they shell out to the Sentry CLI, absent on this dev
    box), which fail identically on unmodified main.
  • Not run locally: the UI, root typecheck, and the metadata:check step of rees:test. This change adds only
    finding kinds and rules, not any analyzer descriptor field, so the committed analyzer-metadata.json / UI
    mirror are unchanged (a local regeneration produces a zero-content diff) and metadata:check passes on CI
    (Linux). On this Windows dev box metadata:check reports a spurious line-ending difference; it fails
    identically on unmodified main. analyzer-metadata.json was NOT modified.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section 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.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

Notes

  • Detection-only additions: 5 new stateless rules following the existing single-line pattern; no existing rule,
    threshold, or descriptor changed, so current findings and analyzer-metadata.json are unaffected. Each new
    kind reports only file:line + the public-safe kind, never the matched line content.
  • The HSTS and Referrer-Policy rules are anchored to their own header token so an unrelated line carrying the
    same value (e.g. Cache-Control: max-age=0) is never flagged.

@superagent-security

Copy link
Copy Markdown

Superagent didn't find any vulnerabilities or security issues in this PR.

@gittensory-orb gittensory-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.5x multiplier. label Jul 5, 2026
@gittensory-orb

gittensory-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - reject/close recommended

Review updated: 2026-07-05 05:43:03 UTC

4 files · 1 AI reviewer · 1 blocker · readiness 80/100 · CI green · clean

🛑 Suggested Action - Reject/Close

  • AI reviewers agree on a likely critical defect: review-enrichment/src/analyzers/iac-misconfig.ts:116 treats `Content-Security-Policy-Report-Only` as `Content-Security-Policy` because `\bContent-Security-Policy\b` has a word boundary before the hyphen, so a line like `+Content-Security-Policy-Report-Only: script-src 'unsafe-inline'` is incorrectly flagged as weakening enforced CSP
  • change the token to exclude the report-only suffix, e.g. `const CSP_UNSAFE_INLINE_RE = /\bContent-Security-Policy\b(?!-Report-Only)[^\n]*\bunsafe-inline\b/i
  • ` and apply the same fix to `CSP_UNSAFE_EVAL_RE`. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Review summary
The change cleanly wires five new IaC misconfiguration kinds through scanning, typing, rendering, and focused positive/negative tests. The same-line anchoring addresses the broad false-positive problem described in the PR, but the CSP header regex still accepts the Report-Only header form, which is not an enforced security-header setting and will produce a false finding on a common deployment pattern.

Blockers

  • review-enrichment/src/analyzers/iac-misconfig.ts:116 treats `Content-Security-Policy-Report-Only` as `Content-Security-Policy` because `\bContent-Security-Policy\b` has a word boundary before the hyphen, so a line like `+Content-Security-Policy-Report-Only: script-src 'unsafe-inline'` is incorrectly flagged as weakening enforced CSP; change the token to exclude the report-only suffix, e.g. `const CSP_UNSAFE_INLINE_RE = /\bContent-Security-Policy\b(?!-Report-Only)[^\n]*\bunsafe-inline\b/i;` and apply the same fix to `CSP_UNSAFE_EVAL_RE`.
Nits — 3 non-blocking
  • nit: review-enrichment/test/iac-misconfig.test.ts should add a negative case for `Content-Security-Policy-Report-Only` once the CSP matcher is tightened, because this is exactly the same class of header-token anchoring regression the PR is trying to prevent.
  • In `review-enrichment/src/analyzers/iac-misconfig.ts`, tighten both CSP regexes with `(?!-Report-Only)` immediately after the enforced header token.
  • In `review-enrichment/test/iac-misconfig.test.ts`, add `+ Content-Security-Policy-Report-Only: script-src 'unsafe-inline'` and the `unsafe-eval` variant to the safe list.

Why this is blocked

  • review-enrichment/src/analyzers/iac-misconfig.ts:116 treats `Content-Security-Policy-Report-Only` as `Content-Security-Policy` because `\bContent-Security-Policy\b` has a word boundary before the hyphen, so a line like `+Content-Security-Policy-Report-Only: script-src 'unsafe-inline'` is incorrectly flagged as weakening enforced CSP; change the token to exclude the report-only suffix, e.g. `const CSP_UNSAFE_INLINE_RE = /\bContent-Security-Policy\b(?!-Report-Only)[^\n]*\bunsafe-inline\b/i;` and apply the same fix to `CSP_UNSAFE_EVAL_RE`.
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ✅ Linked #3370
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ❌ 5/25 Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload ✅ 10/10 Author activity: 142 registered-repo PR(s), 79 merged, 21 issue(s).
Contributor context ✅ Confirmed Gittensor contributor luciferlive112116; Gittensor profile; 142 PR(s), 21 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: luciferlive112116
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 142 PR(s), 21 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Await review-lane availability.
  • Refresh registry data or choose a registered active repo.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 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.

  • Re-run Gittensory review

@gittensory-orb

gittensory-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown

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:116 treats `Content-Security-Policy-Report-Only` as `Content-Security-Policy` because `\bContent-Security-Policy\b` has a word boundary before the hyphen, so a line like `+Content-Security-Policy-Report-Only: script-src 'unsafe-inline'` is incorrectly flagged as weakening enforced CSP; change the token to exclude the report-only suffix, e.g. `const CSP_UNSAFE_INLINE_RE = /\bContent-Security-Policy\b(?!-Report-Only)[^\n]*\bunsafe-inline\b/i;` and apply the same fix to `CSP_UNSAFE_EVAL_RE`.). 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.5x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant