feat(preprod): Add snapshots subcommand by lcian · Pull Request #3110 · getsentry/sentry-cli

lcian · 2026-01-28T09:49:51Z

Updated version of #3049 to discuss and iterate on things.

Notable changes:

Removed shard_index parameter from the command. I'm not sure what the purpose of that was originally.
This uses the new many (batch) API from objectstore_client. All uploads are executed as batch requests, reducing network overhead. Unfortunately, with they way things are implemented now, we will still have to buffer all files in memory before sending the request, as we need to hash their contents to determine the filename. If we could just use the filename as the key in objectstore, it would be much better because that way we could stream the files over.

Note that auth enforcement still needs to be enabled for objectstore, so that's currently blocking this to be used for anything but internal testing.

Ref FS-233

src/api/mod.rs

lcian · 2026-01-28T10:06:26Z

src/utils/api/mod.rs

+use anyhow::{Context, Result};
+
+/// Given an org and project slugs or IDs, returns the IDs of both.
+pub fn get_org_project_id(api: impl AsRef<Api>, org: &str, project: &str) -> Result<(u64, u64)> {


As far as I know, for all commands a user can provide --project and --org as slugs or IDs.
These utils are needed to get the corresponding IDs, so that objects from the same org/proj all have the same paths in objectstore regardless if the user passed in slugs or IDs.

It might be weird to take in Api and it might be possible that these functions should live somewhere else, IDK.
They certainly don't belong to the Api struct as its methods seem to all map 1to1 to Sentry API calls.

Can the backend interpret the slugs in to ids instead of the frontend?

Can the backend interpret the slugs in to ids instead of the frontend?

Indeed, I agree that this would be preferable. The current code adds two additional API calls that could be avoided if the backend resolves the slugs/IDs appropriately.

Not sure what backend and frontend here refers to.
How objectstore works is that the scope (org and project in this case) is an arbitrary sequence of key-value pairs. We recommend using org and project ID, or at least org ID, but that's not mandatory.
Therefore it's not a responsibility of objectstore or its endpoint in the monolith to normalize org and project, we simply take what the user provides. It's responsibility of the user to normalize.

lcian · 2026-01-28T10:11:13Z

Cargo.toml

 lazy_static = "1.4.0"
 libc = "0.2.139"
 log = { version = "0.4.17", features = ["std"] }
+objectstore-client = { git =  "https://github.com/getsentry/objectstore.git", branch = "lcian/feat/rust-batch-client" }


This indirectly adds a dependency to reqwest and we need to double check what the implications of that are, especially in regard to rustls vs native-tls which could be problematic.

It also adds a bunch of deps but I think most of them are inevitable.

Let's discuss the TLS stuff next week, because I am uncertain what problems you're concerned about.

@lcian and I discussed offline today. We agreed we should use native-tis and native-tls-vendored (the latter possibly only on Linux), as these should pull in the same underlying dependencies as curl, thus limiting the increase in binary size.

linear · 2026-01-28T10:15:48Z

FS-233 Add new command to Sentry CLI to upload to Objectstore

src/commands/build/snapshots.rs

szokeasaurusrex · 2026-02-06T15:04:56Z

Hey @lcian, are you ready for me to review this, or are you still planning to check the feedback from Bugbot and/or iterate further?

rbro112 · 2026-02-10T05:47:04Z

feat(preprod): Add snapshots subcommand #3110 👈 (View in Graphite)
master

This stack of pull requests is managed by Graphite. Learn more about stacking.

Adds the initial snapshots POST API. This api does a few things and is intended to be invoked by the CLI: - Creates `PreprodArtifact`, `PreprodSnapshotMetrics` and `CommitComparison` DB models - Creates image metadata based on what's uploaded from CLI - Stores metadata in objectstore Notably images are uploaded directly to objectstore from CLI Tested E2E locally with objectstore and WIP CLI branch (getsentry/sentry-cli#3110) Resolves EME-773

szokeasaurusrex

I left some comments. Most of them are minor and optional, but I would call your attention to the comment about memory usage and the potential for leaking tokens.

Let's discuss further next week

src/api/mod.rs

src/commands/build/snapshots.rs

szokeasaurusrex · 2026-02-13T13:20:00Z

Cargo.toml

 lazy_static = "1.4.0"
 libc = "0.2.139"
 log = { version = "0.4.17", features = ["std"] }
+objectstore-client = { git =  "https://github.com/getsentry/objectstore.git", branch = "lcian/feat/rust-batch-client" }


Let's discuss the TLS stuff next week, because I am uncertain what problems you're concerned about.

szokeasaurusrex · 2026-02-13T13:20:10Z

Cargo.toml

 lazy_static = "1.4.0"
 libc = "0.2.139"
 log = { version = "0.4.17", features = ["std"] }
+objectstore-client = { git =  "https://github.com/getsentry/objectstore.git", branch = "lcian/feat/rust-batch-client" }


h: Before we merge these changes, we should ensure we are depending on a version that's been published to crates.io, not a Git branch.

@lcian will let you take this one as it's more objectstore related

src/commands/build/snapshots.rs

src/utils/api/mod.rs

src/utils/objectstore/mod.rs

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

cursor · 2026-02-16T09:44:26Z

src/commands/build/snapshots.rs

+
+        // SOF markers: C0-CF except C4 (DHT), C8 (JPG extension), and CC (DAC)
+        if (0xC0..=0xCF).contains(&marker) && marker != 0xC4 && marker != 0xC8 && marker != 0xCC {
+            if i + 7 < data.len() {


Off-by-one in JPEG dimension bounds check

Low Severity

The bounds check i + 7 < data.len() is one byte too strict. The highest index accessed is i + 6, which requires i + 7 <= data.len(). The current check requires data.len() >= i + 8, meaning valid JPEG files where the SOF segment ends exactly at the data boundary will incorrectly return None instead of the parsed dimensions.

cursor · 2026-02-16T09:44:26Z

src/commands/build/snapshots.rs

+        debug!("Processing image: {}", image.path.display());
+
+        let contents = fs::read(&image.path)
+            .with_context(|| format!("Failed to read image: {}", image.path.display()))?;


Double file read causes TOCTOU hash-content mismatch

Medium Severity

Each image file is read twice: once in collect_image_info to compute the SHA256 hash and dimensions, and again in upload_images to get the upload content. The hash from the first read becomes the objectstore key, but the content from the second read is what gets uploaded. If a file changes between reads, the manifest hash won't match the uploaded content. Storing the file contents in ImageInfo from the first read would fix both the correctness issue and avoid redundant I/O.

Additional Locations (1)

src/commands/build/snapshots.rs#L200-L201

src/commands/build/snapshots.rs

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

cursor · 2026-02-16T14:57:26Z

src/commands/build/snapshots.rs

+        debug!("Processing image: {}", image.path.display());
+
+        let contents = fs::read(&image.path)
+            .with_context(|| format!("Failed to read image: {}", image.path.display()))?;


Files read twice causing potential hash-content mismatch

Medium Severity

Each image file is read from disk twice: once in collect_image_info to compute the SHA-256 hash and dimensions, then again in upload_images to get the content for upload. The hash stored in ImageInfo comes from the first read, but the content uploaded to objectstore comes from the second fs::read. If a file is modified between the two reads, the obj_key (derived from the first read's hash) won't match the actually uploaded content, causing a silent data integrity mismatch in the manifest. Storing the file contents in ImageInfo during the first read would fix both the correctness issue and avoid the redundant I/O.

Additional Locations (1)

src/commands/build/snapshots.rs#L200-L215

cursor · 2026-02-16T14:57:26Z

src/commands/build/snapshots.rs

+
+        // SOF markers: C0-CF except C4 (DHT), C8 (JPG extension), and CC (DAC)
+        if (0xC0..=0xCF).contains(&marker) && marker != 0xC4 && marker != 0xC8 && marker != 0xCC {
+            if i + 7 < data.len() {


JPEG dimension bounds check off by one

Low Severity

The bounds check i + 7 < data.len() is one byte too strict. The highest index accessed is i + 6 (for the width bytes), so the correct guard is i + 7 <= data.len() (equivalently, i + 6 < data.len()). This causes read_jpeg_dimensions to return None for valid JPEG data where the SOF segment ends exactly at the buffer boundary.

lcian · 2026-02-20T17:31:02Z

Converting to draft until CI passes and this is reviewable again

noahsmartin and others added 8 commits December 17, 2025 20:02

feat(preprod): Add snapshots subcommand

95876c4

update deps

e45f588

no multiple projects, get rid of shard index

f3cfadb

add organization details API util

4794046

introduce necessary utils

c6e67e9

use batch API

1cd38b7

improve

15a89af

improve

1bc440d

This comment was marked as outdated.

Sign in to view

cargo clippy --fix

3417b53

lcian commented Jan 28, 2026

View reviewed changes

src/api/mod.rs Show resolved Hide resolved

improve

3f206dd

lcian commented Jan 28, 2026

View reviewed changes

fmt all

e902bfc

fix test

e5bd2d7

lcian commented Feb 3, 2026

View reviewed changes

src/commands/build/snapshots.rs Outdated Show resolved Hide resolved

lcian added 4 commits February 5, 2026 08:47

add auth, fix url

081e550

cargo clippy --fix

d13ddec

changelog

db855ee

Merge branch 'master' into lcian/feat/snapshots

b70e7b1

lcian marked this pull request as ready for review February 5, 2026 18:32

lcian requested review from a team and szokeasaurusrex as code owners February 5, 2026 18:32

cursor bot reviewed Feb 5, 2026

View reviewed changes

src/commands/build/snapshots.rs Outdated Show resolved Hide resolved

src/commands/build/snapshots.rs Show resolved Hide resolved

src/commands/build/snapshots.rs Outdated Show resolved Hide resolved

Updates to get e2e run working

0d59c90

rbro112 mentioned this pull request Feb 10, 2026

feat(preprod): Snapshots upload API getsentry/sentry#107825

Merged

szokeasaurusrex reviewed Feb 13, 2026

View reviewed changes

rbro112 and others added 7 commits February 13, 2026 17:09

Feedback

46b3edc

Add retention policy

56e0050

ref(api): Use rename_all on OrganizationLinks

e5f1f08

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ref(utils): Move objectstore module from directory to single file

4a3da60

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ref(utils): Accept AuthenticatedApi in get_objectstore_url

07c3b7a

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ref(api): Make PathArg public and use it in objectstore URL

3f4c1ff

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

fix(snapshots): Handle invalid auth token header gracefully

4596f8d

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

cursor bot reviewed Feb 16, 2026

View reviewed changes

use objectstore_client with native-tls feature

b0fc8b0

sentry bot reviewed Feb 16, 2026

View reviewed changes

src/commands/build/snapshots.rs Show resolved Hide resolved

cursor bot reviewed Feb 16, 2026

View reviewed changes

lcian added 2 commits February 20, 2026 18:28

improve

567b86d

improve

f2516bb

lcian marked this pull request as draft February 20, 2026 17:30

improve

7bd56f6

Uh oh!

Comments

Conversation

lcian commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

linear bot commented Jan 28, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

szokeasaurusrex commented Feb 6, 2026

Uh oh!

rbro112 commented Feb 10, 2026

Uh oh!

szokeasaurusrex left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Feb 16, 2026

Choose a reason for hiding this comment

Off-by-one in JPEG dimension bounds check

Uh oh!

cursor bot Feb 16, 2026

Choose a reason for hiding this comment

Double file read causes TOCTOU hash-content mismatch

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Feb 16, 2026

Choose a reason for hiding this comment

Files read twice causing potential hash-content mismatch

Uh oh!

cursor bot Feb 16, 2026

Choose a reason for hiding this comment

JPEG dimension bounds check off by one

Uh oh!

lcian commented Feb 20, 2026

Uh oh!

Reviewers

lcian commented Jan 28, 2026 •

edited

Loading

szokeasaurusrex left a comment •

edited

Loading