Skip to content

DX-3059: add aws s3 style Blob object commands - #24

Open
ytkimirti wants to merge 11 commits into
DX-3059-blob-uploadfrom
DX-3059-blob-X-s3
Open

ytkimirti wants to merge 11 commits into
DX-3059-blob-uploadfrom
DX-3059-blob-X-s3

Conversation

@ytkimirti

@ytkimirti ytkimirti commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Adds upstash blob ls|cp|mv|rm|sync|presign|mb|rb with blob://<bucket>/<key> URIs, mirroring aws s3 flags and semantics, plus short flags (-r, -d, -n, -q).
Replaces blob upload with cp -r/sync, merges list into ls, and lets get/delete/credentials take a bucket name or id (--bucket-id still works).
A Blob token is only used for the bucket it was issued for; names resolve through the account.

@linear-code

linear-code Bot commented Sep 24, 2026

Copy link
Copy Markdown

DX-3059

@ytkimirti
ytkimirti marked this pull request as ready for review September 24, 2026 09:49
@ytkimirti
ytkimirti added this pull request to stack #26 September 28, 2026 05:51

@CahidArda CahidArda left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review. I couldn't run the tests locally, so this comes from reading the code.

Stack note: #22 → #24 → #25. This PR deletes #22's upload.ts and its tests, so of #22 only --token, the deps, the engines bump and the release-tag change survive. I'd merge the three as one unit, or squash #22 into this one.

Should fix

  • mv can delete files outside the source directory. walk() now follows symlinks (#22 deliberately didn't), and after upload perform() calls unlink(source.path). mv ./dir blob://b/ -r where dir/link → ~/important deletes the real files in ~/important. aws does the same but has --no-follow-symlinks. Please add that flag, or at least don't follow symlinks for mv sources.
  • stdin upload without --expected-size buffers up to 5 GB in RAM (maxSize: "5gb"). That can take down a machine or CI runner. Lower the cap a lot, or spill the stream to a temp file.
  • Same-bucket server-side copy has no retries. from.copy() sits outside withRetries, while streaming and every other operation retry. Transient 5xx and 429 errors fail immediately.

Worth a look

  • listDirectory is a hand-rolled SigV4 signer plus a regex XML parser. It doesn't decode numeric entities (&#13;) and doesn't request encoding-type=url. Delimiter listing belongs in @upstash/blob. I'd upstream it and drop about 90 lines here.
  • tokenBucketId decodes the token by fixed byte offsets (raw[2] = length, id at offset 6). That couples the CLI to an undocumented format. It should come from the SDK.
  • findAccountBucket picks the first bucket whose name matches. If names aren't unique per account, this silently targets the wrong bucket, and rb -f then empties it.
  • ls with no argument prints the raw bucket list. If that includes token/token_next, the most casual command dumps every bucket token. get already has --hide-credentials; I'd hide tokens by default here.
  • Same-bucket copy with --metadata but no --content-type: check whether the SDK's copy uses REPLACE semantics and resets the content type.
  • rb detects "not empty" with a regex on the error message (/not empty/), which is fragile. Also, delete and rb now do the same job.
  • The README lost the note on leftover multipart uploads. A killed process still leaves them behind, and only rb -f cleans them up.
  • Nits: the filterOrder counter is module-global and leaks across tests. --metadata " =x" is accepted with an empty key.

Good parts

Temp-file-and-rename downloads, the mtime alignment for sync, the local path-escape guard, nested-prefix detection keyed on the real S3 bucket, and case/symlink handling in sync -d are careful work. Keeping list as an alias and a hidden --bucket-id preserves backward compatibility.

Before merging: this PR shows 2/3 checks while #25 (same head) is 3/3, so rerun the Tests job. Also, v1.4.0-canary.4 already ships these commands, including the symlink-following mv.

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.

2 participants