--- phase: 14-domain-jobs-and-external-integrations reviewed: 2026-10-04T01:03:11Z depth: standard files_reviewed: 99 files_reviewed_list: - summercms.go/cmd/summer/main.go - summercms.go/cmd/summer/parity.go - summercms.go/examples/hello/main.go - fonoteka.go/app/app.go - fonoteka.go/main.go - fonoteka.go/parity/check_corpus.go - fonoteka.go/plugins.gen.go - fonoteka.go/plugins/golem15/fonoteka/classes/ai_config_resolver.go - fonoteka.go/plugins/golem15/fonoteka/classes/album_recognition.go - fonoteka.go/plugins/golem15/fonoteka/classes/album_write_service.go - fonoteka.go/plugins/golem15/fonoteka/classes/cover_importer.go - fonoteka.go/plugins/golem15/fonoteka/classes/csv_canonical_matcher.go - fonoteka.go/plugins/golem15/fonoteka/classes/csv_import_service.go - fonoteka.go/plugins/golem15/fonoteka/classes/discogs/applicator.go - fonoteka.go/plugins/golem15/fonoteka/classes/discogs/client.go - fonoteka.go/plugins/golem15/fonoteka/classes/discogs/cover_fetcher.go - fonoteka.go/plugins/golem15/fonoteka/classes/discogs/import_resolver.go - fonoteka.go/plugins/golem15/fonoteka/classes/discogs/input_parser.go - fonoteka.go/plugins/golem15/fonoteka/classes/discogs/mapper.go - fonoteka.go/plugins/golem15/fonoteka/classes/discogs/price_suggestion.go - fonoteka.go/plugins/golem15/fonoteka/classes/discogs/rate_limiter.go - fonoteka.go/plugins/golem15/fonoteka/classes/discogs/rate_store.go - fonoteka.go/plugins/golem15/fonoteka/classes/discogs/scorer.go - fonoteka.go/plugins/golem15/fonoteka/classes/gates.go - fonoteka.go/plugins/golem15/fonoteka/console/prune_notifications.go - fonoteka.go/plugins/golem15/fonoteka/console/reindex.go - fonoteka.go/plugins/golem15/fonoteka/controllers/api/album_cover_fetch_controller.go - fonoteka.go/plugins/golem15/fonoteka/controllers/api/credentials_controller.go - fonoteka.go/plugins/golem15/fonoteka/controllers/api/discogs_import_controller.go - fonoteka.go/plugins/golem15/fonoteka/controllers/api/inbound_limits.go - fonoteka.go/plugins/golem15/fonoteka/controllers/api/recognize_controller.go - fonoteka.go/plugins/golem15/fonoteka/controllers/api/release_match_controller.go - fonoteka.go/plugins/golem15/fonoteka/controllers/api/wishlist_release_match_controller.go - fonoteka.go/plugins/golem15/fonoteka/csv_import_job.go - fonoteka.go/plugins/golem15/fonoteka/csv_match_job.go - fonoteka.go/plugins/golem15/fonoteka/discogs_wiring.go - fonoteka.go/plugins/golem15/fonoteka/golem_wiring.go - fonoteka.go/plugins/golem15/fonoteka/jobs.go - fonoteka.go/plugins/golem15/fonoteka/mail.go - fonoteka.go/plugins/golem15/fonoteka/plugin.go - fonoteka.go/plugins/golem15/fonoteka/routes.go - fonoteka.go/plugins/golem15/fonoteka/updates/22_discogs_rate_windows_table.go - fonoteka.go/plugins/golem15/fonoteka/wishlist_digest_job.go - summercms.go/internal/build/build.go - summercms.go/modules/beachcomber/searchable.go - summercms.go/modules/beachcomber/typesense/engine.go - summercms.go/modules/conga/job.go - summercms.go/modules/fetchguard/client.go - summercms.go/modules/fetchguard/fetch.go - summercms.go/modules/fetchguard/ip.go - summercms.go/modules/fetchguard/policy.go - summercms.go/modules/sunscreen/sunscreen.go - summercms.go/modules/surf/cors.go - summercms.go/modules/tide/normalize.go - summercms.go/modules/tide/upstream.go - summercms.go/modules/tide/upstream_proxy.go - fonoteka.go/plugins/golem15/golem/admin.go - fonoteka.go/plugins/golem15/golem/plugin.go - fonoteka.go/plugins/golem15/feedback/assets.go - fonoteka.go/plugins/golem15/feedback/admin.go - fonoteka.go/plugins/golem15/feedback/plugin.go - fonoteka.go/plugins/golem15/feedback/jobs.go - fonoteka.go/plugins/golem15/feedback/routes.go - fonoteka.go/plugins/golem15/golem/updates/01_ai_models.go - fonoteka.go/plugins/golem15/golem/updates/registry.go - fonoteka.go/plugins/golem15/feedback/updates/01_feedback.go - fonoteka.go/plugins/golem15/feedback/updates/registry.go - fonoteka.go/plugins/golem15/golem/controllers/models_admin_controller.go - fonoteka.go/plugins/golem15/golem/controllers/admin_registry.go - fonoteka.go/plugins/golem15/golem/console/import_settings.go - fonoteka.go/plugins/golem15/golem/models/registry.go - fonoteka.go/plugins/golem15/golem/models/ai_model.go - fonoteka.go/plugins/golem15/feedback/controllers/submissions_admin_controller.go - fonoteka.go/plugins/golem15/feedback/controllers/admin_registry.go - fonoteka.go/plugins/golem15/feedback/classes/submission_store.go - fonoteka.go/plugins/golem15/feedback/classes/image_guard.go - fonoteka.go/plugins/golem15/feedback/classes/sync_g15office.go - fonoteka.go/plugins/golem15/feedback/classes/str_limit.go - fonoteka.go/plugins/golem15/feedback/classes/g15office_client.go - fonoteka.go/plugins/golem15/feedback/console/import_settings.go - fonoteka.go/plugins/golem15/feedback/models/user_preference.go - fonoteka.go/plugins/golem15/feedback/models/submission.go - fonoteka.go/plugins/golem15/feedback/models/registry.go - fonoteka.go/plugins/golem15/feedback/models/settings.go - fonoteka.go/plugins/golem15/feedback/controllers/api/request.go - fonoteka.go/plugins/golem15/feedback/controllers/api/feedback_api_controller.go - fonoteka.go/plugins/golem15/feedback/controllers/api/winter_errors.go - fonoteka.go/plugins/golem15/feedback/controllers/api/me_hidden_controller.go - fonoteka.go/plugins/golem15/golem/classes/security/ssrf_guard.go - fonoteka.go/plugins/golem15/golem/classes/providers/adapter.go - fonoteka.go/plugins/golem15/golem/classes/providers/anthropic.go - fonoteka.go/plugins/golem15/golem/classes/providers/openai.go - fonoteka.go/plugins/golem15/golem/classes/valueobjects/response.go - fonoteka.go/plugins/golem15/golem/classes/valueobjects/prompt.go - fonoteka.go/plugins/golem15/golem/classes/factories/prompt_factory.go - fonoteka.go/plugins/golem15/golem/classes/services/model_config.go - fonoteka.go/plugins/golem15/golem/classes/services/ai_service.go - fonoteka.go/plugins/golem15/golem/classes/services/settings.go - fonoteka.go/plugins/golem15/feedback/assets/js/embed.js findings: critical: 1 warning: 6 info: 8 total: 15 status: issues_found --- # Phase 14: Code Review Report **Reviewed:** 2026-10-04T01:03:11Z **Depth:** standard **Files Reviewed:** 99 (paths relative to the meta repo `/media/nvme/dev/golem15/summercms.io/summercms/`) **Status:** issues_found ## Summary I reviewed the Phase 14 source across four repositories: summercms.go (fetchguard client, sunscreen, tide upstream recording and replay, surf CORS, beachcomber, conga), fonoteka.go (the Discogs package, the CSV and digest workers, the AI and Discogs routes, the console commands), sm-golem-plugin and sm-feedback-plugin. I read the SUMMARY deviations, CONTEXT, SECURITY-REVIEW and deferred-items. Where the PHP reference was relevant, I compared against `/media/nvme/dev/golem15/fonoteka`. The guarded outbound client, the SSRF guard split, the per-token Postgres limiter, the scoped album lookups and the credential masking in sidecars all hold up. No credential is logged or answered on any path I traced. The main problems are concurrency and write-shape issues the threat register does not cover: - Album writes that follow a slow Discogs call save every column from a stale struct. - The CSV match worker still lets a stale pass rewrite a remapped import, despite the D-10 claim. - The anonymous feedback upload keeps an extension the attacker chooses in public storage. - Several smaller non-idempotency and data-import gaps. Findings marked "parity" reproduce PHP behaviour. They are listed only where the behaviour is a security hole or contradicts a guarantee the phase claims. ## Narrative Findings (AI reviewer) ## Critical Issues ### CR-01: Album writes after slow Discogs I/O save every column from a stale struct, silently reverting concurrent edits **File:** `fonoteka.go/plugins/golem15/fonoteka/classes/discogs/applicator.go:105`, `fonoteka.go/plugins/golem15/fonoteka/classes/discogs/cover_fetcher.go:345` and `:378`, through `fonoteka.go/plugins/golem15/fonoteka/classes/album_write_service.go:269` **Issue:** `SaveAlbumRow` → `saveAlbumRow` ends in `db.Omit(...).Save(a)`, and GORM `Save` writes every column of the struct. The album passed in is loaded by `findAlbum` at the start of the request (`release_match_controller.go:215/444`, `album_cover_fetch_controller.go:58`). It is saved only after the Discogs work: - the limiter `Acquire` and 429 sleeps can take up to the 15 s wait budget; - `GetRelease` and the price suggestions; - in `attachCover`, a cover download of up to 10 s. Any edit committed to the same album in that window is overwritten with the old values: name, notes, shelf, condition, quantity, format and the other columns. Sources of such edits include `PUT albums/{id}` from the Nuxt app, another household member, or the MCP client, which calls `cover-price/discogs` while the user keeps editing. PHP's `$album->save()` writes only dirty attributes, so this lost update is new in the Go port. It is not covered by the "no transaction around apply" deviation (14-03 #7). That deviation is about atomicity, not about clobbering columns the apply never touched. **Fix:** Write only the columns this operation changed, or re-read and lock the row right before the write: ```go // applicator: collect the fields actually set, then cols := append(changed, "updated_at") if err := db.Model(album).Select(cols).Updates(album).Error; err != nil { ... } // cover_fetcher.persistMarketPrice err := db.Model(c.album).Select("market_price_stored", "market_price_currency", "market_price_source", "updated_at").Updates(c.album).Error // cover_fetcher.attachCover err := db.Model(c.album).Select("cover_import_failures", "updated_at").Updates(c.album).Error ``` The model rules (`checkAlbumModelRules`) and the broadcast hooks still have to run, so add a `SaveAlbumColumns(ctx, db, a, cols...)` helper next to `saveAlbumRow`. Do not drop the hooks. ## Warnings ### WR-01: A stale CSV match pass can cancel, advance or fork a remapped import (the race D-10 claims to close) **File:** `fonoteka.go/plugins/golem15/fonoteka/csv_match_job.go:127-128`, `:145-146`, `:196-197`, `:232-252` **Issue:** The function comment says "a pass never moves a cancelled, remapped or committed import", but only the start update checks `match_job_id`. In this sequence, an old pass is running and the user saves a new mapping: `UpdateCsvMapping` cancels the old job, replaces the rows, sets `uploaded` and queues job B. Then: 1. On its next `CheckIfCanceled`, the old pass runs `UPDATE … SET status = canceled WHERE id = ?` with no condition (line 127). The remapped import becomes `canceled`. Job B then sees a terminal status and skips, so the user's remap destroys the import. This happens whenever River has not cancelled the old job's context yet, and every time in poll-only mode. 2. If B has already moved the import to `matching`, the old pass's final `UPDATE … SET status = preview WHERE id = ? AND status = matching` (line 145) or `fail()` (line 196) moves B's import to `preview` or `failed` while B's rows are still pending. Commit can then import unmatched rows. 3. `pause()` checks only `cur.Status == matching` under the lock (line 232). An old pass can therefore queue a third job and overwrite `match_job_id`, leaving two passes matching the same rows. PHP has the same unconditional cancel write (`AlbumCsvMatchJob.php:100-103`). This is still a defect because D-10 and the T-14-10 row present the class as fixed. **Fix:** Scope every status write in the pass to the job that owns the import: ```sql ... WHERE id = ? AND status = ? AND match_job_id = ? -- final, fail UPDATE golem15_fonoteka_csv_imports SET status = 'canceled', updated_at = ? WHERE id = ? AND match_job_id = ? -- cancel branch ``` In `pause()`, also require `cur.MatchJobID != nil && *cur.MatchJobID == jobID` before dispatching. Otherwise call `lostImport`. ### WR-02: The anonymous feedback upload keeps an attacker-chosen file extension in public storage **File:** `fonoteka.go/plugins/golem15/feedback/classes/submission_store.go:85-87` **Issue:** `storeScreenshot` builds the disk name from `filepath.Ext(shot.Filename)`, which the client controls, and writes it public. The `image`/`mimes` rules (`lagoon.validateMimes`) and `IsAllowedImage` check only the content: magic bytes plus `image.DecodeConfig` of the header. A valid PNG or GIF named `x.html`, `x.svg` or `x.xhtml`, with HTML or script in a text chunk after IHDR, therefore passes. It is stored as `storage/app/uploads/public/
/
/
/