SecurityHeadersInterceptor CSP/HSTS/X-Frame defaults drift from baseline #3
Loading…
Add table
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Location:
include/oatpp-authkit/interceptor/SecurityHeadersInterceptor.hpp:22-37The shipped CSP allows
'unsafe-inline'onscript-srcandstyle-src;X-Frame-OptionsisSAMEORIGIN;img-src/connect-srcuse wildcards; noPermissions-Policyheader is sent. The documented baseline (docs/security-baseline.md) prescribes strict defaults: nounsafe-inline,frame-ancestors 'none', no wildcardhttps:inimg-src, nowss:/ws:inconnect-src,X-Frame-Options: DENY, plus aPermissions-Policyheader.Either align the interceptor with the baseline or expose constructor args so consumers can opt in. From audit #1.
Agent Evaluation
Feasibility: Straightforward header rewrite plus optional constructor knobs. The interceptor is a single 40-line class with no consumers depending on its CSP value (fewo-webapp already overrides it via
CspInterceptor, palibu/webapp-template/scaffold use it as-is so will pick up whatever ships). The risk is breaking a downstream that quietly relied on the lax defaults — but the explicit purpose of this library is hardened defaults, so tightening is the right direction.Impact: Medium. Closes a doc-vs-code gap that quietly weakens every consumer that doesn't override. Strict defaults mean apps inherit the protection without thinking; opt-in for relaxations.
Effort: Small.
Recommendation: Accept.
Implementation plan
docs/security-baseline.md: drop'unsafe-inline'fromscript-src/style-src, drophttps://unpkg.com, narrowimg-srcto'self' data:, narrowconnect-srcto'self', setframe-ancestors 'none'.X-Frame-Options: DENYinstead ofSAMEORIGIN(matchesframe-ancestors 'none').Permissions-Policyheader per baseline (accelerometer=(), camera=(), geolocation=(), …).Strict-Transport-Security— keepmax-age=63072000but dropincludeSubDomainsfrom the default (apex-clobbering risk noted in audit #1). Consumers can opt back in.struct CspOverride { std::string scriptSrc, styleSrc, imgSrc, connectSrc, frameAncestors; }— fields default to empty meaning "use the strict baseline".test/test_security_headers.cppasserting (a) default config emits no'unsafe-inline', (b) overrides reach the wire. Reuse the harness bootstrapped in #2.Decision needed
Check one (edit this comment):
CspInterceptor). Smallest API surface.CspOverridector — keeps the library useful for consumers that want one knob without writing a new interceptor. Slightly larger API; aligns with how downstream typically wants "strict + one exception".IAuthPolicy::cspOverride(path)so consumers can relax CSP for/swagger/*etc. without writing a new interceptor. Most flexible but adds another virtual to the policy interface.Evaluated #3 — Small, recommend accept; decision checkbox for strict-only / strict+ctor / per-path hook.
Re-evaluation
Re-checking before implementation: the issue is in the
acceptedqueue but the original evaluation listed three options (A — strict only, B — strict +CspOverridector, C — per-path policy hook) without a default recommendation, and no checkbox is ticked. The three shapes give meaningfully different long-term API surfaces, so guessing risks rework.Recommended default if you'd like me to proceed without further input: Option A. Rationale:
CspInterceptor(the #410 work). Smallest API surface, no virtuals onIAuthPolicy, no struct knob.IAuthPolicy, which then ripples to every consumer test fixture.Please tick a box on the original evaluation comment (or reply with the chosen letter) and I'll proceed.
Re-evaluated #3 — accepted but decision unticked; recommend Option A, awaiting choice before implementing.
Implemented in
0d23124— strict baseline + CspOverride ctor (Option B). All 3 ctest tests pass. Note: did not tag v0.3.6 yet because owner has #6 (ObjectMapper refactor) in flight in the same working tree; tag will land once that work is committed.