Repository navigation
feat(storage): optional S3 object storage and safe image ingestion - #114
Merged
Merged
Conversation
jayleaton
force-pushed
the
feat/storage-ingestion
branch
2 times, most recently
from
October 23, 2026 09:40
0828bd2 to
cdf6fa0
Compare
jayleaton
marked this pull request as ready for review
October 23, 2026 09:41
jayleaton
force-pushed
the
feat/storage-ingestion
branch
3 times, most recently
from
October 23, 2026 10:35
c29526e to
c5d06db
Compare
This was referenced Oct 23, 2026
Adds packages/storage (S3 client over net/http + SigV4, no SDK), packages/storage/safefetch (user-URL fetch that checks every dialled IP, redirect hop included, so DNS rebinding fails) and packages/storage/imaging (pure-Go JPEG/PNG/GIF/WebP/AVIF decode, EXIF orientation, fit to 2048 px, re-encode to WebP). Off unless STORAGE_S3_ENDPOINT/BUCKET/ACCESS_KEY_ID/SECRET_ACCESS_KEY are set (live secrets, all or none, probed at boot). One example behind the layers: ImageService with ObjectStore/ImageFetcher/ImageCodec ports and /api/core/v1/images (upload, import by URL, get, delete), mounted only when storage is on. The SSRF address and URL rules live in services (PublicAddr, FetchURLAllowed) and are table-tested there. New dependencies: gen2brain/webp and gen2brain/avif (libwebp transpiled to Go, libavif under wazero; the only pure-Go WebP encoder and AVIF decoder) and golang.org/x/image (resizing). Builds need -tags nodynamic to stay static: make build and the Dockerfile pass it, and the image build fails without it. Source: jayleaton/auto-visuals#9 and #26 (generic parts only). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…98 MB x/image/draw's CatmullRom.Scale keeps the whole horizontally scaled image (target width x source height x 32 bytes): 198 MB for a 12 MP photo, 396 MB for 48 MP, and the server held about 540 MB after a few uploads. resize does the same separable Catmull-Rom (same weights, same 16-bit premultiplied arithmetic, same rounding) one band of output rows at a time, scaling only the source rows a band reads, support rows included, so the output is identical byte for byte and bands leave no seams. Peak RSS on the real server: 12 MP upload 567 -> 179 MB, 48 MP 591 -> 218 MB, two 12 MP at once 528 -> 266 MB; the stored WebP is byte-identical. golang.org/x/image is no longer needed (image/draw from the standard library covers the copy). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jayleaton
force-pushed
the
feat/storage-ingestion
branch
from
October 23, 2026 11:18
c5d06db to
5217d43
Compare
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.
The template had no way to keep files: no bucket client, and nothing safe for taking an image from a user or fetching one from a URL they name. This adds optional S3-compatible object storage (Railway buckets, R2, S3, MinIO) and safe image ingestion. It is off unless configured. Source: jayleaton/auto-visuals#9 (storage, SSRF-safe fetch, pure-Go imaging) and #26 (storage stays optional; partial config is refused). Only the generic parts are ported.
Summary
The routes register through the central access gate (#124): import and delete are
Endpoints (access.SignedIn); the raw upload and the streamed download use two smallroute.govariants,RouteLimit(10 MB body cap) andRouteFile(file instead of the envelope). They share oneadmithelper withRoute, so 401/403/500 behave identically, andAdmitstays inroute.go.STORAGE_S3_ENDPOINT,_BUCKET,_ACCESS_KEY_IDand_SECRET_ACCESS_KEYare live secrets (secrets.Secret, validated as all-or-none invalidateLive, accessorSecrets.Storage(), revealed only incmd/server).STORAGE_S3_REGIONand_PATH_STYLEare plain env. In this PR the server reads them at boot; live rotation is the next PR in the stack.packages/storageout of services and transports, and keeps it from importing apps, database, auth or secrets.make lint-layeringhas four new probes.gen2brain/webpv0.6.4 (libwebp transpiled to Go; decode and encode) andgen2brain/avifv0.6.0 (libavif/dav1d in WASM under wazero). Both are current, released in July 2026.golang.org/x/imageis no longer needed: the resize is our own (below), and the copy uses the standard library'simage/draw. They are the only pure-Go WebP encoder and AVIF decoder; the alternative is libvips over cgo, which rules out the static binary.-tags nodynamicthese codecs link purego, and aCGO_ENABLED=0binary comes out dynamically linked, which crash-loops on distroless/static.make buildand theDockerfilepass the tag, and the Docker build fails (go version -mcheck) if it is dropped. CI's owngo build/go testwork without it.CatmullRom.Scaleholds the whole horizontally scaled image: target width × source height × 32 B = 198 MB for a 12 MP photo, 396 MB for 48 MP. A server held about 540 MB after a few uploads, too much for Railway's 512 MB.imaging.resizedoes the same separable Catmull-Rom (weights from the samenewDistribalgorithm, the same 16-bit premultiplied arithmetic, the sameftourounding) one band of output rows at a time. Each band scales only the source rows it reads, with the kernel-support rows it shares with its neighbours, and scratch is capped at 8 MiB. The output is identical byte for byte, so bands leave no seams. No lasting test was added: by construction there is no seam risk, andimagingsits outside the test boundaries (services see the codec only as a mocked port); the comparison below proves it.maxErrorBody,orientRotate90,imageIDBytes,bandBudget, …). Checked withmnd+varnamelen(min length 2, receivers and params included) on the new files: 0 findings. Tests usetest *testing.T, as main now does. After that pass the outputs were byte-identical (same sizes, same 39.54 dB orientation PSNR, same SSRF refusals).docs/storage.md(turn on, use, safety, delete), one README "Learn more" line, the names indocs/secrets.mdand.env.example, and one line in AGENTS "Where code lives".Evidence
Real binary (
make build) against a throwaway Postgres and a local S3 server (RustFS:minio/miniois no longer pullable from Docker Hub, and quay returned 401). Both were on free ports and removed afterwards.Static binary. Before: a probe build of the codecs without the tag gave
dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2. After:bin/server: ELF 64-bit LSB executable, x86-64, … statically linked, with build info-tags=nodynamic CGO_ENABLED=0. Size (stripped, linux/amd64, decimal MB), measured on drop-one builds of the real server: 24.5 MB vs 16.9 MB on maine80a772, +7.6 MB: AVIF + wazero 4.13 MB, WebP 3.08 MB, the rest ofimaging0.37 MB, S3 client + safefetch + service/API under 0.1 MB. Dropping x/image saved 0.15 MB. README's Performance table now says 24.5 MB, notes in one sentence that 7.6 MB of it is the image codecs, and gives the image as about 11 MB compressed (0.86 MB distroless base from its manifest + the binary, which gzips to 9.9 MB; on main it gzips to 6.0 MB, so the old "about 5 MB" was already stale).Boot. Before: no storage. After:
storage: bucket reachable storage=endpoint:fp:957fb5c0,bucket:fp:ff1bc3c0,key:fp:c6d09e1a,secret:fp:b22e42ac.serve failed … storage: access denied: head 403 Forbidden, exit 1.… access denied: no such bucket.… unavailable: head: dns lookup failed. No values are logged, fingerprints only.invalid config: STORAGE_S3_ACCESS_KEY_ID is required when any STORAGE_S3_* secret is set.storage: off …, soPOST /imagesis a 404 whileGET /users/meis a 200.Uploads (
POST /api/core/v1/images):WebP image, with alpha, 2000x1500Image has too many pixelsImage could not be read/Unsupported image type…/Unsupported image type…The bucket held only
users/<uid>/images/<32 hex>.webp. GET servesContent-Type: image/webpandContent-Security-Policy: default-src 'none'; sandbox. Another user's id,..%2F.., uppercase ids and no session get 404/401. Delete, then GET, is a 404. Browser headers get the bare 404.Banded resize vs today (x/image), same decoded images (throwaway program):
On the real server (fresh process per scenario, after sign-up ≈ 148 MB, which includes argon2): the stored WebP is byte-identical (same SHA-256, 81 766 B).
The comment on
imageConcurrencyanddocs/storage.mdgive this per-upload memory: about 30 MB above idle for 12 MP, 70 MB for 48 MP, 120 MB for two 12 MP at once.Codec idle cost (measured for JJ before this change): about +1.7 MB RSS; startup within noise; browser bytes identical. Nothing runs at boot: AVIF compiles its wasm on the first AVIF image (1.4 s, 6.5 MB, once).
Import / SSRF (
POST /api/core/v1/images/import, live):https://www.gstatic.com/webp/gallery/1.webp→ stored 550×368. A redirect via httpbin to a public image → stored.This URL cannot be fetched):127.0.0.1,localhost,[::1],[::ffff:127.0.0.1],169.254.169.254,[fd00:ec2::254],10.0.0.1,100.100.100.200,2130706433,0x7f.1,127.1, the Postgres and S3 ports,user:pw@, andfile:(400 from the validator).https://localtest.me/(resolves to 127.0.0.1; it passes the URL rule, so only the dial check can refuse it) is refused. A redirect hop tohttps://localtest.me/a.pngorhttps://169.254.169.254/…is refused.7f000001.08080808.rbndr.usfour times: the first answer was 8.8.8.8 (dialled, 10 s dial timeout →could not be fetched); the next three were 127.0.0.1 → refused at dial.httpbin.org/redirect/5→ refused (over 3 hops). A 100 MB file →Image is larger than 10 MBin 1.2 s.httpbin.org/html→Unsupported image type.Tests (boundaries only):
services:TestPublicAddr(56 addresses, including NAT64/6to4/Teredo/IPv4-mapped/CGNAT/metadata),TestFetchURLAllowed(26 URLs), sniffing for every format plus HEIC/SVG/HTML/MP4, limits, port error mapping, the fetch deadline, per-user keys and id tricks. 100.0% coverage.api: the four cases per route (throughapitest.Gate()), the upload cap, the import validator and the sandboxed file response. 97.5% coverage.Checks: rebased on main
24be189(merged beside the new Google/Apple sign-in: both features' routes, config, live secrets and server wiring are kept); before that one80a772(no migrations in this PR, so nothing to renumber);make check-fastok (full run, because go.mod changed) andmake lint-layeringok, including main's newAdmitprobes. After the rebase, the gate-registered routes on the real binary: upload 200, signed out 401, another user 404, get with the sandbox CSP,localtest.merefused, public import 200,file://400, delete then get 404, browser headers 404.CI: green on
c29526e, but only on the second attempt. The first failed the test-time budget inpackages/auth(TestBetterAuthScryptHash10.34 s, budget 10 s; on the re-run 5.97 s). The cause is CI contention, not this code:scripts/ci-check.shruns lint and the race tests in parallel, and the PR build cache comes from main, which has never built gen2brain, so its 2.7 MB transpiledlibwebp.gocompiles cold during the tests (the same test took 3.08 s on another branch). After merge, main's cache includes it. If it recurs, the CI owner could give that test more headroom or compile before testing.Not covered by a lasting test:
packages/storage/**(the S3 client, safefetch, imaging). Tests are not allowed there, so they were proven by the run above.Merge Danger
Door: two-way. No migrations and no stored state outside the bucket; storage is off by default, and a revert removes the routes.
Blast Radius: build. Every server binary gains 7.6 MB and needs
-tags nodynamicto stay static (Makefile and Dockerfile updated, guarded). A deploy pipeline that runs its owngo buildfor distroless images must add the tag. Known follow-up: deleting an account leaves that user's images in the bucket (users/<id>/images/). It is documented in docs/storage.md. The fix is aDeletePrefix(ListObjectsV2 + delete) on theObjectStoreport, called fromUserService.DeleteAccount, with service tests. Deploy follow-up: neitherapps/web(Dockerfile) nordeploy/platform.yamlsetsGOMEMLIMIT, and the web service hasmemory_mb: 512. SettingGOMEMLIMITto about 80% of the service memory lets Go hand freed upload memory back sooner (docs/storage.md says so). That belongs to the deploy owner; this PR does not touchdeploy/.Stack: this is the bottom layer; key rotation follows on top.
🤖 Generated with Claude Code