fix(core): result-not-source dangerous-type forcing for attachment downloads (#66)#95
Merged
Merged
Conversation
…wnloads (#66) The download Content-Type policy previously only forced the ?contentType param; extension inference from ?filename and the stored _attachment@1 mimeType were unforced spec gaps, making the hardened path the one an attacker doesn't have to use. Adds resolveAttachmentDownloadContentType(), which computes the candidate type from whichever source wins and applies a safe-list to that result regardless of source, plus isSafeAttachmentContentType()/inferContentTypeFromFilename() and the nosniff header constants, all exported from @haverstack/core so server implementations share one policy instead of each re-deriving it. Updates docs/spec.md's Download section with the normative result-not-source rule, the safe-list, and the nosniff requirement.
#66) Adds attachmentDownloadFixtures: dangerous type x each of the three Content-Type sources -> forced (application/octet-stream + Content-Disposition: attachment), safe type x each source -> passes through, nosniff present on every response. Kept as a separate fixture type from ConformanceFixture since GET /attachments/:fileId has no JSON body -- what's being pinned is response headers, not a body shape.
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
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.
Summary
Closes #66.
Stack/ScopedStack.getAttachment()never took?contentType/?filenamequery params — the three-source Content-Type resolution this issue describes (?contentType, extension inference from?filename, stored_attachment@1mimeType) is entirely an HTTP-server concern that doesn't exist anywhere in this repo today. So instead of aStackmethod change (like #65), this adds a shared pure implementation of the policy in@haverstack/coreforhaverstack/server(and any other server implementation) to consume, rather than each re-deriving a security-relevant safe-list independently — the same "written once, not a per-server convention" principle already applied to other invariants in this issue set (#67, #68).packages/core/src/attachment-download.ts(new):resolveAttachmentDownloadContentType()computes the candidate type (?contentType→ extension inference from?filename→ storedmimeType→application/octet-stream) and applies a safe-list to that result, never to the source — so a server can't leave a hole by routing a dangerous type through whichever source forcing used to skip. Also exportsisSafeAttachmentContentType(),inferContentTypeFromFilename(),FORCED_CONTENT_TYPE, and theNOSNIFF_HEADER_NAME/NOSNIFF_HEADER_VALUEconstants. Safe-list (per the issue's recommendation over an ad hoc deny-list):image/*(exceptimage/svg+xml),video/*,audio/*,application/pdf,text/plain,application/octet-stream— checked against the MIME type's base, case-insensitively, so parameter tricks (text/html; charset=...) or casing can't bypass it. Everything else forcesContent-Type: application/octet-stream+Content-Disposition: attachment.docs/spec.md: rewrote the Download section with the normative result-not-source forcing rule, the safe-list, and theX-Content-Type-Options: nosniffrequirement on every attachment response (forced or not).packages/conformance-fixtures: addedattachmentDownloadFixtures— dangerous type × each of the three sources → forced; safe type × each source → passes through; nosniff present on every response. Kept as a separate fixture type fromConformanceFixturesinceGET /attachments/:fileIdhas no JSON body — what's being pinned is response headers, not a body shape. Not wired intoadapter-api's existing fixture loops, sinceAPIAdapter.getAttachment()has no query-param/header surface to test against them (a plain bytes fetch); these are forhaverstack/server's own test suite (and for our in-repo unit tests against the pure function directly).Test plan
pnpm -r run test— 620 tests passing across all packages, including 32 new tests inpackages/core/tests/attachment-download.test.tscovering safe-list membership (incl. theimage/svg+xmlexclusion, case-insensitivity, parameter stripping), extension inference (incl. dangerous extensions inferring their real type, since forcing applies downstream), source precedence, and forcing per sourcepnpm --filter @haverstack/core run build/typecheck/lintpnpm --filter @haverstack/conformance-fixtures run build/typecheck/lintGenerated by Claude Code