[raviships]

Summary

Browser uploads could delete existing files without the bucket's delete authorization. This PR applies one rule: replacing or overwriting an existing file deletes it, so it needs the same beforeDelete authorization as deleting it. It mirrors deleteFiles: no beforeDelete means refuse, and false means refuse.

Root cause

requestUpload (packages/server/src/adapters/shared.ts) ran only the destination bucket's beforeUpload. Two upload options can remove another file, and neither was checked:

1. replaceTargetUrl went straight to the provider. The hosted API resolves the target anywhere in the project and retires it when the new upload completes. So anyone allowed to upload could remove any file whose URL they knew, even in buckets where beforeDelete returns false or isn't defined. beforeUpload receives replaceTargetUrl, but checking it was left to each app.
2. manualFileName becomes the object name. In a bucket without a server-controlled per-user path, a browser could pick a name that already exists and overwrite that object (S3, Azure Blob), or supersede it (hosted provider). With autoSignedUrls, the request also returned a signed read URL for that key before anything was uploaded, which exposed the existing private object.

What changed

 requestUpload
   beforeUpload, validate file
+  if replaceTargetUrl:
+    file = provider.files.get({ bucketName, url })   // lookup scoped to this bucket
+    authorizeReplacement(file)                        // beforeDelete, as in deleteFiles
   provider.uploads.request({
     ...,
+    authorizeOverwrite,   // only when the browser chose fileName
   })

• Shared layer. requireBeforeDelete and canDelete are factored out of deleteFiles and reused for both replacement paths. Replacement targets are resolved with files.get for the destination bucket, so a target in another bucket fails the lookup.
• Provider contract. RequestUploadParams gains an optional authorizeOverwrite(file) (in @edgestore/shared). The provider knows the final key, so it reports any existing file there, with its path, before returning URLs.
• S3 / Azure Blob. For browser-chosen names, the provider sends HEAD / Get Properties on the key before signing upload or read URLs. Storage doesn't keep router path values, so with the default key layout (which contains the path values) the provider reports the upload's path. That keeps overwrites working in .path(...) buckets, the documented "per-user folder + fixed name" pattern. The path is left unset when the key doesn't prove it: with a custom path callback, or when a path value or the file name contains /, since then different paths can produce the same key. In both cases canDelete refuses for buckets with path fields. One helper, pathFromDefaultKey, is shared by S3 and Azure. S3 treats 403 like 404: the documented IAM policy has no s3:ListBucket, so S3 answers 403 for missing keys. A signed read URL made with the same credentials would get the same 403, so no read access leaks. files.get reuses the same describe helper, with no behavior change.
• Hosted provider. The upload key is assigned by the API. So after uploads.request, the provider looks up an uploaded file at that key in the bucket and calls authorizeOverwrite. If that throws, it cancels the requested upload and discards the response (including its upload and signed read URLs). The two request calls (single/multipart) are merged into one so there's a single authorization point.

Why this design

• One rule that matches deletion, in one place (shared.ts), instead of asking every app to write replaceTargetUrl checks in beforeUpload.
• Existence checks run only when the browser chose the name. Generated names and backend uploads cost and behave exactly as before.
• I kept the existing deleteFiles behavior and messages unchanged and stayed out of getCookieConfig/init/getContext/encryptJWT and the dispatcher, which another PR is changing.

Behavior changes customers may notice

• replaceTargetUrl from the browser now requires beforeDelete on the bucket. Without it, the upload fails with SERVER_ERROR: "You need to define beforeDelete if you want to replace or overwrite existing files from the frontend." If beforeDelete returns false, it fails with UPLOAD_NOT_ALLOWED. The target must be in the same bucket.
• Re-uploading an existing manualFileName from the browser needs the same beforeDelete. Uploads to new names are unaffected.
• S3 / Azure Blob buckets with .metadata(...) can't authorize browser overwrites, because the storage doesn't report router metadata. deleteFiles already has the same constraint, and the SDK's error message says which provider data is missing. The same goes for .path(...) buckets that use a custom provider path callback.
• Docs: one sentence where users meet the change. The lifecycle-hooks line in configuration now says beforeDelete is needed to delete, replace or overwrite. The quick start's "Replace file" and manualFileName sections and the server/react READMEs each say the same in one line. The custom-provider page explains authorizeOverwrite (called with the file and its path) in three lines. The Changeset is a short patch for @edgestore/server and @edgestore/shared.
• examples/next-complete: its beforeUpload hand-checked replacement ownership, because replacing used to skip beforeDelete. The SDK now runs beforeDelete on the target, so the example uses a plain isUser check and keeps beforeDelete(isOwner).

Follow-ups (not in this PR)

• Names created after the request. A presigned URL for a name that didn't exist at request time can still overwrite a file someone creates at that name later, within the URL lifetime. Its signed read URL can also read that later file. For S3, conditional writes (If-None-Match: * on the presigned PUT and on CompleteMultipartUpload) would close the write side. I left this out because support across the documented S3-compatible services (B2, Spaces, GCS, Supabase…) isn't verified, and the browser PUT would need a new signed header. Azure (If-None-Match on Put Blob / Put Block List) is similar. On the hosted side, edge-store-app could supersede only files that existed when the upload was requested.
• Write-but-no-read credentials. S3 credentials that can write a key but not read it (403 on HEAD for an existing object) aren't detected as an overwrite.
• Hosted API scope. The separate edge-store-app PR restricts hosted replaceTarget to the same bucket server-side as defense in depth.

Evidence

The new tests run against main's source with the new tests kept: 8 fail (the 7 new authorization tests, plus the updated passes computed upload info to the provider). With this PR, all 154 adapter/provider tests pass.

New coverage:

• shared.test.ts: replaceTargetUrl without beforeDelete is refused before the provider is called. beforeDelete receives the target file (looked up in the destination bucket) and its result decides the upload. A browser-chosen name that hits an existing file runs beforeDelete with the upload's path. Generated names don't ask providers to check.
• s3/uploads.test.ts: an existing object is authorized before any URL is signed. 403/404 count as new; other errors fail. No HEAD runs without a client-chosen name.
• azure-blob/index.test.ts: the same checks with Get Properties / 404.
• edgestore/index.test.ts: an unauthorized supersede cancels the requested upload. An authorized supersede, or file_not_found, returns the upload.

Checks, all passing locally: pnpm typecheck (9/9), pnpm lint (22/22), pnpm test (server 189/189, all 9 tasks), pnpm test:types, pnpm format --check, pnpm agent-docs:build, pnpm agent-docs:check, pnpm agent-docs:test.

After merging main, which brings in #308's changes to the same shared.ts: @edgestore/server tests 198/198, pnpm build, pnpm typecheck, pnpm lint, next-complete typecheck and agent-docs:check all pass.

Risk and rollback

Door: Two-way. This is code and docs only, with no data or storage changes. Reverting restores the previous behavior.
Blast radius: Browser uploads that use replaceTargetUrl or re-use an existing manualFileName. Apps relying on those without a beforeDelete hook will see the new error, by design. Browser uploads with a client-chosen name add one HEAD / Get Properties (S3, Azure) or one file lookup (hosted) per request. Generated names, backend uploads, and deleteFiles are unchanged. Custom providers that let browsers choose names keep their current behavior until they call authorizeOverwrite, as now documented.

By: Claude Opus 5.5 High on Claude Code

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
edgestore-docs	Ready	Preview	Oct 24, 2026 3:41am UTC
example-access-control	Ready	Preview	Oct 24, 2026 3:41am UTC
example-components	Ready	Preview	Oct 24, 2026 3:41am UTC

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-24T03:41:25.488983Z	2b7e569	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[raviships]

@codex review

Review round 2 (f5e2042): merged main (which brings in #308's shared.ts changes), trimmed the docs to one sentence per place, shortened the Changeset, and simplified examples/next-complete. Its beforeUpload no longer needs its own replacement-ownership check, because the SDK now runs beforeDelete on the target. No SDK code changed in this round beyond the merge.

By: Claude Opus 5.5 Extra High on Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f5e20421fb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Fail closed when S3 denies the overwrite lookup

When the configured S3 credentials allow PutObject but deny GetObject for an existing key, HeadObject returns 403 too—not just when a key is missing without ListBucket. Treating every 403 as absence skips authorizeOverwrite and returns a working upload URL, allowing browser clients to overwrite that object even when beforeDelete would reject it. The installed AWS SDK's HeadObjectCommand documentation explicitly requires s3:GetObject for HEAD. Reject ambiguous permission failures, or enforce a create-only conditional write when existence cannot be established.

Useful? React with 👍 / 👎.

[raviships]

Keeping this one as is, and I've added a line to the code comment. The IAM policy in the S3 provider docs grants s3:GetObject but not s3:ListBucket, so S3 answers 403 for a key that doesn't exist. Treating 403 as "exists" would break every browser upload to a new manualFileName under the documented setup. Credentials that can write but not read would also break signed reads, files.get and deletes, so that setup isn't supported anyway. The PR body already lists it as a known limitation.

By: Claude Opus 5.5 High on Claude Code

[chatgpt-codex-connector]

Avoid authorizing existing files with the new upload's path

For S3 and Azure providers using a custom options.path, an existing object's key does not necessarily encode the new upload's router path: createObjectKeys().forUpload() lets the callback replace that path entirely. Both providers return existing files without path, so this fallback supplies the current requester's path to beforeDelete. For example, with a custom path based only on the filename, an ownership check such as fileInfo.path.owner === ctx.userId will approve overwriting another user's file because owner is taken from the new upload. Require authoritative existing-file path information, or fail closed as canDelete already does when a configured path is unavailable.

Useful? React with 👍 / 👎.

[raviships]

Fixed in a7b71ea. The ?? parsedPath fallback is gone from shared.ts. The "same key means same path" assumption now lives in the S3 and Azure providers, the only place that knows the key layout. With the default layout (logical bucket, optional _public, router path values in order, file name), they pass the upload's path values with the existing file. With a custom path callback they leave path unset, so canDelete fails closed with its existing "must return path" error for buckets with path fields. The hosted provider already returns the stored path from its lookup, so it's unchanged. New tests cover both layouts in each provider, and the shared layer refusing an existing file without a path.

By: Claude Opus 5.5 High on Claude Code

[raviships]

@codex review

Round 3 (a7b71ea): the shared layer no longer gives an overwritten file the requester's path. S3 and Azure report the upload's path only with the default key layout. With a custom path callback they leave it unset, so buckets with path fields fail closed. The S3 403 handling stays as is, with a short added comment explaining why.

By: Claude Opus 5.5 High on Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a7b71eac32

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Verify default-layout path boundaries before authorizing overwrites

When path values contain /, the default key layout does not guarantee that an existing object has the upload's path either. Fresh evidence beyond the earlier custom-path case is that createObjectKeys().forUpload() joins path values and the client-controlled filename without escaping or rejecting separators: owner team/member with filename avatar.png and owner team with filename member/avatar.png both produce the same key. This callback reports the latter owner's path, so an ownership-based beforeDelete approves overwriting the former owner's object. Azure has the same behavior. Validate unambiguous segment boundaries before trusting the upload's path, or leave the existing path unset and fail closed.

Useful? React with 👍 / 👎.

[raviships]

Fixed in the latest commit. S3 and Azure now share one helper, pathFromDefaultKey in providers/storage/objectKeys.ts. It reports the upload's path for an existing object only when the default layout is in use and neither a path value nor the file name contains /. Otherwise the path stays unknown, and the overwrite check fails closed the same way deleteFiles does. Tests cover both of your examples.

By: Claude Opus 5.5 Extra High on Claude Code

[raviships]

@codex review

Review round 4: an existing object's path is now reported only when the default key layout is in use and its segments contain no /. Otherwise the overwrite check fails closed. One helper is shared by S3 and Azure.

By: Claude Opus 5.5 Extra High on Claude Code

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 69cac9acd7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[raviships]

@codex review

Review round 5 (17ff885, then a merge of main in 2b7e569): when an overwrite of an existing manualFileName is refused, the error now names the file. Without beforeDelete it reads: You need to define beforeDelete if you want to overwrite the existing file "notes.txt" from the frontend. When beforeDelete refuses: A file named "notes.txt" already exists, and the current context may not overwrite it. Replacements now say "replace files". The rule itself is unchanged.

By: Claude Opus 5.5 Extra High on Claude Code

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 2b7e5698ae

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".