Skip to content

S3 image: Apply cloudflare worker to access S3 buckets - #202

Merged
chai-inu merged 4 commits into
mainfrom
upload/read-path-proxy
Aug 16, 2026
Merged

chai-inu merged 4 commits into
mainfrom
upload/read-path-proxy

Conversation

@chai-inu

Copy link
Copy Markdown
Member

No description provided.

chai-inu added a commit that referenced this pull request Aug 16, 2026
#202 defends Storm egress well -- content-addressed objects, immutable
caching, and caches.default inside the Worker put Storm at about one fetch
per object per PoP. It defends Worker invocations not at all: the Worker is
the origin on a Custom Domain, so it runs on every request, hit or miss.
Measured on the live hostname, a never-seen ?v=<random> URL still returned
cf-cache-status HIT with the same age as the plain URL, which can only happen
if the Worker executed and did its own normalized lookup.

That matters because the free-tier ceiling fails as a cliff: 100k requests a
day, then error 1027 for the rest of the UTC day, every image broken at once
and recovering on a clock rather than on an action. Chafan 1.0 was taken down
by scraper load, so this is a documented history rather than a hypothetical.

The plan records the measures in the order worth doing them -- rate limiting,
the bot toggles, a referer rule, Workers Paid, then robots.txt and negative
caching in the Worker -- with what each does not cover, and why blocking at
the edge is cheap: WAF and rate limiting run before Workers, so a blocked
request never invokes one.

It also records the fork not taken. Static Hosting behind a proxied CNAME
would make cache hits free, but publishes a stormsites.ca URL that cannot be
turned off while serving, trading a raisable invocation ceiling for an
uncapped egress bypass.

Nothing here is implemented. It is written down now because the trigger is
external and sudden, and because none of the configuration lives in this
repo, so no branch or review would ever reveal that a rule is missing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017LrW5uHuh4J4SbqExTjYE4
chai-inu and others added 4 commits August 16, 2026 13:20
Garage serves no anonymous S3 request and implements no bucket policy, so
the read half of image upload had no working design: pointing
UPLOADS_PUBLIC_URL_BASE at the S3 endpoint made every upload succeed and
every <img> 403.

Storm's own answer is Static Hosting, which is rejected here. It is a
per-bucket switch that exposes everything at a stormsites.ca URL, and that
URL cannot be turned off while serving -- so fronting it with Cloudflare
would leave a permanent way to bypass the cache and the rate limits and
bill the egress to us. Routing bytes through the server instead is worse:
the tunnel is the scarcest link we have.

So the bucket stays private and this Worker is the only reader. It holds a
Read Only per-bucket key, signs each GET with SigV4, and answers repeats
from the edge cache, which puts Storm egress at roughly one fetch per
object per PoP. Rate limiting becomes an ordinary zone rule.

KEY_PATTERN is the security boundary rather than a nicety: without it the
Worker signs anything, including list operations. It duplicates the key
format from object_storage.py, which the docstring there now flags.

workers_dev = false is also load-bearing. caches.default is a silent no-op
on *.workers.dev, and that URL would serve the same images outside the
zone, so both the caching and the rate limiting would be off with no
visible symptom.

The README's secret steps say what `wrangler secret put` does -- a hidden
prompt, no shell history, no file, and no lookup of a same-named shell
variable, which matters precisely because launch_env exports
UPLOADS_S3_ACCESS_KEY_ID as the Read / Write key, so piping the environment
in would hand the public read path write access. Deploy comes before
`secret put`, since `secret put` against a Worker that does not exist yet
creates a bare one without these vars or the route. A further section
answers the question the custom domain invites: being on Cloudflare is not
what caches this. The Worker is the origin and runs on every request; the
caching is the caches.default calls and nothing else, so deleting them would
be silent. Each of the three consequences was observed against the live
hostname: the cache is per-PoP, invocations bill on hits, and 404s are not
cached.

Verified against the live dev bucket: the signature is accepted (200,
38125 bytes, matching head_object), and under `wrangler dev` a valid key
returns the image with immutable cache headers, a second request reports
CF-Cache-Status: HIT, and the bucket-probe paths all 404. tsc --noEmit is
clean against workers-types 5, and `wrangler deploy --env dev --dry-run`
uploads 4.15 KiB with the three vars bound and no warnings.

Not deployed to prod: that still needs its own Read Only key and
img.cha.fan, though [env.prod.vars] now names the real bucket rather than a
placeholder that would deploy against a nonexistent one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017LrW5uHuh4J4SbqExTjYE4
Nothing in the server ever exercises the read path -- UPLOADS_PUBLIC_URL_BASE
is only string-concatenated into a response -- so a base pointed at the wrong
host fails silently and only a browser finds out. check_upload_public_read.py
walks it the way a reader does: list the bucket over S3, fetch each
<base>/<key> unsigned, compare status, content type and length against a
signed head_object. It refuses the specific mistake that prompted it, a
public base equal to the S3 endpoint.

The README section now describes the private-bucket Worker rather than
implying anonymous reads are a matter of bucket permissions, and records why
Static Hosting is not used. object_storage.py's docstring gains the pointer
tying _key() to the Worker's KEY_PATTERN, since the two must not drift.

Also a TODO, unfixed on purpose: stored URLs are absolute and get baked into
answer bodies at post time, so changing the base only fixes uploads made
after it. Keeping the hostname stable is what makes what sits behind it
swappable without a body rewrite.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017LrW5uHuh4J4SbqExTjYE4
This branch defends Storm egress well -- content-addressed objects,
immutable caching, and caches.default inside the Worker put Storm at about
one fetch per object per PoP. It defends Worker invocations not at all: the
Worker is the origin on a Custom Domain, so it runs on every request, hit or
miss. Measured on the live hostname, a never-seen ?v=<random> URL still
returned cf-cache-status HIT with the same age as the plain URL, which can
only happen if the Worker executed and did its own normalized lookup.

That matters because the free-tier ceiling fails as a cliff: 100k requests a
day, then error 1027 for the rest of the UTC day, every image broken at once
and recovering on a clock rather than on an action. Chafan 1.0 was taken down
by scraper load, so this is a documented history rather than a hypothetical.

The plan records the measures in the order worth doing them -- rate limiting,
the bot toggles, a referer rule, Workers Paid, then robots.txt and negative
caching in the Worker -- with what each does not cover, and why blocking at
the edge is cheap: WAF and rate limiting run before Workers, so a blocked
request never invokes one.

The two Worker-side items carry their code and, more usefully, the reasoning
that is not recoverable from the code. robots.txt has to be served by the
image host itself, because a crawler that finds an image URL checks
robots.txt there and not on dev.cha.fan; it goes ahead of the key check so
nothing is signed for it; and Disallow: / is right here specifically, since
nothing on this host is a page. For negative caching, only 404 is worth
storing -- a 502 means Storm was unreachable or the key rotated, and caching
that extends an outage past its own end. The obvious objection, that a
negative entry could shadow an object that shows up moments later, is
recorded as unreachable rather than unconsidered: the URL is derived from the
sha and only handed to the client after put_object returns, so nothing knows
the URL before the bytes exist.

It also records the fork not taken. Static Hosting behind a proxied CNAME
would make cache hits free, but publishes a stormsites.ca URL that cannot be
turned off while serving, trading a raisable invocation ceiling for an
uncapped egress bypass.

Nothing here is implemented. It is written down now because the trigger is
external and sudden, and because none of the configuration lives in this
repo, so no branch or review would ever reveal that a rule is missing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017LrW5uHuh4J4SbqExTjYE4
print_app_settings iterated settings.__dict__ and logged every value as-is, so
SECRET_KEY, the SMTP password, UPLOADS_S3_SECRET_ACCESS_KEY and the DATABASE_URL
password all landed in FASTAPI_LOG_PATH in plaintext.

The rule is name-based rather than a list of the secrets we happen to have
today, so a credential added later is masked without anyone remembering to come
back here: a str/SecretStr setting named like a secret prints as
***redacted***, and any value carrying URL userinfo has the part before the `@`
masked -- which is what covers DATABASE_URL, REDIS_URL and the Sentry DSN key,
none of which are caught by the name rule. The type check keeps the durations
that are merely named like secrets (ACCESS_TOKEN_EXPIRE_MINUTES,
EMAIL_SIGNUP_CODE_EXPIRE_HOURS) printing their value; an int is never a
credential. Unset settings still print None, which discloses nothing and
answers most configuration questions.

This does not clean the log files that already hold those values in plaintext.
The Storm R/W key still wants rotating.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@chai-inu
chai-inu force-pushed the upload/read-path-proxy branch from 9ff69bd to bda4c22 Compare August 16, 2026 17:21
@chai-inu
chai-inu merged commit a97754c into main Aug 16, 2026
11 checks passed
@chai-inu
chai-inu deleted the upload/read-path-proxy branch August 16, 2026 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant