golem15/summercms
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(quick-261106-d20): fonoteka.go uploads named after the sniffed type, attach.UploadWriterOptions exported, 2 todos closed

Browse Source
This commit is contained in:
Jakub Zych
2026-11-06 09:38:19 +01:00
parent 903859a131
commit 8a600ca704
4 changed files with 361 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

233
.planning/quick/261106-d20-fix-stored-xss-in-fonoteka-go-uploads-an/261106-d20-PLAN.md Normal file
View File

@@ -0,0 +1,233 @@
---
phase: quick-261106-d20
plan: 01
type: execute
wave: 1
depends_on: []
files_modified:
- modules/lagoon/attach/inline.go
- modules/lagoon/attach/store.go
- modules/lagoon/attach/inline_test.go
- modules/lagoon/README.md
- docs/database/attachments.md
- docs/services/storage.md
- ../fonoteka.go/plugins/golem15/fonoteka/controllers/api/collection_media_controller.go
- ../fonoteka.go/plugins/golem15/fonoteka/controllers/api/store_upload_test.go
- ../fonoteka.go/plugins/golem15/fonoteka/classes/album_files.go
- ../fonoteka.go/plugins/golem15/fonoteka/classes/album_files_test.go
autonomous: true
requirements:
- todo:2026-10-28-fonoteka-uploads-use-disk-extension
- todo:2026-11-06-blob-writers-set-content-disposition
estimate:
tokens: 45000
raw_tokens: 45000
tasks: 2
confidence: low
must_haves:
truths:
- "The framework exports attach.UploadWriterOptions(contentType string) *blob.WriterOptions in modules/lagoon/attach/inline.go. It returns a new value on every call with ContentType set to contentType verbatim and ContentDisposition set to the bare value attachment exactly when attach.IsInlineContentType(contentType) is false; every other field is zero. JPEG, PNG, GIF, WebP and AVIF (parameters and case ignored) get no disposition; text/html, text/xml, image/svg+xml, application/pdf, text/plain and the empty string get attachment"
- "attach.Store keeps writing through the same options: writeBlob in store.go calls attach.UploadWriterOptions, the unexported helper no longer exists, and TestWriteBlobDisposition passes without edits to its assertions. The change is additive: no other exported identifier in modules/lagoon is added, removed or changes its signature"
- "modules/lagoon/README.md (Features bullet and API table row) and docs/database/attachments.md plus docs/services/storage.md (deploy note) describe attach.UploadWriterOptions for code that writes uploads without attach.Store, in the same commit as the code. go vet ./..., go test -short ./..., go test ./cmd/summer -run TestDocsTree and go run ./cmd/summer docs:build --check are green in summercms.go at that commit"
- "In fonoteka.go, storeUpload (collection media) and classes.StorePublicFile (album photos, covers) sniff the content type before naming the file, name the disk file as 22 lowercase hex characters plus a dot plus attach.DiskExtension(contentType, filepath.Ext(clientName)), keep the client name only in file_name, and write the blob with attach.UploadWriterOptions(contentType)"
- "Regression, per call site: a PNG body with a script payload uploaded as x.html is stored as <22 hex>.png with file_name x.html, content_type image/png and no stored Content-Disposition; an HTML body uploaded as x.html and an XML-prologue SVG body uploaded as logo.svg are stored with a .txt disk name and Content-Disposition: attachment; an upload without an extension still gets one (.png for a PNG, .txt for plain text). The blob bytes read back equal the input"
- "Existing fonoteka.go behaviour is otherwise unchanged: content_type stays the sniffed base type, file_size and is_public are as before, and the Docker-backed photo upload smoke tests (Cover.PNG with a PNG body stays .png) still pass. fonoteka.go's go.mod files are not edited (require stays v0.4.4; the new export resolves through the replace to ../summercms.go)"
- "Two commits, one per repository, no co-author tag, files staged by explicit path, nothing pushed or tagged; sm-user-plugin and sm-feedback-plugin are untouched"
artifacts:
- path: modules/lagoon/attach/inline.go
provides: "Exported UploadWriterOptions with a Go doc comment for callers that bypass Store"
contains: "func UploadWriterOptions(contentType string) *blob.WriterOptions"
- path: modules/lagoon/attach/store.go
provides: "writeBlob calls UploadWriterOptions"
contains: "UploadWriterOptions(contentType)"
- path: modules/lagoon/attach/inline_test.go
provides: "TestUploadWriterOptions table test, Docker-free"
contains: "TestUploadWriterOptions"
- path: modules/lagoon/README.md
provides: "Features bullet mention and an attach.UploadWriterOptions API table row"
contains: "attach.UploadWriterOptions"
- path: docs/database/attachments.md
provides: "Storing an upload: paragraph on writing uploads without attach.Store"
contains: "attach.UploadWriterOptions"
- path: docs/services/storage.md
provides: "Deploy note: code that bypasses attach.Store uses attach.DiskExtension and attach.UploadWriterOptions"
contains: "attach.UploadWriterOptions"
- path: ../fonoteka.go/plugins/golem15/fonoteka/controllers/api/collection_media_controller.go
provides: "storeUpload names by sniffed type and writes with the upload writer options"
contains: "attach.DiskExtension("
- path: ../fonoteka.go/plugins/golem15/fonoteka/classes/album_files.go
provides: "StorePublicFile names by sniffed type and writes with the upload writer options"
contains: "attach.UploadWriterOptions("
- path: ../fonoteka.go/plugins/golem15/fonoteka/controllers/api/store_upload_test.go
provides: "TestStoreUploadNamesBySniffedType on memblob, Docker-free"
contains: "TestStoreUploadNamesBySniffedType"
- path: ../fonoteka.go/plugins/golem15/fonoteka/classes/album_files_test.go
provides: "TestStorePublicFileNamesBySniffedType on memblob, Docker-free"
contains: "TestStorePublicFileNamesBySniffedType"
key_links:
- from: modules/lagoon/attach/store.go writeBlob
to: modules/lagoon/attach/inline.go UploadWriterOptions
via: "Store and external writers share one definition of the stored type and disposition"
pattern: "bucket\\.NewWriter\\(writeCtx, key, UploadWriterOptions\\(contentType\\)\\)"
- from: ../fonoteka.go/plugins/golem15/fonoteka/controllers/api/collection_media_controller.go storeUpload
to: modules/lagoon/attach (DiskExtension, UploadWriterOptions)
via: "fonoteka plugin go.mod replace => ../../../../summercms.go (go.work workspace)"
pattern: "attach\\.UploadWriterOptions\\(contentType\\)"
- from: ../fonoteka.go/plugins/golem15/fonoteka/classes/album_files.go StorePublicFile
to: modules/lagoon/attach (DiskExtension, UploadWriterOptions)
via: "StoreAlbumPhoto (album photo upload, cover importer) goes through StorePublicFile"
pattern: "attach\\.DiskExtension\\(contentType, filepath\\.Ext\\(filename\\)\\)"
---
<objective>
Close two linked upload-security todos in one quick task across two repositories.
1. Framework (summercms.go): export the blob writer-options helper of `attach.Store` as `attach.UploadWriterOptions(contentType string) *blob.WriterOptions`, so code that writes an upload to the bucket itself stores the same Content-Type and `Content-Disposition: attachment` for non-inline types (todo 2026-11-06-blob-writers-set-content-disposition, g15office ifdz5o0m). Additive change in a core module, documented in the same commit.
2. fonoteka.go (sibling repo `../fonoteka.go`, its own git repository): its two writers that bypass `attach.Store`, `storeUpload` and `classes.StorePublicFile`, still name the disk file after the client extension (a PNG polyglot uploaded as `x.html` is stored as `<hex>.html` and rendered as HTML by a static server: stored XSS) and store no disposition. Name the file with `attach.DiskExtension` from the sniffed type and write with `attach.UploadWriterOptions` (todo 2026-10-28-fonoteka-uploads-use-disk-extension, g15office cf81chjk).
Purpose: a stored HTML/SVG/polyglot upload never renders on a trusted origin, whether served by a static server (extension-driven) or by a public bucket, CDN or `attach.StaticHandler` (stored-header-driven).
Output: one framework commit in summercms.go, one fix commit in fonoteka.go, Docker-free regression tests in both.
Out of scope (locked by the user): sm-user-plugin avatar and sm-feedback-plugin screenshot writers (image-only inputs, follow-up), bumping fonoteka.go's `require` of the framework, any tag or push. Existing blobs are not rewritten.
</objective>
<execution_context>
@~/.claude/msd-core/workflows/execute-plan.md
@~/.claude/msd-core/templates/summary.md
</execution_context>
<context>
@.planning/STATE.md
@CLAUDE.md
@../fonoteka.go/CLAUDE.md
@.planning/todos/pending/2026-10-28-fonoteka-uploads-use-disk-extension.md
@.planning/todos/pending/2026-11-06-blob-writers-set-content-disposition.md
@modules/lagoon/attach/inline.go
@modules/lagoon/attach/diskext.go
Interfaces the executor needs (already in the codebase):
- `attach.IsInlineContentType(contentType string) bool` (inline.go): true only for image/jpeg, image/png, image/gif, image/webp, image/avif, parameters and case ignored.
- Unexported `attachmentDisposition = "attachment"` (inline.go) and unexported `uploadWriterOptions(contentType string) *blob.WriterOptions` (inline.go, lines 36-44), used once by `writeBlob` in store.go (line 208).
- `attach.DiskExtension(contentType, clientExt string) string` (diskext.go): extension without the dot, never empty. Strips a leading dot and lowercases clientExt. Specific sniffed type: client ext if it belongs to the type, else canonical (PNG named .html becomes png). Generic sniff: client ext unless script-capable or not matching `^[a-z0-9]{1,10}$`, then txt for text/* and bin otherwise.
- `http.DetectContentType` sniffs `<html>...` as text/html, a body starting `<?xml` as text/xml, the PNG signature `\x89PNG\r\n\x1a\n` as image/png, short ASCII as text/plain; both fonoteka call sites already reduce it to the base type with `mime.ParseMediaType`.
- `lagoon.UploadedFile{Filename string; Size int64; Header textproto.MIMEHeader; Open func() (io.ReadCloser, error)}` (modules/lagoon/validate_request.go).
- fonoteka.go is a go.work workspace; the root `./...` does NOT include the plugin module `plugins/golem15/fonoteka` (its own go.mod, `require git.golem15.com/golem15/summercms v0.0.0` + `replace => ../../../../summercms.go`). Run vet/tests in both the root and the plugin directory. The `api` and `classes` test packages have a TestMain that starts Postgres only when not `-short`, so the new memblob tests run under `-short`.
- Existing Docker-backed smoke tests in the plugin root package (`albums_smoke_test.go` TestAlbumPhotoUpload, `collections_smoke_test.go`) upload `Cover.PNG` with a real PNG body and assert the disk name ends in `.png`; that must keep passing.
</context>
<tasks>
<task type="auto" tdd="true">
<name>Task 1: Export attach.UploadWriterOptions with a unit test and docs (summercms.go commit)</name>
<files>modules/lagoon/attach/inline.go, modules/lagoon/attach/store.go, modules/lagoon/attach/inline_test.go, modules/lagoon/README.md, docs/database/attachments.md, docs/services/storage.md</files>
<behavior>
- UploadWriterOptions("image/png"), ("image/jpeg"), ("image/avif") and ("IMAGE/WEBP; charset=binary") return ContentType equal to the input verbatim and an empty ContentDisposition.
- UploadWriterOptions("text/html"), ("text/xml"), ("image/svg+xml"), ("application/pdf"), ("text/plain"), ("application/octet-stream") and ("") return ContentType equal to the input and ContentDisposition "attachment" (bare, no filename parameter).
- Each result is reflect.DeepEqual to a blob.WriterOptions literal with only those two fields set (CacheControl, ContentEncoding, ContentLanguage, ContentMD5, Metadata, BeforeWrite all zero), and two calls return distinct pointers (mutating one result does not change the next).
- The existing TestIsInlineContentType and TestWriteBlobDisposition pass unchanged.
</behavior>
<action>
Before editing, run git status --short on the six paths in this task and stop if another session has uncommitted edits there (CLAUDE.md concurrency rule).
In modules/lagoon/attach/inline.go rename the unexported helper uploadWriterOptions to the exported UploadWriterOptions with the unchanged signature (contentType string) *blob.WriterOptions and unchanged body (new WriterOptions with ContentType set, ContentDisposition set to the attachmentDisposition constant when IsInlineContentType is false). Rewrite its doc comment as exported Go doc starting with the identifier name: it returns the blob writer options of an upload, the content type plus Content-Disposition: attachment (no filename) for every type IsInlineContentType does not report as inline; Store writes with these options; code that writes an upload to the bucket itself, with NewWriter or WriteAll, passes the sniffed type (the value stored in content_type) and names the file with DiskExtension, so its blob is served like one Store wrote; each call returns a new value the caller may extend. Do not change IsInlineContentType, the inline type set or the constant.
In modules/lagoon/attach/store.go change the single call in writeBlob to UploadWriterOptions(contentType). If the Store or writeBlob doc comments name the old helper, update them; no behaviour change.
In modules/lagoon/attach/inline_test.go add TestUploadWriterOptions covering the behavior list (table test, reflect.DeepEqual against the expected blob.WriterOptions value, plus the distinct-pointer check). Write the test first and confirm it fails to compile before the rename (RED), then rename (GREEN).
Docs in the same commit (CLAUDE.md documentation rule, exported API change in a core module):
- modules/lagoon/README.md: in the Attachments Features bullet (line 44), after the Store disposition clause, add that attach.UploadWriterOptions gives code which writes an upload to the bucket without attach.Store the same content type and disposition. Add an API table row for attach.UploadWriterOptions directly after the attach.IsInlineContentType row: the blob writer options of an upload, the content type plus Content-Disposition: attachment for a type attach.IsInlineContentType does not report as inline; attach.Store writes with the same options; pair it with attach.DiskExtension when writing uploads yourself.
- docs/database/attachments.md, section "Storing an upload": after the paragraph that starts with attach.Store writes every file whose type (line 140), add a short paragraph: code that writes an upload to the bucket itself instead of calling attach.Store gets the same protection by sniffing the type first, naming the file with attach.DiskExtension(contentType, clientExt) and passing attach.UploadWriterOptions(contentType) to the bucket writer (NewWriter or WriteAll); file_name keeps the client's name. Do not touch the src= code samples in this page (they are synced from example_test.go, which this task does not change).
- docs/services/storage.md, "Deploy note": in the paragraph that starts Objects written before this change, or by code that bypasses attach.Store, carry no disposition, say that such code should write with attach.UploadWriterOptions (and name files with attach.DiskExtension), and that objects written earlier still need the disposition set on them or the CDN rule.
Keep the prose style of the surrounding text (plain sentences, no planning IDs). Every `pkg.Ident` code span must name an existing identifier; the checker only resolves framework packages, so `blob.WriterOptions` spans are fine.
Commit in summercms.go, staging exactly these six paths with git add followed by the explicit paths (never git add -A; leave .planning/STATE.md, .planning/state.json, .planning/G15OFFICE.md and other sessions' files alone). Message: feat(lagoon/attach): export UploadWriterOptions for blob writers that bypass Store (quick-261106-d20). No co-author tag (user's global CLAUDE.md overrides any attribution reminder).
</action>
<verify>
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./... && go test -short ./modules/lagoon/attach -run 'TestUploadWriterOptions|TestIsInlineContentType|TestWriteBlobDisposition' -v && go test -short ./... && go test ./cmd/summer -run TestDocsTree && go run ./cmd/summer docs:build --check && grep -c '^func UploadWriterOptions(contentType string) \*blob.WriterOptions' modules/lagoon/attach/inline.go && grep -c 'attach.UploadWriterOptions' modules/lagoon/README.md docs/database/attachments.md docs/services/storage.md</automated>
</verify>
<done>attach.UploadWriterOptions is exported with Go doc, Store uses it, TestUploadWriterOptions passes alongside the unchanged existing tests, the README and both docs pages name it, all docs checks pass, and one summercms.go commit contains exactly the six paths.</done>
</task>
<task type="auto" tdd="true">
<name>Task 2: fonoteka.go storeUpload and StorePublicFile name by sniffed type and store the disposition, with regression tests (fonoteka.go commit)</name>
<files>../fonoteka.go/plugins/golem15/fonoteka/controllers/api/collection_media_controller.go, ../fonoteka.go/plugins/golem15/fonoteka/controllers/api/store_upload_test.go, ../fonoteka.go/plugins/golem15/fonoteka/classes/album_files.go, ../fonoteka.go/plugins/golem15/fonoteka/classes/album_files_test.go</files>
<precondition>Task 1's attach.UploadWriterOptions exists in ../summercms.go/modules/lagoon/attach/inline.go (fonoteka.go resolves the framework through its replace directive).</precondition>
<behavior>
Same table for both writers, on memblob.OpenBucket(nil):
- PNG signature followed by a script payload, client name x.html: DiskName matches ^[0-9a-f]{22}\.png$, FileName x.html, ContentType image/png, blob Attributes ContentType image/png and empty ContentDisposition.
- JPEG bytes (or the JPEG magic prefix), client name photo.jpg: DiskName ends .jpg, empty ContentDisposition.
- An HTML body with a script tag, client name x.html: DiskName ends .txt, ContentType text/html, ContentDisposition attachment.
- A body starting with an XML prologue followed by an svg element with a script, client name logo.svg: DiskName ends .txt, ContentType text/xml, ContentDisposition attachment.
- A PNG with client name scan (no extension) ends .png; plain ASCII text with client name README ends .txt with attachment.
- For every case the bytes read back from attach.BlobKey(DiskName) equal the input, FileSize equals the input length and IsPublic is true. StorePublicFile with a nil bucket still returns an error.
</behavior>
<action>
Before editing, run git -C ../fonoteka.go status --short and stop if any of the four paths has uncommitted edits from another session.
Write both test files first (RED: the HTML and polyglot cases fail against the current code, which keeps the client extension and stores no disposition).
- controllers/api/store_upload_test.go, package api: TestStoreUploadNamesBySniffedType calls storeUpload(t.Context(), bucket, lagoon.UploadedFile{Filename, Size, Open returning io.NopCloser over a bytes.Reader}) for each behavior case and checks the returned *attach.File and bucket.Attributes on attach.BlobKey(f.DiskName).
- classes/album_files_test.go, package classes: TestStorePublicFileNamesBySniffedType calls StorePublicFile(t.Context(), bucket, name, data) with the same cases, plus the nil-bucket error case.
Both use only memblob, no DB, so they run under -short. Build test bodies inline (PNG signature bytes, a JPEG magic prefix or the existing testJPEG helper in classes, HTML/SVG/plain strings).
Then fix both writers (per todos cf81chjk and ifdz5o0m):
- controllers/api/collection_media_controller.go storeUpload: after reading data, sniff contentType first (keep the existing http.DetectContentType plus mime.ParseMediaType base reduction), then build diskName as the 22-character hex of 11 random bytes plus a dot plus attach.DiskExtension(contentType, filepath.Ext(upload.Filename)). The extension is never empty now, so drop the conditional append. Pass attach.UploadWriterOptions(contentType) to bucket.NewWriter instead of the inline blob.WriterOptions literal. FileName stays upload.Filename, ContentType stays the sniffed base type.
- classes/album_files.go StorePublicFile: same ordering and naming with attach.DiskExtension(contentType, filepath.Ext(filename)), and pass attach.UploadWriterOptions(contentType) to bucket.WriteAll.
- Update both doc comments: 22 hex characters plus the extension attach.DiskExtension derives from the sniffed type (the client name is kept only in file_name, so a PNG named x.html is stored as .png), and a type that is not an inline raster image is stored with Content-Disposition: attachment. Note in StorePublicFile's comment that this deliberately differs from WinterCMS File::fromData, which keeps the client extension.
- Remove imports that become unused (strings, and blob if only the literal used it; StorePublicFile still takes *blob.Bucket so blob likely stays). Let go vet decide.
Do not change response payloads, the system_files columns written, size limits, the image guard or any other call site. Do not touch go.mod, go.sum, go.work or the sm-user-plugin / sm-feedback-plugin code.
Run the Docker-backed photo upload smoke tests in the plugin root package as well (go test . -run 'Photo' in plugins/golem15/fonoteka without -short; if the Docker socket is denied, run through sudo -u golem per summercms.go CLAUDE.md). If Docker is unavailable entirely, record that in the summary instead of failing the task.
Commit in fonoteka.go only, staging exactly the four paths with git -C ../fonoteka.go add followed by the explicit paths (never stage submodule files). Message: fix(fonoteka): name uploads after the sniffed type and store non-inline types as attachments (quick-261106-d20). No co-author tag.
In the SUMMARY note that fonoteka.go's root go.mod still requires git.golem15.com/golem15/summercms v0.4.4 and resolves attach.UploadWriterOptions only through the replace to ../summercms.go; the require must be raised to the next framework tag that contains the export once one is cut. Also list the out-of-scope follow-up (sm-user-plugin avatar, sm-feedback-plugin screenshot writers) as open.
</action>
<verify>
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/fonoteka.go/plugins/golem15/fonoteka && go vet ./... && go test -short ./controllers/api ./classes -run 'TestStoreUploadNamesBySniffedType|TestStorePublicFileNamesBySniffedType' -v && go test -short ./... && cd /media/nvme/dev/golem15/summercms.io/summercms/fonoteka.go && go vet ./... && go test -short ./... && grep -c 'attach.UploadWriterOptions(contentType)' plugins/golem15/fonoteka/controllers/api/collection_media_controller.go plugins/golem15/fonoteka/classes/album_files.go && git diff --quiet HEAD -- go.mod plugins/golem15/fonoteka/go.mod</automated>
</verify>
<done>Both fonoteka.go writers name files from the sniffed type and store attachment disposition for non-inline types; both regression tests pass under -short; go vet and go test -short are green in the fonoteka.go root and plugin module; the photo upload smoke tests pass (or Docker absence is recorded); one fonoteka.go commit contains exactly the four paths; go.mod files are unchanged.</done>
</task>
</tasks>
<threat_model>
## Trust Boundaries
| Boundary | Description |
|----------|-------------|
| client upload → fonoteka.go storeUpload / StorePublicFile | The client controls the file name and the bytes; only the sniffed type is trustworthy |
| uploads directory → static web server | nginx and similar pick the Content-Type from the disk extension, so an extension taken from the client decides how the bytes render |
| bucket object → public bucket / CDN / attach.StaticHandler | These send the headers stored with the object; a missing disposition lets HTML or SVG render inline on that origin |
| framework core module → consuming apps and plugins | lagoon/attach is used by many applications; changes must be additive |
## STRIDE Threat Register
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|-----------|----------|-----------|----------|-------------|-----------------|
| T-q261106d20-01 | Elevation of privilege | fonoteka.go storeUpload and StorePublicFile: PNG polyglot uploaded as x.html stored as <hex>.html and rendered as HTML by a static server (stored XSS) | high | mitigate | Task 2: disk name from attach.DiskExtension(sniffed, client ext); regression case asserts ^[0-9a-f]{22}\.png$ for the x.html PNG in both writers |
| T-q261106d20-02 | Elevation of privilege | HTML or SVG blob written by fonoteka.go served inline by a public bucket, CDN or attach.StaticHandler(Public) | medium | mitigate | Task 1 exports attach.UploadWriterOptions; Task 2 writes with it; regression cases assert Content-Disposition attachment for text/html and the text/xml SVG, and none for PNG/JPEG |
| T-q261106d20-03 | Information disclosure / Tampering | Client file name reaching a response header through the stored disposition | low | mitigate | The disposition is the bare attachment constant with no filename parameter; the client name stays only in the file_name column (unchanged). TestUploadWriterOptions asserts the exact value |
| T-q261106d20-04 | Tampering | Breaking change in the shared lagoon/attach module | medium | mitigate | Additive rename of an unexported helper to an exported one with the same body; Store behaviour unchanged, TestWriteBlobDisposition and TestIsInlineContentType unedited and green; go test -short ./... in summercms.go |
| T-q261106d20-05 | Elevation of privilege | Blobs already in fonoteka.go buckets with client extensions and no disposition, and the sm-user-plugin avatar / sm-feedback-plugin screenshot writers | low | accept | Out of scope by user decision: existing objects are covered by the storage deploy note (set the disposition or a CDN rule); the two plugin writers accept images only (defence-in-depth follow-up) |
| T-q261106d20-SC | Tampering | npm/pip/cargo/go module installs | low | accept | No new dependencies: gocloud.dev/blob is already required in both modules; no package-manager install task |
</threat_model>
<verification>
- summercms.go: go vet ./..., go test -short ./..., go test ./cmd/summer -run TestDocsTree and go run ./cmd/summer docs:build --check are green at the Task 1 commit; that commit contains exactly the six framework paths.
- fonoteka.go: go vet ./... and go test -short ./... green in both the repository root and plugins/golem15/fonoteka; the Docker-backed photo upload smoke tests pass (or Docker absence is recorded); the commit contains exactly the four paths and no go.mod/go.sum/go.work change.
- No co-author tag in either commit; nothing pushed or tagged; STATE.md, state.json, G15OFFICE.md and other sessions' files untouched by these commits.
</verification>
<success_criteria>
- A PNG uploaded to fonoteka.go as x.html is stored as <22 hex>.png through both storeUpload and StorePublicFile.
- An HTML or SVG upload to fonoteka.go is stored under a .txt disk name with Content-Disposition: attachment.
- attach.UploadWriterOptions is an exported, documented, unit-tested framework helper, and attach.Store's stored headers are unchanged.
- The summary records that fonoteka.go's framework require (v0.4.4) must be raised to the next framework tag before building without the replace, and that the sm-user-plugin / sm-feedback-plugin writers remain a follow-up.
</success_criteria>
<output>
Create `.planning/quick/261106-d20-fix-stored-xss-in-fonoteka-go-uploads-an/261106-d20-SUMMARY.md` when done
</output>

128
.planning/quick/261106-d20-fix-stored-xss-in-fonoteka-go-uploads-an/261106-d20-SUMMARY.md Normal file
View File

@@ -0,0 +1,128 @@
---
phase: quick-261106-d20
plan: 01
subsystem: lagoon/attach, fonoteka.go uploads
tags: [security, xss, uploads, attachments, content-disposition]
status: complete
requires:
- attach.DiskExtension (existing)
- attach.IsInlineContentType (existing)
provides:
- attach.UploadWriterOptions (exported framework helper)
- fonoteka.go storeUpload / classes.StorePublicFile named by sniffed type with stored disposition
affects:
- ../fonoteka.go (collection media upload, album photo upload, cover importer)
tech-stack:
added: []
patterns:
- "Writers that bypass attach.Store: sniff type, name with attach.DiskExtension, write with attach.UploadWriterOptions"
key-files:
created:
- ../fonoteka.go/plugins/golem15/fonoteka/controllers/api/store_upload_test.go
- ../fonoteka.go/plugins/golem15/fonoteka/classes/album_files_test.go
modified:
- modules/lagoon/attach/inline.go
- modules/lagoon/attach/store.go
- modules/lagoon/attach/inline_test.go
- modules/lagoon/README.md
- docs/database/attachments.md
- docs/services/storage.md
- ../fonoteka.go/plugins/golem15/fonoteka/controllers/api/collection_media_controller.go
- ../fonoteka.go/plugins/golem15/fonoteka/classes/album_files.go
decisions:
- "attach.UploadWriterOptions is the exported form of the former unexported uploadWriterOptions (same body); Store behaviour unchanged"
- "fonoteka.go StorePublicFile deliberately diverges from WinterCMS File::fromData: the disk extension follows the sniffed type, not the client name"
metrics:
duration: ~20m
completed: 2026-11-06
actuals:
tokens: 9000
tasks: 2
commits: 1
plan_head_before: 9fa740e8b79362cafcfc5fd1cbc6df03a51172e7
plan_head_after: 903859a1310e754ab96fe8226dcbc5622fec6803
---
# Quick 261106-d20: Fix stored XSS in fonoteka.go uploads and export attach.UploadWriterOptions Summary
The framework now exports `attach.UploadWriterOptions`, the stored Content-Type plus `Content-Disposition: attachment` for non-inline types. fonoteka.go's two writers that bypass `attach.Store` (`storeUpload`, `classes.StorePublicFile`) now name disk files with `attach.DiskExtension` from the sniffed type and write with those options. A PNG polyglot uploaded as `x.html` is stored as `<hex>.png`, and HTML/SVG bodies are stored as `.txt` with an attachment disposition.
## Commits
| Repo | Hash | Message |
|------|------|---------|
| summercms.go | 903859a | feat(lagoon/attach): export UploadWriterOptions for blob writers that bypass Store (quick-261106-d20) |
| fonoteka.go | 07e820d | fix(fonoteka): name uploads after the sniffed type and store non-inline types as attachments (quick-261106-d20) |
The ledger `commits: 1` counts summercms.go only (plan_head_before/after are summercms.go hashes). fonoteka.go moved from 0e1114e to 07e820d, which is one commit. Neither commit has a co-author tag. Files were staged by explicit path, and nothing was pushed or tagged.
## Task 1: framework (summercms.go)
- `modules/lagoon/attach/inline.go`: renamed `uploadWriterOptions` to `UploadWriterOptions` with an exported Go doc comment. The body is unchanged.
- `modules/lagoon/attach/store.go`: `writeBlob` calls `UploadWriterOptions(contentType)`.
- `modules/lagoon/attach/inline_test.go`: added `TestUploadWriterOptions`. It is a table test using `reflect.DeepEqual` against a `blob.WriterOptions` with only ContentType/ContentDisposition set, plus checks for distinct pointers and that mutating one result does not affect the next.
- Docs, in the same commit:
- `modules/lagoon/README.md`: a Features clause and an API table row after `attach.IsInlineContentType`.
- `docs/database/attachments.md`: a paragraph in "Storing an upload".
- `docs/services/storage.md`: the Deploy note now says bypassing code should use `attach.UploadWriterOptions` and `attach.DiskExtension`.
- TDD: I wrote the test first, and it failed to compile (`undefined: UploadWriterOptions`). After the rename it passed.
- Verification at 903859a, all green:
- `go vet ./...`
- `go test -short ./...`
- `TestUploadWriterOptions`, `TestIsInlineContentType` and `TestWriteBlobDisposition`, the last two with no edits
- `go test ./cmd/summer -run TestDocsTree`
- `go run ./cmd/summer docs:build --check` ("no problems found")
## Task 2: fonoteka.go
- `storeUpload` and `StorePublicFile` sniff the type first, then name the file as 22 hex characters plus `.` plus `attach.DiskExtension(contentType, filepath.Ext(<client name>))`, and write with `attach.UploadWriterOptions(contentType)`.
- What stays the same:
- `file_name` keeps the client name.
- `content_type` is still the sniffed base type.
- `file_size` and `is_public` are as before.
- I updated both doc comments. StorePublicFile's comment notes the deliberate divergence from `File::fromData`. I dropped the unused `strings` imports.
- New memblob regression tests, which run Docker-free under `-short`: `TestStoreUploadNamesBySniffedType` (package api) and `TestStorePublicFileNamesBySniffedType` (package classes, plus the nil-bucket error case).
- Test cases:
- PNG polyglot as `x.html` is stored as `.png` with no disposition.
- JPEG as `photo.jpg` is stored as `.jpg` with no disposition.
- HTML as `x.html` is stored as `.txt` with attachment.
- `<?xml` SVG as `logo.svg` is stored as `.txt`, with `text/xml` and attachment.
- PNG named `scan` is stored as `.png`.
- Plain text named `README` is stored as `.txt` with attachment.
- In every case the bytes read back are identical to the input.
- RED: before the fix, 5 of 6 cases failed in both tests (`.html`/`.svg`/no-extension disk names, missing disposition). GREEN: all of them pass after the fix.
- Verification:
- Plugin module (`plugins/golem15/fonoteka`): `go vet ./...` and `go test -short ./...` are green.
- Repo root: `go vet ./...` and `go test -short ./...` are green.
- `go.mod` and `plugins/golem15/fonoteka/go.mod` are unchanged.
- Docker-backed smoke tests: Docker was available and `sudo -u golem` was not needed. `go test . -run Photo` passed `TestAlbumPhotoUpload` and `TestCollectionPhotoUpload`, so Cover.PNG still ends in `.png`. Without `-short`, all subpackages passed: `classes`, `classes/auth`, `classes/csv`, `classes/discogs`, `console`, `controllers/api`, `middleware`, `models` and `updates`.
## go.mod note
fonoteka.go's root `go.mod` still has `require git.golem15.com/golem15/summercms v0.4.4`. It resolves `attach.UploadWriterOptions` only through `replace git.golem15.com/golem15/summercms => ../summercms.go`. Once a framework tag containing 903859a is cut, the require must be raised to that tag before fonoteka.go is built without the replace. No go.mod was edited in this task.
## Open follow-up (out of scope by user decision)
- The sm-user-plugin avatar writer and the sm-feedback-plugin screenshot writer still write blobs without `attach.Store`. Both accept images only. The follow-up is to switch them to `attach.DiskExtension` plus `attach.UploadWriterOptions` as defence in depth.
- Existing blobs in fonoteka.go buckets keep their client extensions and have no disposition. The storage deploy note covers this: set the disposition on the objects, or add a CDN rule.
## Deviations from Plan
None in the code: both tasks were executed as written.
## Deferred Issues (pre-existing, out of scope)
- In the full non-short run of the fonoteka.go plugin root package, 4 tests fail: `TestPhase8RedMCPMe`, `TestOAuthToolsMeRouteIsolation` (unexpected `GET /_user/api/v1/token/me` route from the sm-user-plugin bump), `TestMustChangePasswordLock` (login 401) and `TestPhase14Edges/prune-cutoff-90-days-then-one-second-older`. I checked that they are pre-existing: with the two writer files temporarily reverted to HEAD, the same 4 fail. None of them touches uploads.
- `classes/postgres_test.go` `testShort()` matches only `-test.short` exactly. Plain `go test -short` passes `-test.short=true`, so the classes package starts Postgres even under `-short`. CLAUDE.md warns about exactly this. The new test still passes, but it is not Docker-free in that package until `testShort` is fixed (`controllers/api` and `classes/auth` already handle `-test.short=true`).
## Threat Flags
None: no new network endpoints, auth paths or schema changes. T-q261106d20-01..04 are mitigated as planned, and T-05 is accepted as the follow-up above.
## Self-Check: PASSED
- FOUND: modules/lagoon/attach/inline.go (`func UploadWriterOptions(contentType string) *blob.WriterOptions`)
- FOUND: ../fonoteka.go/plugins/golem15/fonoteka/controllers/api/store_upload_test.go
- FOUND: ../fonoteka.go/plugins/golem15/fonoteka/classes/album_files_test.go
- FOUND: summercms.go commit 903859a
- FOUND: fonoteka.go commit 07e820d

0
.planning/todos/pending/2026-10-28-fonoteka-uploads-use-disk-extension.md → .planning/todos/done/2026-10-28-fonoteka-uploads-use-disk-extension.md
View File

0
.planning/todos/pending/2026-11-06-blob-writers-set-content-disposition.md → .planning/todos/done/2026-11-06-blob-writers-set-content-disposition.md
View File