fix(artifact): harden /artifact response headers (review H1/L4/L6)#115
Merged
Conversation
Three release-review findings on the public, pre-OAuth /artifact route, all in serveArtifact: - H1 (svg hardening): the route re-emitted the stored content-type inline with no nosniff/disposition/CSP. SVG (limner_vectorize + recraft vector_illustration) is active content served same-origin with the OAuth flow. Now every response sets X-Content-Type-Options: nosniff, and any non-raster type (notably image/svg+xml) is forced to a download under a no-script sandbox CSP (default-src 'none'; sandbox) so it can't execute script in the auth origin. Raster images (png/jpeg/webp/avif/gif) stay inline. (Adversarially downgraded to Low — no stealable credential on the origin — but cheap defense-in-depth.) - L4 (decode 500): decodeURIComponent on the key was unguarded, so a malformed escape like /artifact/%zz threw URIError -> generic 500 on an unauthenticated route. Now caught and returned as 404, matching the other invalid-key paths. - L6 (cache TTL): cache-control was a flat private/max-age=86400 while signed URLs default to a 1h signature TTL, so a private-cached copy outlived the capability. max-age is now derived from the remaining signature lifetime (exp - now), clamped to [0, 24h]; unsigned dev URLs keep the 24h default. Tests: malformed-key 404; raster inline+nosniff; svg attachment+sandbox+nosniff; signed cache-control tracks the TTL. Suite 238 -> 242. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Jim Vinson <jim@vinson.org>
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.
Batch A of the max-strength review fixes. Three findings on the public, pre-OAuth
/artifactroute, all inserveArtifact(artifact.ts).Content-Disposition: attachment+Content-Security-Policy: default-src 'none'; sandbox; all responses getX-Content-Type-Options: nosniff. Raster (png/jpeg/webp/avif/gif) stays inline.decodeURIComponenton the key was unguarded →/artifact/%zzthrewURIError→ 500. Now caught → 404.cache-controlwas a flatprivate, max-age=86400while signed URLs default to a 1h TTL.max-agenow tracks the remaining signature lifetime (exp - now, clamped to [0, 24h]); unsigned dev URLs keep 24h.* H1 was originally flagged High; the adversarial-verify pass downgraded it to Low — the origin holds no stealable credential (OAuth is bearer-token-in-header; the only cookie is an HttpOnly/SameSite=Strict CSRF nonce). This is cheap defense-in-depth, not a release blocker.
Tests
Malformed-key → 404; raster inline +
nosniff; svg →attachment+ sandbox CSP +nosniff; signedcache-controltracks the TTL. Suite 238 → 242. Typecheck + lint clean.🤖 Generated with Claude Code