From 7da85f9dc026eca01ec04eb5562f28b7101ef7bf Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 3 Jul 2026 10:09:26 +0000 Subject: [PATCH 1/5] feat(snapshots): add upload command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port `snapshots upload` from the legacy Rust CLI. Scans a folder for PNG/JPEG screenshots, uploads each to Sentry's object store (skipping any already present by content hash), and POSTs a manifest to create the snapshot for visual diffing. Objectstore is live; until a native TS objectstore client ships we make the raw calls the Rust `objectstore-client` crate makes: - GET snapshots/upload-options → { objectstore: { url, scopes, authToken, expirationPolicy } }. - HEAD {url}/v1/objects/preprod/{k=v;...}/{orgId}/{projectId}/{sha256} (dedup), auth via `x-os-auth: Bearer `. - PUT the raw bytes with `x-sn-expiration`. Objects are keyed by the original file's SHA-256, so dedup is compression-independent (images are already compressed → uploaded raw). - POST snapshots/ with the manifest (app_id, per-image width/height/content_hash + sidecar, flattened VCS, diff_threshold/selective/all_image_file_names). New: src/lib/objectstore.ts (HEAD/PUT client), src/lib/snapshots/images.ts (collect + validate, dimensions via image-size), preprod-artifacts fetchSnapshotsUploadOptions/createPreprodSnapshot, src/commands/snapshots/upload.ts. Adds image-size as a devDependency. Tests cover the objectstore client, image collection/validation, the two API calls, and the command's dedup + manifest. --- docs/src/content/docs/contributing.md | 2 +- docs/src/fragments/commands/snapshots.md | 16 + package.json | 1 + plugins/sentry-cli/skills/sentry-cli/SKILL.md | 1 + .../skills/sentry-cli/references/snapshots.md | 30 ++ pnpm-lock.yaml | 10 + src/commands/snapshots/index.ts | 4 +- src/commands/snapshots/upload.ts | 498 ++++++++++++++++++ src/lib/api/preprod-artifacts.ts | 70 +++ src/lib/objectstore.ts | 130 +++++ src/lib/snapshots/images.ts | 168 ++++++ test/commands/snapshots/upload.test.ts | 200 +++++++ test/lib/api/preprod-artifacts.test.ts | 38 ++ test/lib/objectstore.test.ts | 104 ++++ test/lib/snapshots/images.test.ts | 128 +++++ 15 files changed, 1398 insertions(+), 2 deletions(-) create mode 100644 src/commands/snapshots/upload.ts create mode 100644 src/lib/objectstore.ts create mode 100644 src/lib/snapshots/images.ts create mode 100644 test/commands/snapshots/upload.test.ts create mode 100644 test/lib/objectstore.test.ts create mode 100644 test/lib/snapshots/images.test.ts diff --git a/docs/src/content/docs/contributing.md b/docs/src/content/docs/contributing.md index 2bf3adbe89..a3d0c9bc9d 100644 --- a/docs/src/content/docs/contributing.md +++ b/docs/src/content/docs/contributing.md @@ -71,7 +71,7 @@ cli/ │ │ ├── release/ # archive, create, delete, deploy, deploys, finalize, list, propose-version, restore, set-commits, view │ │ ├── replay/ # list, view │ │ ├── repo/ # list -│ │ ├── snapshots/ # diff, download +│ │ ├── snapshots/ # diff, download, upload │ │ ├── sourcemap/ # inject, resolve, upload │ │ ├── span/ # list, view │ │ ├── team/ # list diff --git a/docs/src/fragments/commands/snapshots.md b/docs/src/fragments/commands/snapshots.md index d4d8dc4eed..ee58e6f1f0 100644 --- a/docs/src/fragments/commands/snapshots.md +++ b/docs/src/fragments/commands/snapshots.md @@ -3,6 +3,15 @@ ## Examples ```bash +# Upload a folder of screenshots as a snapshot for an app +sentry snapshots upload ./screenshots --app-id com.example.app + +# Upload only a subset of images (removals/renames not inferred on PRs) +sentry snapshots upload ./screenshots --app-id my-app --selective + +# Only flag images that differ by more than 1% +sentry snapshots upload ./screenshots --app-id my-app --diff-threshold 0.01 + # Compare two directories of snapshot images locally sentry snapshots diff ./baseline ./head @@ -21,6 +30,13 @@ sentry snapshots download --app-id my-app --output ./baseline/ ## Important Notes +- `snapshots upload` scans a folder for PNG/JPEG images (skipping hidden files), + uploads each to Sentry's object store — images already present are skipped by + content hash — and creates a snapshot. **Sentry SaaS only.** A companion + `.json` sidecar adds per-image metadata; `--all-image-file-names` + (or `--all-image-file-names-file`) lists the full suite for selective uploads. + Each image must be at most 40,000,000 pixels. Git metadata is auto-collected + in CI (see `build upload`); a `--pr-number` requires a resolvable base SHA. - `snapshots diff` compares two local image directories (PNG/JPEG) perceptually — anti-aliasing aware, with a per-pixel `--threshold` (0.0–1.0) — and writes a PNG diff mask per changed image. It makes **no network requests**. Use diff --git a/package.json b/package.json index a9a1ed4e8a..23404aae4e 100644 --- a/package.json +++ b/package.json @@ -38,6 +38,7 @@ "hono": "^4.12.27", "http-cache-semantics": "^4.2.0", "ignore": "^7.0.5", + "image-size": "^2.0.2", "ink": "^7.1.0", "ink-spinner": "^5.0.0", "jpeg-js": "^0.4.4", diff --git a/plugins/sentry-cli/skills/sentry-cli/SKILL.md b/plugins/sentry-cli/skills/sentry-cli/SKILL.md index 355136aaa1..0bf05de498 100644 --- a/plugins/sentry-cli/skills/sentry-cli/SKILL.md +++ b/plugins/sentry-cli/skills/sentry-cli/SKILL.md @@ -526,6 +526,7 @@ Manage and compare snapshots - `sentry snapshots diff ` — Compare two directories of snapshot images - `sentry snapshots download` — Download baseline snapshot images +- `sentry snapshots upload ` — Upload snapshots to a project → Full flags and examples: `references/snapshots.md` diff --git a/plugins/sentry-cli/skills/sentry-cli/references/snapshots.md b/plugins/sentry-cli/skills/sentry-cli/references/snapshots.md index 566edc0b3a..004f19fabd 100644 --- a/plugins/sentry-cli/skills/sentry-cli/references/snapshots.md +++ b/plugins/sentry-cli/skills/sentry-cli/references/snapshots.md @@ -32,9 +32,39 @@ Download baseline snapshot images - `--branch - Git branch filter (only with --app-id)` - `-o, --output - Directory for extracted images (default: ./snapshots-base/)` +### `sentry snapshots upload ` + +Upload snapshots to a project + +**Flags:** +- `--app-id - The application identifier` +- `--diff-threshold - Only report an image as changed when its difference exceeds this fraction (0.0–1.0, e.g. 0.01 = 1%)` +- `--selective - This upload contains only a subset of images (removals/renames won't be detected on PRs)` +- `--all-image-file-names - Comma-separated list of all image names in the full suite (for selective uploads; implies --selective)` +- `--all-image-file-names-file - Path to a file listing all image names, one per line (for selective uploads; implies --selective)` +- `--head-sha - VCS commit SHA (defaults to the current commit)` +- `--base-sha - VCS base commit SHA (defaults to the merge-base with the base ref)` +- `--vcs-provider - VCS provider (defaults to the current remote's provider)` +- `--head-repo-name - Head repository name, e.g. owner/repo (defaults to the current)` +- `--base-repo-name - Base repository name, e.g. owner/repo (for forks)` +- `--head-ref - Head branch/reference (defaults to the current branch)` +- `--base-ref - Base branch/reference (defaults to the merge-base tracking ref)` +- `--pr-number - Pull request number (auto-detected in pull_request GitHub Actions runs)` +- `--force-git-metadata - Force collecting git metadata even outside CI (conflicts with --no-git-metadata)` +- `--no-git-metadata - Disable automatic git metadata collection` + **Examples:** ```bash +# Upload a folder of screenshots as a snapshot for an app +sentry snapshots upload ./screenshots --app-id com.example.app + +# Upload only a subset of images (removals/renames not inferred on PRs) +sentry snapshots upload ./screenshots --app-id my-app --selective + +# Only flag images that differ by more than 1% +sentry snapshots upload ./screenshots --app-id my-app --diff-threshold 0.01 + # Compare two directories of snapshot images locally sentry snapshots diff ./baseline ./head diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 085b61435f..a7690424ad 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -119,6 +119,9 @@ importers: ignore: specifier: ^7.0.5 version: 7.0.5 + image-size: + specifier: ^2.0.2 + version: 2.0.2 ink: specifier: ^7.1.0 version: 7.1.0(@types/react@19.2.17)(react-devtools-core@7.0.1)(react@19.2.7) @@ -1563,6 +1566,11 @@ packages: resolution: {integrity: sha512-Hs59xBNfUIunMFgWAbGX5cq6893IbWg4KnrjbYwX3tx0ztorVgTDA6B2sxf8ejHJ4wz8BqGUMYlnzNBer5NvGg==} engines: {node: '>= 4'} + image-size@2.0.2: + resolution: {integrity: sha512-IRqXKlaXwgSMAMtpNzZa1ZAe8m+Sa1770Dhk8VkSsP9LS+iHD62Zd8FQKs8fbPiagBE7BzoFX23cxFnwshpV6w==} + engines: {node: '>=16.x'} + hasBin: true + import-in-the-middle@3.2.0: resolution: {integrity: sha512-vR2B6HKIhaBjcZr2bLpFiJ1VbzOlRQ7aby4/gw5WPIzToLjqpfWw3VJ4sk1uDchoOODEirvO2jyrSPtUSL5CrQ==} engines: {node: '>=18'} @@ -4068,6 +4076,8 @@ snapshots: ignore@7.0.5: {} + image-size@2.0.2: {} + import-in-the-middle@3.2.0: dependencies: acorn: 8.17.0 diff --git a/src/commands/snapshots/index.ts b/src/commands/snapshots/index.ts index bdf94b5fef..aeae742f1f 100644 --- a/src/commands/snapshots/index.ts +++ b/src/commands/snapshots/index.ts @@ -1,17 +1,19 @@ /** * `sentry snapshots` — manage and compare preprod snapshot images. * - * Exposes `download` and `diff`. `upload` is ported separately (objectstore). + * Exposes `upload`, `download`, and `diff`. */ import { buildRouteMap } from "../../lib/route-map.js"; import { diffCommand } from "./diff.js"; import { downloadCommand } from "./download.js"; +import { uploadCommand } from "./upload.js"; export const snapshotsRoute = buildRouteMap({ routes: { diff: diffCommand, download: downloadCommand, + upload: uploadCommand, }, docs: { brief: "Manage and compare snapshots", diff --git a/src/commands/snapshots/upload.ts b/src/commands/snapshots/upload.ts new file mode 100644 index 0000000000..681c263907 --- /dev/null +++ b/src/commands/snapshots/upload.ts @@ -0,0 +1,498 @@ +/** + * sentry snapshots upload --app-id + * + * Upload a folder of screenshot images as a snapshot for visual diffing. Each + * image is hashed and uploaded to Objectstore (skipping any already present), + * then a manifest is POSTed to create the snapshot. Sentry SaaS only. + */ + +import { readFile, stat } from "node:fs/promises"; +import type { SentryContext } from "../../context.js"; +import { + type CreateSnapshotResponse, + createPreprodSnapshot, + fetchSnapshotsUploadOptions, +} from "../../lib/api/preprod-artifacts.js"; +import { + collectVcsMetadata, + isCi, + type VcsFlags, + type VcsInfo, + vcsInfoToBody, +} from "../../lib/build/vcs.js"; +import { buildCommand } from "../../lib/command.js"; +import { ContextError, ValidationError } from "../../lib/errors.js"; +import { + colorTag, + mdKvTable, + renderMarkdown, +} from "../../lib/formatters/markdown.js"; +import { CommandOutput } from "../../lib/formatters/output.js"; +import { logger } from "../../lib/logger.js"; +import { + type ObjectstoreConfig, + objectExists, + putObject, +} from "../../lib/objectstore.js"; +import { resolveOrgAndProject } from "../../lib/resolve-target.js"; +import { + type CollectedImage, + collectImages, + normalizeImageNames, + splitAndTrim, + validateImageSizes, +} from "../../lib/snapshots/images.js"; + +const log = logger.withTag("snapshots.upload"); + +const USAGE_HINT = "sentry snapshots upload --app-id "; + +/** Concurrency for objectstore HEAD/PUT requests. */ +const UPLOAD_CONCURRENCY = 8; + +/** Flags accepted by `snapshots upload`. */ +type UploadFlags = { + "app-id": string; + "diff-threshold"?: number; + selective?: boolean; + "all-image-file-names"?: string; + "all-image-file-names-file"?: string; +} & VcsFlags; + +/** Structured result for `snapshots upload`. */ +type SnapshotUploadResult = { + /** Number of image files discovered. */ + imagesFound: number; + /** Number of images newly uploaded to objectstore. */ + uploaded: number; + /** Number of images skipped (already present in objectstore). */ + skipped: number; + /** The created snapshot, or `null` when there were no images. */ + snapshot: CreateSnapshotResponse | null; +}; + +/** Parse `--diff-threshold` as a float in [0, 1]. */ +function parseDiffThreshold(value: string): number { + const parsed = Number(value); + if (Number.isNaN(parsed) || parsed < 0 || parsed > 1) { + throw new Error("diff threshold must be a number between 0.0 and 1.0"); + } + return parsed; +} + +/** Parse `--pr-number` as a non-negative integer. */ +function parsePrNumber(value: string): number { + const parsed = Number(value); + if (!Number.isInteger(parsed) || parsed < 0) { + throw new Error("PR number must be a non-negative integer"); + } + return parsed; +} + +/** + * Resolve `--all-image-file-names` / `--all-image-file-names-file` into a + * normalized list, or `undefined` when neither is set. + */ +async function resolveAllImageNames( + flags: UploadFlags +): Promise { + if (flags["all-image-file-names"] && flags["all-image-file-names-file"]) { + throw new ValidationError( + "--all-image-file-names and --all-image-file-names-file cannot be used together", + "all-image-file-names" + ); + } + if (flags["all-image-file-names"]) { + const names = normalizeImageNames( + splitAndTrim(flags["all-image-file-names"], ",") + ); + if (names.length === 0) { + throw new ValidationError( + "--all-image-file-names must not be empty", + "all-image-file-names" + ); + } + return names; + } + if (flags["all-image-file-names-file"]) { + const path = flags["all-image-file-names-file"]; + let content: string; + try { + content = await readFile(path, "utf8"); + } catch (err) { + log.debug(`Failed to read --all-image-file-names-file ${path}`, err); + throw new ValidationError( + `Failed to read --all-image-file-names-file: ${path}`, + "all-image-file-names-file" + ); + } + const names = normalizeImageNames(splitAndTrim(content, "\n")); + if (names.length === 0) { + throw new ValidationError( + `--all-image-file-names-file is empty or contains only blank lines: ${path}`, + "all-image-file-names-file" + ); + } + return names; + } + return; +} + +/** Run `fn` over `items` with bounded concurrency. */ +async function runPooled( + items: T[], + limit: number, + fn: (item: T) => Promise +): Promise { + for (let i = 0; i < items.length; i += limit) { + await Promise.all(items.slice(i, i + limit).map(fn)); + } +} + +/** A per-image manifest metadata entry (width/height override sidecar). */ +function imageMetadata(image: CollectedImage): Record { + return { + ...image.sidecar, + content_hash: image.hash, + width: image.width, + height: image.height, + }; +} + +/** Result of uploading images to objectstore. */ +type UploadImagesResult = { + /** Manifest entries keyed by relative image path. */ + entries: Record>; + /** Number of images newly uploaded. */ + uploaded: number; + /** Number of images skipped (already present). */ + skipped: number; +}; + +/** + * Upload the collected images to objectstore, deduping by content and skipping + * objects that already exist. + */ +async function uploadImages( + org: string, + project: string, + images: CollectedImage[] +): Promise { + const { objectstore } = await fetchSnapshotsUploadOptions(org, project); + const config: ObjectstoreConfig = objectstore; + + const findScope = (name: string): string | undefined => + config.scopes.find(([key]) => key === name)?.[1]; + const orgId = findScope("org"); + const projectId = findScope("project"); + if (!(orgId && projectId)) { + throw new ValidationError( + "Snapshot upload options are missing org/project scope", + "app-id" + ); + } + + const entries: Record> = {}; + const prepared: { path: string; key: string }[] = []; + const duplicates: string[] = []; + for (const image of images) { + if (entries[image.relativePath]) { + duplicates.push(image.relativePath); + continue; + } + entries[image.relativePath] = imageMetadata(image); + prepared.push({ + path: image.path, + key: `${orgId}/${projectId}/${image.hash}`, + }); + } + if (duplicates.length > 0) { + log.warn(`Duplicate paths encountered, skipping: ${duplicates.join(", ")}`); + } + + // HEAD to find objects already present, then PUT only the missing ones. + const existing = new Set(); + await runPooled(prepared, UPLOAD_CONCURRENCY, async (item) => { + if (await objectExists(config, item.key)) { + existing.add(item.key); + } + }); + const missing = prepared.filter((item) => !existing.has(item.key)); + await runPooled(missing, UPLOAD_CONCURRENCY, async (item) => { + await putObject(config, item.key, await readFile(item.path)); + }); + + return { + entries, + uploaded: missing.length, + skipped: prepared.length - missing.length, + }; +} + +/** Inputs for {@link buildManifest}. */ +type ManifestOptions = { + appId: string; + entries: Record>; + diffThreshold?: number; + selective: boolean; + allImageNames?: string[]; + vcs: VcsInfo; +}; + +/** Build the snapshot manifest body (VcsInfo flattened at the top level). */ +function buildManifest(opts: ManifestOptions): Record { + const manifest: Record = { + app_id: opts.appId, + images: opts.entries, + ...vcsInfoToBody(opts.vcs), + }; + if (opts.diffThreshold !== undefined) { + manifest.diff_threshold = opts.diffThreshold; + } + if (opts.selective) { + manifest.selective = true; + } + if (opts.allImageNames) { + manifest.all_image_file_names = opts.allImageNames; + } + return manifest; +} + +/** Human-readable formatter for the upload result. */ +function formatUploadResult(data: SnapshotUploadResult): string { + if (!data.snapshot) { + return renderMarkdown("No image files found."); + } + const rows: [string, string][] = [ + ["Snapshot", data.snapshot.artifactId], + ["Images", String(data.snapshot.imageCount)], + ["Uploaded", String(data.uploaded)], + ["Skipped (already present)", String(data.skipped)], + ]; + if (data.snapshot.snapshotUrl) { + rows.push(["URL", data.snapshot.snapshotUrl]); + } + return renderMarkdown( + `${colorTag("green", "Created snapshot")}\n\n${mdKvTable(rows)}` + ); +} + +export const uploadCommand = buildCommand({ + docs: { + brief: "Upload snapshots to a project", + fullDescription: + "Upload a folder of screenshot images as a snapshot for visual diffing.\n\n" + + "Each image (PNG/JPEG) is hashed and uploaded to Sentry's object store " + + "(images already present are skipped), then a manifest is created. " + + "Companion `.json` sidecar files add per-image metadata. " + + "This feature only works with Sentry SaaS.\n\n" + + "Usage:\n" + + " sentry snapshots upload ./screenshots --app-id com.example.app\n" + + " sentry snapshots upload ./shots --app-id my-app --diff-threshold 0.01\n" + + " sentry snapshots upload ./shots --app-id my-app --selective", + }, + output: { + human: formatUploadResult, + }, + parameters: { + positional: { + kind: "tuple", + parameters: [ + { + brief: "Path to the folder containing images to upload", + parse: String, + placeholder: "path", + }, + ], + }, + flags: { + "app-id": { + kind: "parsed", + parse: String, + brief: "The application identifier", + }, + "diff-threshold": { + kind: "parsed", + parse: parseDiffThreshold, + brief: + "Only report an image as changed when its difference exceeds this fraction (0.0–1.0, e.g. 0.01 = 1%)", + optional: true, + }, + selective: { + kind: "boolean", + brief: + "This upload contains only a subset of images (removals/renames won't be detected on PRs)", + optional: true, + }, + "all-image-file-names": { + kind: "parsed", + parse: String, + brief: + "Comma-separated list of all image names in the full suite (for selective uploads; implies --selective)", + optional: true, + }, + "all-image-file-names-file": { + kind: "parsed", + parse: String, + brief: + "Path to a file listing all image names, one per line (for selective uploads; implies --selective)", + optional: true, + }, + "head-sha": { + kind: "parsed", + parse: String, + brief: "VCS commit SHA (defaults to the current commit)", + optional: true, + }, + "base-sha": { + kind: "parsed", + parse: String, + brief: + "VCS base commit SHA (defaults to the merge-base with the base ref)", + optional: true, + }, + "vcs-provider": { + kind: "parsed", + parse: String, + brief: "VCS provider (defaults to the current remote's provider)", + optional: true, + }, + "head-repo-name": { + kind: "parsed", + parse: String, + brief: + "Head repository name, e.g. owner/repo (defaults to the current)", + optional: true, + }, + "base-repo-name": { + kind: "parsed", + parse: String, + brief: "Base repository name, e.g. owner/repo (for forks)", + optional: true, + }, + "head-ref": { + kind: "parsed", + parse: String, + brief: "Head branch/reference (defaults to the current branch)", + optional: true, + }, + "base-ref": { + kind: "parsed", + parse: String, + brief: + "Base branch/reference (defaults to the merge-base tracking ref)", + optional: true, + }, + "pr-number": { + kind: "parsed", + parse: parsePrNumber, + brief: + "Pull request number (auto-detected in pull_request GitHub Actions runs)", + optional: true, + }, + "force-git-metadata": { + kind: "boolean", + brief: + "Force collecting git metadata even outside CI (conflicts with --no-git-metadata)", + optional: true, + }, + "no-git-metadata": { + kind: "boolean", + brief: "Disable automatic git metadata collection", + optional: true, + }, + }, + }, + async *func(this: SentryContext, flags: UploadFlags, path: string) { + const info = await stat(path).catch(() => null); + if (!info?.isDirectory()) { + throw new ValidationError(`Path is not a directory: ${path}`, "path"); + } + + const resolved = await resolveOrgAndProject({ + cwd: this.cwd, + usageHint: USAGE_HINT, + }); + if (!resolved) { + throw new ContextError("Organization and project", USAGE_HINT); + } + const { org, project } = resolved; + + if (flags["force-git-metadata"] && flags["no-git-metadata"]) { + throw new ValidationError( + "--force-git-metadata and --no-git-metadata cannot be used together", + "force-git-metadata" + ); + } + const shouldCollectVcs = + Boolean(flags["force-git-metadata"]) || + (!flags["no-git-metadata"] && isCi(this.env)); + const vcs = collectVcsMetadata(flags, this.cwd, this.env, shouldCollectVcs); + if (vcs.prNumber !== undefined && !vcs.baseSha) { + throw new ValidationError( + "A PR number was provided but no base SHA could be determined. " + + "Pass --base-sha explicitly or ensure your CI exposes the merge base.", + "pr-number" + ); + } + + const images = await collectImages(path); + if (images.length === 0) { + yield new CommandOutput({ + imagesFound: 0, + uploaded: 0, + skipped: 0, + snapshot: null, + }); + return { hint: "No image files found." }; + } + validateImageSizes(images); + + const allImageNames = await resolveAllImageNames(flags); + const selective = Boolean(flags.selective) || allImageNames !== undefined; + if (allImageNames) { + const known = new Set(allImageNames); + const unknown = images + .map((img) => img.relativePath) + .filter((key) => !known.has(key)) + .sort(); + if (unknown.length > 0) { + throw new ValidationError( + `The following uploaded images are not in --all-image-file-names: ${unknown.join( + ", " + )}`, + "all-image-file-names" + ); + } + } + + log.info(`Uploading ${images.length} image(s)...`); + const { entries, uploaded, skipped } = await uploadImages( + org, + project, + images + ); + + const manifest = buildManifest({ + appId: flags["app-id"], + entries, + diffThreshold: flags["diff-threshold"], + selective, + allImageNames, + vcs, + }); + const snapshot = await createPreprodSnapshot(org, project, manifest); + + yield new CommandOutput({ + imagesFound: images.length, + uploaded, + skipped, + snapshot, + }); + return { + hint: snapshot.snapshotUrl + ? `View your snapshot at ${snapshot.snapshotUrl}` + : "View your snapshot in Sentry.", + }; + }, +}); diff --git a/src/lib/api/preprod-artifacts.ts b/src/lib/api/preprod-artifacts.ts index 7aa97ea4cf..7779c339b9 100644 --- a/src/lib/api/preprod-artifacts.ts +++ b/src/lib/api/preprod-artifacts.ts @@ -384,6 +384,76 @@ export async function getLatestBaseSnapshot( } } +/** Objectstore config within the snapshots upload-options response. */ +const ObjectstoreUploadOptionsSchema = z.object({ + url: z.string(), + scopes: z.array(z.tuple([z.string(), z.string()])), + authToken: z.string().nullish(), + expirationPolicy: z.string(), +}); + +/** Response from `.../snapshots/upload-options/`. */ +const SnapshotsUploadOptionsSchema = z.object({ + objectstore: ObjectstoreUploadOptionsSchema, +}); + +/** Snapshot upload options (objectstore config), for the caller. */ +export type SnapshotsUploadOptions = z.infer< + typeof SnapshotsUploadOptionsSchema +>; + +/** + * Fetch objectstore upload options (URL, scopes, token, expiration) for + * uploading snapshot images. + * + * @throws {ApiError} On a non-2xx response. + */ +export async function fetchSnapshotsUploadOptions( + org: string, + project: string +): Promise { + const regionUrl = await resolveOrgRegion(org); + const { data } = await apiRequestToRegion( + regionUrl, + `projects/${org}/${project}/preprodartifacts/snapshots/upload-options/`, + { schema: SnapshotsUploadOptionsSchema } + ); + return data; +} + +/** Response from the create-snapshot endpoint. */ +const CreateSnapshotResponseSchema = z.object({ + artifactId: z.string(), + imageCount: z.number(), + snapshotUrl: z.string().nullish(), +}); + +/** Result of creating a preprod snapshot. */ +export type CreateSnapshotResponse = z.infer< + typeof CreateSnapshotResponseSchema +>; + +/** + * Create a preprod snapshot from an uploaded image manifest. + * + * @param manifest - The snapshot manifest (app id, per-image metadata, VCS, + * selective flags). + * @throws {ApiError} On a non-2xx response. + */ +export async function createPreprodSnapshot( + org: string, + project: string, + manifest: Record +): Promise { + const regionUrl = await resolveOrgRegion(org); + const { data } = await apiRequestToRegion( + regionUrl, + `projects/${org}/${project}/preprodartifacts/snapshots/`, + { method: "POST", body: manifest, schema: CreateSnapshotResponseSchema } + ); + return data; +} + const SnapshotArchiveStatusSchema = z.object({ ready: z.boolean() }); /** diff --git a/src/lib/objectstore.ts b/src/lib/objectstore.ts new file mode 100644 index 0000000000..05bf15e4d2 --- /dev/null +++ b/src/lib/objectstore.ts @@ -0,0 +1,130 @@ +/** + * Minimal Objectstore HTTP client for snapshot image uploads. + * + * Replicates the subset of the `objectstore-client` protocol (getsentry/objectstore) + * that `snapshots upload` needs: HEAD an object to check existence (dedup) and + * PUT its bytes. The service URL, scopes, auth token, and expiration policy all + * come from the Sentry `snapshots/upload-options/` endpoint — this client never + * signs tokens itself (the token is a pre-signed JWT). + * + * Object path layout: `{serviceUrl}/v1/objects/{usecase}/{scope}/{key}` where + * `scope` is a `;`-joined list of ordered `key=value` pairs. Auth is carried in + * the `x-os-auth: Bearer ` header (not the standard `Authorization`). + */ + +import { customFetch } from "./custom-ca.js"; +import { ApiError } from "./errors.js"; + +/** The Objectstore usecase snapshots are stored under. */ +export const OBJECTSTORE_USECASE = "preprod"; + +/** Header carrying the Objectstore bearer token. */ +const AUTH_HEADER = "x-os-auth"; +/** Header carrying an object's expiration policy (e.g. `ttl:30d`). */ +const EXPIRATION_HEADER = "x-sn-expiration"; + +/** Matches one or more trailing slashes (for base-URL normalization). */ +const TRAILING_SLASHES = /\/+$/; + +/** + * Objectstore upload configuration, as returned (camelCase) by the Sentry + * `snapshots/upload-options/` endpoint. + */ +export type ObjectstoreConfig = { + /** Base service URL (may include a path prefix). */ + url: string; + /** Ordered scope pairs (e.g. `[["org","1"],["project","2"]]`). */ + scopes: [string, string][]; + /** Pre-signed bearer token, or null/absent for unauthenticated stores. */ + authToken?: string | null; + /** Expiration policy string applied to uploaded objects. */ + expirationPolicy: string; +}; + +/** Render scope pairs into the `k=v;k=v` path segment. */ +function scopeSegment(scopes: [string, string][]): string { + return scopes.map(([key, value]) => `${key}=${value}`).join(";"); +} + +/** + * Build the full URL for an object key within the configured usecase + scope. + * + * @param config - The objectstore configuration. + * @param key - The object key (e.g. `//`). + */ +export function buildObjectUrl(config: ObjectstoreConfig, key: string): string { + const base = config.url.replace(TRAILING_SLASHES, ""); + return `${base}/v1/objects/${OBJECTSTORE_USECASE}/${scopeSegment( + config.scopes + )}/${key}`; +} + +/** Auth headers for an objectstore request, if a token is configured. */ +function authHeaders(config: ObjectstoreConfig): Record { + return config.authToken + ? { [AUTH_HEADER]: `Bearer ${config.authToken}` } + : {}; +} + +/** + * Check whether an object already exists (used to skip re-uploads). + * + * @returns `true` if the object exists, `false` on a 404. + * @throws {ApiError} On any non-2xx, non-404 response. + */ +export async function objectExists( + config: ObjectstoreConfig, + key: string +): Promise { + const url = buildObjectUrl(config, key); + const response = await customFetch(url, { + method: "HEAD", + headers: authHeaders(config), + }); + if (response.status === 404) { + return false; + } + if (!response.ok) { + throw new ApiError( + "Objectstore HEAD failed", + response.status, + response.statusText || "HEAD failed", + url + ); + } + return true; +} + +/** + * Upload an object's bytes (raw — no content encoding). + * + * The key is derived from the original file's SHA-256, so dedup via + * {@link objectExists} is independent of any upload compression; images are + * already-compressed formats, so storing them raw avoids pointless CPU. + * + * @throws {ApiError} On a non-2xx response. + */ +export async function putObject( + config: ObjectstoreConfig, + key: string, + body: Uint8Array +): Promise { + const url = buildObjectUrl(config, key); + const response = await customFetch(url, { + method: "PUT", + headers: { + ...authHeaders(config), + [EXPIRATION_HEADER]: config.expirationPolicy, + "content-type": "application/octet-stream", + }, + body, + }); + if (!response.ok) { + throw new ApiError( + "Objectstore upload failed", + response.status, + response.statusText || "PUT failed", + url + ); + } +} diff --git a/src/lib/snapshots/images.ts b/src/lib/snapshots/images.ts new file mode 100644 index 0000000000..f0b970f45d --- /dev/null +++ b/src/lib/snapshots/images.ts @@ -0,0 +1,168 @@ +/** + * Snapshot image collection for `snapshots upload`. + * + * Walks a directory for PNG/JPEG screenshots (skipping hidden files), reads each + * image's dimensions (header-only, via `image-size`) and SHA-256, and loads any + * companion `.json` sidecar metadata. Mirrors the legacy Rust + * `collect_images` / `validate_image_sizes` behaviour. + */ + +import { createHash } from "node:crypto"; +import { readFile } from "node:fs/promises"; +import { imageSize } from "image-size"; +import { ValidationError } from "../errors.js"; +import { logger } from "../logger.js"; +import { walkFiles } from "../scan/walker.js"; + +const log = logger.withTag("snapshots.images"); + +/** Matches a file extension (for deriving the sidecar `.json` path). */ +const FILE_EXTENSION = /\.[^./\\]+$/; + +/** Image extensions considered snapshot images (without a leading dot). */ +export const IMAGE_EXTENSIONS: ReadonlySet = new Set([ + "png", + "jpg", + "jpeg", +]); + +/** Same extensions in the walker's `.ext` (lowercased, dotted) form. */ +const WALK_EXTENSIONS: ReadonlySet = new Set( + [...IMAGE_EXTENSIONS].map((ext) => `.${ext}`) +); + +/** Maximum pixels (width × height) allowed per image. */ +export const MAX_PIXELS_PER_IMAGE = 40_000_000; + +/** A discovered snapshot image with its metadata. */ +export type CollectedImage = { + /** Absolute path on disk. */ + path: string; + /** Path relative to the scan root, as a forward-slash URL key. */ + relativePath: string; + /** Image width in pixels. */ + width: number; + /** Image height in pixels. */ + height: number; + /** SHA-256 hex digest of the file's bytes. */ + hash: string; + /** Parsed companion `.json` sidecar metadata (empty if none). */ + sidecar: Record; +}; + +/** Normalize a filesystem-relative path to a forward-slash URL key. */ +export function pathAsUrl(relativePath: string): string { + return relativePath.replaceAll("\\", "/"); +} + +/** Read and parse an image's `.json` sidecar metadata, if present. */ +async function readSidecarMetadata( + imagePath: string +): Promise> { + const sidecarPath = imagePath.replace(FILE_EXTENSION, ".json"); + if (sidecarPath === imagePath) { + return {}; + } + try { + const parsed = JSON.parse(await readFile(sidecarPath, "utf8")); + return parsed && typeof parsed === "object" + ? (parsed as Record) + : {}; + } catch (err) { + // Missing sidecars are expected; malformed ones are ignored (matching the + // legacy CLI, which warns and drops them rather than failing the upload). + log.debug(`No usable sidecar for ${imagePath}`, err); + return {}; + } +} + +/** + * Collect snapshot images under `dir`. + * + * Reads each file once (for dimensions + hash) and does not retain its bytes. + * Images whose dimensions cannot be read are skipped with a warning, matching + * the legacy CLI. + * + * @param dir - Directory to scan. + * @returns The collected images (unsorted). + */ +export async function collectImages(dir: string): Promise { + const images: CollectedImage[] = []; + for await (const entry of walkFiles({ + cwd: dir, + extensions: WALK_EXTENSIONS, + hidden: false, + followSymlinks: true, + respectGitignore: false, + alwaysSkipDirs: [], + maxFileSize: Number.POSITIVE_INFINITY, + classifyBinary: false, + })) { + const content = await readFile(entry.absolutePath); + let width: number | undefined; + let height: number | undefined; + try { + const size = imageSize(content); + width = size.width; + height = size.height; + } catch (err) { + log.warn(`Could not read dimensions from ${entry.relativePath}: ${err}`); + continue; + } + if (!(width && height)) { + log.warn(`Could not read dimensions from ${entry.relativePath}`); + continue; + } + + const hash = createHash("sha256").update(content).digest("hex"); + const sidecar = await readSidecarMetadata(entry.absolutePath); + images.push({ + path: entry.absolutePath, + relativePath: pathAsUrl(entry.relativePath), + width, + height, + hash, + sidecar, + }); + } + return images; +} + +/** + * Validate that no image exceeds {@link MAX_PIXELS_PER_IMAGE}. + * + * @throws {ValidationError} Listing every violating image. + */ +export function validateImageSizes(images: CollectedImage[]): void { + const violations = images + .filter((img) => img.width * img.height > MAX_PIXELS_PER_IMAGE) + .map( + (img) => + ` ${img.relativePath} (${img.width}x${img.height} = ${ + img.width * img.height + } pixels)` + ); + if (violations.length > 0) { + throw new ValidationError( + `The following images exceed the maximum pixel limit of ${MAX_PIXELS_PER_IMAGE}:\n${violations.join( + "\n" + )}`, + "path" + ); + } +} + +/** Split a string on `separator`, trimming and dropping empty entries. */ +export function splitAndTrim(input: string, separator: string): string[] { + return input + .split(separator) + .map((s) => s.trim()) + .filter((s) => s.length > 0); +} + +/** Normalize image name entries: strip a leading `./` and `\`→`/`. */ +export function normalizeImageNames(names: string[]): string[] { + return names.map((s) => + (s.startsWith("./") ? s.slice(2) : s).replaceAll("\\", "/") + ); +} diff --git a/test/commands/snapshots/upload.test.ts b/test/commands/snapshots/upload.test.ts new file mode 100644 index 0000000000..cb40a9ef4c --- /dev/null +++ b/test/commands/snapshots/upload.test.ts @@ -0,0 +1,200 @@ +/** + * Tests for `sentry snapshots upload`. + * + * Drives the command via its wrapper `loader()`. Org/project resolution, the + * upload-options + create-snapshot API, and the objectstore HEAD/PUT primitives + * are spied; image collection runs for real against PNG fixtures in a temp dir. + */ + +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { PNG } from "pngjs"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { uploadCommand } from "../../../src/commands/snapshots/upload.js"; +// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking +import * as preprod from "../../../src/lib/api/preprod-artifacts.js"; +import { ValidationError } from "../../../src/lib/errors.js"; +// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking +import * as objectstore from "../../../src/lib/objectstore.js"; +// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking +import * as resolveTarget from "../../../src/lib/resolve-target.js"; + +let tmpDir: string; + +function createContext() { + const writes: string[] = []; + return { + context: { + stdout: { + write: (data: string | Uint8Array) => { + writes.push( + typeof data === "string" ? data : new TextDecoder().decode(data) + ); + return true; + }, + }, + stderr: { write: () => true }, + cwd: tmpDir, + env: {} as NodeJS.ProcessEnv, + process: { ...process, exitCode: undefined } as typeof process, + }, + output: () => writes.join(""), + get exitCode() { + return this.context.process.exitCode; + }, + }; +} + +function pngBytes(width: number, height: number): Buffer { + const png = new PNG({ width, height }); + png.data.fill(0xff); + return PNG.sync.write(png); +} + +const UPLOAD_OPTIONS = { + objectstore: { + url: "https://os.example.com", + scopes: [ + ["org", "1"], + ["project", "2"], + ] as [string, string][], + authToken: "tok", + expirationPolicy: "ttl:30d", + }, +}; + +describe("snapshots upload", () => { + let existsSpy: ReturnType; + let putSpy: ReturnType; + let createSpy: ReturnType; + + beforeEach(async () => { + tmpDir = await mkdtemp(join(tmpdir(), "snap-up-")); + vi.spyOn(resolveTarget, "resolveOrgAndProject").mockResolvedValue({ + org: "test-org", + project: "test-project", + }); + vi.spyOn(preprod, "fetchSnapshotsUploadOptions").mockResolvedValue( + UPLOAD_OPTIONS + ); + existsSpy = vi.spyOn(objectstore, "objectExists").mockResolvedValue(false); + putSpy = vi.spyOn(objectstore, "putObject").mockResolvedValue(undefined); + createSpy = vi.spyOn(preprod, "createPreprodSnapshot").mockResolvedValue({ + artifactId: "snap-1", + imageCount: 2, + snapshotUrl: "https://sentry.io/snap-1", + }); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + await rm(tmpDir, { recursive: true, force: true }); + }); + + /** Write a folder with two PNGs (one nested) + a sidecar; returns its path. */ + async function writeShots(): Promise { + const dir = join(tmpDir, "shots"); + await mkdir(join(dir, "sub"), { recursive: true }); + await writeFile(join(dir, "a.png"), pngBytes(4, 3)); + await writeFile(join(dir, "a.json"), JSON.stringify({ note: "hi" })); + await writeFile(join(dir, "sub", "b.png"), pngBytes(2, 2)); + return dir; + } + + test("uploads images and creates a snapshot with a correct manifest", async () => { + const dir = await writeShots(); + const harness = createContext(); + const func = await uploadCommand.loader(); + + await func.call(harness.context, { "app-id": "com.example.app" }, dir); + + // Two images uploaded (none pre-existing). + expect(putSpy).toHaveBeenCalledTimes(2); + expect(createSpy).toHaveBeenCalledTimes(1); + + const [, , manifest] = createSpy.mock.calls[0] as [ + string, + string, + Record, + ]; + expect(manifest.app_id).toBe("com.example.app"); + const images = manifest.images as Record>; + expect(Object.keys(images).sort()).toEqual(["a.png", "sub/b.png"]); + expect(images["a.png"]).toMatchObject({ + width: 4, + height: 3, + note: "hi", + }); + expect(images["a.png"].content_hash).toMatch(/^[0-9a-f]{64}$/); + // selective omitted when not requested. + expect(manifest.selective).toBeUndefined(); + expect(harness.output()).toContain("snap-1"); + }); + + test("skips objects already present in objectstore", async () => { + const dir = await writeShots(); + // First image already exists, second does not. + existsSpy.mockResolvedValueOnce(true).mockResolvedValueOnce(false); + const harness = createContext(); + const func = await uploadCommand.loader(); + + await func.call(harness.context, { "app-id": "app" }, dir); + + expect(putSpy).toHaveBeenCalledTimes(1); + }); + + test("reports no images and creates nothing for an empty folder", async () => { + const dir = join(tmpDir, "empty"); + await mkdir(dir); + const harness = createContext(); + const func = await uploadCommand.loader(); + + await func.call(harness.context, { "app-id": "app" }, dir); + + expect(createSpy).not.toHaveBeenCalled(); + expect(putSpy).not.toHaveBeenCalled(); + expect(harness.output()).toContain("No image files found"); + }); + + test("rejects a path that is not a directory", async () => { + const file = join(tmpDir, "a.png"); + await writeFile(file, pngBytes(1, 1)); + const func = await uploadCommand.loader(); + await expect( + func.call(createContext().context, { "app-id": "app" }, file) + ).rejects.toThrow(ValidationError); + }); + + test("marks selective + rejects images missing from --all-image-file-names", async () => { + const dir = await writeShots(); + const func = await uploadCommand.loader(); + await expect( + func.call( + createContext().context, + { "app-id": "app", "all-image-file-names": "a.png" }, + dir + ) + ).rejects.toThrow(ValidationError); + }); + + test("passes diff-threshold and selective into the manifest", async () => { + const dir = await writeShots(); + const harness = createContext(); + const func = await uploadCommand.loader(); + + await func.call( + harness.context, + { "app-id": "app", "diff-threshold": 0.05, selective: true }, + dir + ); + + const [, , manifest] = createSpy.mock.calls[0] as [ + string, + string, + Record, + ]; + expect(manifest.diff_threshold).toBe(0.05); + expect(manifest.selective).toBe(true); + }); +}); diff --git a/test/lib/api/preprod-artifacts.test.ts b/test/lib/api/preprod-artifacts.test.ts index 68e2d0e633..02b6326b9e 100644 --- a/test/lib/api/preprod-artifacts.test.ts +++ b/test/lib/api/preprod-artifacts.test.ts @@ -62,7 +62,9 @@ vi.mock("../../../src/lib/api/chunk-upload.js", async (importOriginal) => { import { buildFormatFromUrl, + createPreprodSnapshot, downloadBuildArtifact, + fetchSnapshotsUploadOptions, getBuildInstallDetails, getLatestBaseSnapshot, LatestBaseSnapshotSchema, @@ -372,6 +374,42 @@ describe("snapshots", () => { ).toBe(false); }); + test("fetchSnapshotsUploadOptions hits the upload-options endpoint", async () => { + apiRequestToRegionMock.mockResolvedValue({ + data: { + objectstore: { + url: "https://os.example.com", + scopes: [["org", "1"]], + authToken: "tok", + expirationPolicy: "ttl:30d", + }, + }, + }); + const opts = await fetchSnapshotsUploadOptions("my-org", "my-project"); + expect(opts.objectstore.url).toBe("https://os.example.com"); + const [, endpoint] = apiRequestToRegionMock.mock.calls.at(-1) ?? []; + expect(endpoint).toBe( + "projects/my-org/my-project/preprodartifacts/snapshots/upload-options/" + ); + }); + + test("createPreprodSnapshot POSTs the manifest and parses the response", async () => { + apiRequestToRegionMock.mockResolvedValue({ + data: { artifactId: "snap-1", imageCount: 3, snapshotUrl: "https://s" }, + }); + const manifest = { app_id: "app", images: {} }; + const res = await createPreprodSnapshot("my-org", "my-project", manifest); + expect(res.artifactId).toBe("snap-1"); + expect(res.imageCount).toBe(3); + const [, endpoint, options] = + apiRequestToRegionMock.mock.calls.at(-1) ?? []; + expect(endpoint).toBe( + "projects/my-org/my-project/preprodartifacts/snapshots/" + ); + expect(options.method).toBe("POST"); + expect(options.body).toBe(manifest); + }); + test("getLatestBaseSnapshot maps the response and forwards params", async () => { // apiRequestToRegion returns schema-validated snake_case data. apiRequestToRegionMock.mockResolvedValue({ diff --git a/test/lib/objectstore.test.ts b/test/lib/objectstore.test.ts new file mode 100644 index 0000000000..a21cdc9d7e --- /dev/null +++ b/test/lib/objectstore.test.ts @@ -0,0 +1,104 @@ +/** + * Tests for the minimal Objectstore HTTP client. + * + * `customFetch` is mocked so URL construction, the `x-os-auth` header, HEAD + * existence semantics, and PUT request shape can be verified without a network. + */ + +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { ApiError } from "../../src/lib/errors.js"; + +const { customFetchMock } = vi.hoisted(() => ({ customFetchMock: vi.fn() })); +vi.mock("../../src/lib/custom-ca.js", () => ({ customFetch: customFetchMock })); + +import { + buildObjectUrl, + type ObjectstoreConfig, + objectExists, + putObject, +} from "../../src/lib/objectstore.js"; + +const config: ObjectstoreConfig = { + url: "https://objectstore.example.com/", + scopes: [ + ["org", "123"], + ["project", "456"], + ], + authToken: "jwt-token", + expirationPolicy: "ttl:30d", +}; + +beforeEach(() => { + customFetchMock.mockReset(); +}); +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("buildObjectUrl", () => { + test("joins usecase, scope, and key (stripping a trailing slash)", () => { + expect(buildObjectUrl(config, "123/456/abc")).toBe( + "https://objectstore.example.com/v1/objects/preprod/org=123;project=456/123/456/abc" + ); + }); +}); + +describe("objectExists", () => { + test("returns true on a 2xx HEAD and sends the auth header", async () => { + customFetchMock.mockResolvedValue({ ok: true, status: 200 }); + expect(await objectExists(config, "123/456/abc")).toBe(true); + const [url, init] = customFetchMock.mock.calls[0] ?? []; + expect(url).toContain( + "/v1/objects/preprod/org=123;project=456/123/456/abc" + ); + expect(init.method).toBe("HEAD"); + expect(init.headers["x-os-auth"]).toBe("Bearer jwt-token"); + }); + + test("returns false on a 404", async () => { + customFetchMock.mockResolvedValue({ ok: false, status: 404 }); + expect(await objectExists(config, "123/456/abc")).toBe(false); + }); + + test("throws on other non-2xx responses", async () => { + customFetchMock.mockResolvedValue({ + ok: false, + status: 500, + statusText: "err", + }); + await expect(objectExists(config, "123/456/abc")).rejects.toThrow(ApiError); + }); + + test("omits the auth header when no token is configured", async () => { + customFetchMock.mockResolvedValue({ ok: true, status: 200 }); + await objectExists({ ...config, authToken: null }, "k"); + const [, init] = customFetchMock.mock.calls[0] ?? []; + expect(init.headers["x-os-auth"]).toBeUndefined(); + }); +}); + +describe("putObject", () => { + test("PUTs the body with auth + expiration headers", async () => { + customFetchMock.mockResolvedValue({ ok: true, status: 200 }); + const body = new Uint8Array([1, 2, 3]); + await putObject(config, "123/456/abc", body); + + const [url, init] = customFetchMock.mock.calls[0] ?? []; + expect(url).toContain("/123/456/abc"); + expect(init.method).toBe("PUT"); + expect(init.headers["x-os-auth"]).toBe("Bearer jwt-token"); + expect(init.headers["x-sn-expiration"]).toBe("ttl:30d"); + expect(init.body).toBe(body); + }); + + test("throws on a non-2xx response", async () => { + customFetchMock.mockResolvedValue({ + ok: false, + status: 413, + statusText: "too large", + }); + await expect(putObject(config, "k", new Uint8Array([0]))).rejects.toThrow( + ApiError + ); + }); +}); diff --git a/test/lib/snapshots/images.test.ts b/test/lib/snapshots/images.test.ts new file mode 100644 index 0000000000..43b2484104 --- /dev/null +++ b/test/lib/snapshots/images.test.ts @@ -0,0 +1,128 @@ +/** + * Tests for snapshot image collection + validation. + * + * Real PNG fixtures are generated in-memory with pngjs (no committed binaries) + * so `image-size` reads genuine headers. + */ + +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { PNG } from "pngjs"; +import { afterEach, describe, expect, test } from "vitest"; +import { ValidationError } from "../../../src/lib/errors.js"; +import { + type CollectedImage, + collectImages, + MAX_PIXELS_PER_IMAGE, + normalizeImageNames, + splitAndTrim, + validateImageSizes, +} from "../../../src/lib/snapshots/images.js"; + +/** Encode a solid PNG of the given dimensions. */ +function pngBytes(width: number, height: number): Buffer { + const png = new PNG({ width, height }); + png.data.fill(0xff); + return PNG.sync.write(png); +} + +const dirs: string[] = []; +function tempTree(build: (root: string) => void): string { + const root = mkdtempSync(join(tmpdir(), "snap-img-")); + dirs.push(root); + build(root); + return root; +} +afterEach(() => { + while (dirs.length > 0) { + const d = dirs.pop(); + if (d) { + rmSync(d, { recursive: true, force: true }); + } + } +}); + +describe("collectImages", () => { + test("collects PNGs with dimensions, hash, and sidecar; skips hidden + non-images", async () => { + const root = tempTree((r) => { + writeFileSync(join(r, "a.png"), pngBytes(4, 3)); + writeFileSync(join(r, "a.json"), JSON.stringify({ custom: "value" })); + mkdirSync(join(r, "sub")); + writeFileSync(join(r, "sub", "b.png"), pngBytes(2, 2)); + writeFileSync(join(r, "notes.txt"), "not an image"); + writeFileSync(join(r, ".hidden.png"), pngBytes(1, 1)); + }); + + const images = await collectImages(root); + const byKey = new Map(images.map((i) => [i.relativePath, i])); + + expect([...byKey.keys()].sort()).toEqual(["a.png", "sub/b.png"]); + const a = byKey.get("a.png"); + expect(a?.width).toBe(4); + expect(a?.height).toBe(3); + expect(a?.hash).toMatch(/^[0-9a-f]{64}$/); + expect(a?.sidecar).toEqual({ custom: "value" }); + // No sidecar for sub/b.png → empty object. + expect(byKey.get("sub/b.png")?.sidecar).toEqual({}); + }); + + test("returns an empty list for a directory with no images", async () => { + const root = tempTree((r) => { + writeFileSync(join(r, "readme.md"), "hi"); + }); + expect(await collectImages(root)).toEqual([]); + }); +}); + +describe("validateImageSizes", () => { + function img(width: number, height: number): CollectedImage { + return { + path: "/x.png", + relativePath: "x.png", + width, + height, + hash: "h", + sidecar: {}, + }; + } + + test("passes at the pixel limit", () => { + expect(() => validateImageSizes([img(8000, 5000)])).not.toThrow(); // 40,000,000 + }); + + test("throws listing images over the pixel limit", () => { + expect(() => validateImageSizes([img(8001, 5000)])).toThrow( + ValidationError + ); + expect(() => validateImageSizes([img(8001, 5000)])).toThrow( + String(MAX_PIXELS_PER_IMAGE) + ); + }); +}); + +describe("splitAndTrim", () => { + test("splits on a comma, trimming and dropping empties", () => { + expect(splitAndTrim("a.png, b.png , , c.png", ",")).toEqual([ + "a.png", + "b.png", + "c.png", + ]); + }); + + test("splits on newlines", () => { + expect(splitAndTrim("a.png\nb.png\n\nc.png\n", "\n")).toEqual([ + "a.png", + "b.png", + "c.png", + ]); + }); +}); + +describe("normalizeImageNames", () => { + test("strips a leading ./ and backslashes → forward slashes", () => { + expect(normalizeImageNames(["./img/a.png", "img\\b.png", "c.png"])).toEqual( + ["img/a.png", "img/b.png", "c.png"] + ); + }); +}); From 035242ac2acada751215798f4f8fedb06f5b646a Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 3 Jul 2026 10:10:40 +0000 Subject: [PATCH 2/5] refactor(snapshots): omit Content-Type on objectstore PUT (match reference default) --- src/lib/objectstore.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib/objectstore.ts b/src/lib/objectstore.ts index 05bf15e4d2..81ebab0d07 100644 --- a/src/lib/objectstore.ts +++ b/src/lib/objectstore.ts @@ -115,7 +115,6 @@ export async function putObject( headers: { ...authHeaders(config), [EXPIRATION_HEADER]: config.expirationPolicy, - "content-type": "application/octet-stream", }, body, }); From d592dbd1dce34fb29b3c5701bf6744ab49e2750c Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 3 Jul 2026 10:31:31 +0000 Subject: [PATCH 3/5] fix(snapshots): zstd manifest body + self-review fixes - M1: POST the create-snapshot manifest with Content-Encoding: zstd (matching the legacy CLI's with_zstd_json_body) so large image suites don't risk a body limit. Adds an opt-in `bodyEncoding: "zstd"` to apiRequestToRegion (additive; falls back to plain JSON when the runtime lacks zstd). - L2: ignore array/scalar JSON sidecars (only objects are usable metadata). - L3: enforce the --all-image-file-names[/-file] conflict before the empty-folder early return. - N1: reject an empty --diff-threshold (Number("") was silently accepted as 0). - Tests: assert the objectstore key is {orgId}/{projectId}/{sha256}; sidecar width/height/content_hash override; --pr-number without a base SHA is rejected. --- src/commands/snapshots/upload.ts | 14 ++++---- src/lib/api/infrastructure.ts | 26 +++++++++++++-- src/lib/api/preprod-artifacts.ts | 8 ++++- src/lib/snapshots/images.ts | 4 ++- test/commands/snapshots/upload.test.ts | 44 ++++++++++++++++++++++++++ 5 files changed, 85 insertions(+), 11 deletions(-) diff --git a/src/commands/snapshots/upload.ts b/src/commands/snapshots/upload.ts index 681c263907..b6af3dcf47 100644 --- a/src/commands/snapshots/upload.ts +++ b/src/commands/snapshots/upload.ts @@ -74,7 +74,7 @@ type SnapshotUploadResult = { /** Parse `--diff-threshold` as a float in [0, 1]. */ function parseDiffThreshold(value: string): number { const parsed = Number(value); - if (Number.isNaN(parsed) || parsed < 0 || parsed > 1) { + if (value.trim() === "" || Number.isNaN(parsed) || parsed < 0 || parsed > 1) { throw new Error("diff threshold must be a number between 0.0 and 1.0"); } return parsed; @@ -96,12 +96,6 @@ function parsePrNumber(value: string): number { async function resolveAllImageNames( flags: UploadFlags ): Promise { - if (flags["all-image-file-names"] && flags["all-image-file-names-file"]) { - throw new ValidationError( - "--all-image-file-names and --all-image-file-names-file cannot be used together", - "all-image-file-names" - ); - } if (flags["all-image-file-names"]) { const names = normalizeImageNames( splitAndTrim(flags["all-image-file-names"], ",") @@ -424,6 +418,12 @@ export const uploadCommand = buildCommand({ "force-git-metadata" ); } + if (flags["all-image-file-names"] && flags["all-image-file-names-file"]) { + throw new ValidationError( + "--all-image-file-names and --all-image-file-names-file cannot be used together", + "all-image-file-names" + ); + } const shouldCollectVcs = Boolean(flags["force-git-metadata"]) || (!flags["no-git-metadata"] && isCi(this.env)); diff --git a/src/lib/api/infrastructure.ts b/src/lib/api/infrastructure.ts index 15694b0d14..8b0083c6fc 100644 --- a/src/lib/api/infrastructure.ts +++ b/src/lib/api/infrastructure.ts @@ -6,6 +6,8 @@ * other modules in `src/lib/api/` import from. */ +import { promisify } from "node:util"; +import { zstdCompress as zstdCompressCb } from "node:zlib"; import { parseSentryLinkHeader } from "@sentry/api"; // biome-ignore lint/performance/noNamespaceImport: Sentry SDK recommends namespace import import * as Sentry from "@sentry/node-core/light"; @@ -171,10 +173,20 @@ function enrichDetail( */ export const parseLinkHeader = parseSentryLinkHeader; +/** zstd body compressor, or null when the runtime lacks zstd support. */ +const zstdCompressAsync = + typeof zstdCompressCb === "function" ? promisify(zstdCompressCb) : null; + /** Options for raw API requests to Sentry endpoints. */ export type ApiRequestOptions = { method?: "GET" | "POST" | "PUT" | "DELETE" | "PATCH"; body?: unknown; + /** + * Compress the JSON body with zstd and send `Content-Encoding: zstd`. Useful + * for large bodies (e.g. a snapshot manifest). Silently falls back to plain + * JSON when the runtime lacks zstd support. + */ + bodyEncoding?: "zstd"; /** Query parameters. String arrays create repeated keys (e.g., tags=1&tags=2) */ params?: Record; /** Optional Zod schema for runtime validation of response data */ @@ -448,7 +460,7 @@ export async function apiRequestToRegion( endpoint: string, options: ApiRequestOptions = {} ): Promise<{ data: T; headers: Headers }> { - const { method = "GET", body, params, schema } = options; + const { method = "GET", body, bodyEncoding, params, schema } = options; const config = getSdkConfig(regionUrl); const searchParams = buildSearchParams(params); @@ -463,10 +475,20 @@ export async function apiRequestToRegion( const headers: Record = { "Content-Type": "application/json", }; + let requestBody: string | Uint8Array | undefined; + if (body !== undefined) { + const json = JSON.stringify(body); + if (bodyEncoding === "zstd" && zstdCompressAsync) { + requestBody = await zstdCompressAsync(Buffer.from(json)); + headers["Content-Encoding"] = "zstd"; + } else { + requestBody = json; + } + } const response = await fetchFn(url, { method, headers, - body: body ? JSON.stringify(body) : undefined, + body: requestBody, }); if (!response.ok) { diff --git a/src/lib/api/preprod-artifacts.ts b/src/lib/api/preprod-artifacts.ts index 7779c339b9..fa3201ea3a 100644 --- a/src/lib/api/preprod-artifacts.ts +++ b/src/lib/api/preprod-artifacts.ts @@ -449,7 +449,13 @@ export async function createPreprodSnapshot( const { data } = await apiRequestToRegion( regionUrl, `projects/${org}/${project}/preprodartifacts/snapshots/`, - { method: "POST", body: manifest, schema: CreateSnapshotResponseSchema } + { + method: "POST", + body: manifest, + // A large image suite makes a large manifest; compress like the legacy CLI. + bodyEncoding: "zstd", + schema: CreateSnapshotResponseSchema, + } ); return data; } diff --git a/src/lib/snapshots/images.ts b/src/lib/snapshots/images.ts index f0b970f45d..c7c11a9760 100644 --- a/src/lib/snapshots/images.ts +++ b/src/lib/snapshots/images.ts @@ -65,7 +65,9 @@ async function readSidecarMetadata( } try { const parsed = JSON.parse(await readFile(sidecarPath, "utf8")); - return parsed && typeof parsed === "object" + // Only a JSON object is usable metadata; arrays/scalars are ignored (as the + // legacy CLI does, deserializing into a map and dropping non-objects). + return parsed && typeof parsed === "object" && !Array.isArray(parsed) ? (parsed as Record) : {}; } catch (err) { diff --git a/test/commands/snapshots/upload.test.ts b/test/commands/snapshots/upload.test.ts index cb40a9ef4c..22d8e3d2f3 100644 --- a/test/commands/snapshots/upload.test.ts +++ b/test/commands/snapshots/upload.test.ts @@ -130,6 +130,50 @@ describe("snapshots upload", () => { // selective omitted when not requested. expect(manifest.selective).toBeUndefined(); expect(harness.output()).toContain("snap-1"); + + // The objectstore key is `{orgId}/{projectId}/{sha256}` from the scope. + const key = putSpy.mock.calls[0]?.[1] as string; + expect(key).toMatch(/^1\/2\/[0-9a-f]{64}$/); + expect(key.endsWith(images["a.png"].content_hash as string)).toBe(true); + }); + + test("CLI width/height/content_hash override sidecar keys", async () => { + const dir = join(tmpDir, "shots"); + await mkdir(dir, { recursive: true }); + await writeFile(join(dir, "a.png"), pngBytes(4, 3)); + await writeFile( + join(dir, "a.json"), + JSON.stringify({ width: 999, height: 888, content_hash: "nope", keep: 1 }) + ); + const harness = createContext(); + const func = await uploadCommand.loader(); + + await func.call(harness.context, { "app-id": "app" }, dir); + + const [, , manifest] = createSpy.mock.calls[0] as [ + string, + string, + Record, + ]; + const entry = (manifest.images as Record>)[ + "a.png" + ]; + expect(entry.width).toBe(4); + expect(entry.height).toBe(3); + expect(entry.content_hash).toMatch(/^[0-9a-f]{64}$/); + expect(entry.keep).toBe(1); + }); + + test("rejects --pr-number without a resolvable base SHA", async () => { + const dir = await writeShots(); + const func = await uploadCommand.loader(); + await expect( + func.call( + createContext().context, + { "app-id": "app", "pr-number": 7 }, + dir + ) + ).rejects.toThrow(ValidationError); }); test("skips objects already present in objectstore", async () => { From c24f1e095f3ade1291849180380c9a8f3a5e3165 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 3 Jul 2026 10:33:41 +0000 Subject: [PATCH 4/5] refactor(snapshots): extract collectVcs + resolveSelective helpers Reduce the upload func's cognitive complexity below the Biome limit after the added flag-conflict check. --- src/commands/snapshots/upload.ts | 96 +++++++++++++++++++++----------- 1 file changed, 62 insertions(+), 34 deletions(-) diff --git a/src/commands/snapshots/upload.ts b/src/commands/snapshots/upload.ts index b6af3dcf47..bb7aa4d6db 100644 --- a/src/commands/snapshots/upload.ts +++ b/src/commands/snapshots/upload.ts @@ -132,6 +132,66 @@ async function resolveAllImageNames( return; } +/** + * Validate the git-metadata flags and collect VCS info for the manifest. + * + * @throws {ValidationError} On conflicting flags or a PR number without a base SHA. + */ +function collectVcs( + flags: UploadFlags, + cwd: string, + env: NodeJS.ProcessEnv +): VcsInfo { + if (flags["force-git-metadata"] && flags["no-git-metadata"]) { + throw new ValidationError( + "--force-git-metadata and --no-git-metadata cannot be used together", + "force-git-metadata" + ); + } + const shouldCollect = + Boolean(flags["force-git-metadata"]) || + (!flags["no-git-metadata"] && isCi(env)); + const vcs = collectVcsMetadata(flags, cwd, env, shouldCollect); + if (vcs.prNumber !== undefined && !vcs.baseSha) { + throw new ValidationError( + "A PR number was provided but no base SHA could be determined. " + + "Pass --base-sha explicitly or ensure your CI exposes the merge base.", + "pr-number" + ); + } + return vcs; +} + +/** + * Resolve the selective-upload settings and validate that every collected image + * is present in `--all-image-file-names` (when provided). + * + * @throws {ValidationError} When an uploaded image is not in the provided list. + */ +async function resolveSelective( + flags: UploadFlags, + images: CollectedImage[] +): Promise<{ selective: boolean; allImageNames?: string[] }> { + const allImageNames = await resolveAllImageNames(flags); + const selective = Boolean(flags.selective) || allImageNames !== undefined; + if (allImageNames) { + const known = new Set(allImageNames); + const unknown = images + .map((img) => img.relativePath) + .filter((key) => !known.has(key)) + .sort(); + if (unknown.length > 0) { + throw new ValidationError( + `The following uploaded images are not in --all-image-file-names: ${unknown.join( + ", " + )}`, + "all-image-file-names" + ); + } + } + return { selective, allImageNames }; +} + /** Run `fn` over `items` with bounded concurrency. */ async function runPooled( items: T[], @@ -412,29 +472,13 @@ export const uploadCommand = buildCommand({ } const { org, project } = resolved; - if (flags["force-git-metadata"] && flags["no-git-metadata"]) { - throw new ValidationError( - "--force-git-metadata and --no-git-metadata cannot be used together", - "force-git-metadata" - ); - } if (flags["all-image-file-names"] && flags["all-image-file-names-file"]) { throw new ValidationError( "--all-image-file-names and --all-image-file-names-file cannot be used together", "all-image-file-names" ); } - const shouldCollectVcs = - Boolean(flags["force-git-metadata"]) || - (!flags["no-git-metadata"] && isCi(this.env)); - const vcs = collectVcsMetadata(flags, this.cwd, this.env, shouldCollectVcs); - if (vcs.prNumber !== undefined && !vcs.baseSha) { - throw new ValidationError( - "A PR number was provided but no base SHA could be determined. " + - "Pass --base-sha explicitly or ensure your CI exposes the merge base.", - "pr-number" - ); - } + const vcs = collectVcs(flags, this.cwd, this.env); const images = await collectImages(path); if (images.length === 0) { @@ -448,23 +492,7 @@ export const uploadCommand = buildCommand({ } validateImageSizes(images); - const allImageNames = await resolveAllImageNames(flags); - const selective = Boolean(flags.selective) || allImageNames !== undefined; - if (allImageNames) { - const known = new Set(allImageNames); - const unknown = images - .map((img) => img.relativePath) - .filter((key) => !known.has(key)) - .sort(); - if (unknown.length > 0) { - throw new ValidationError( - `The following uploaded images are not in --all-image-file-names: ${unknown.join( - ", " - )}`, - "all-image-file-names" - ); - } - } + const { selective, allImageNames } = await resolveSelective(flags, images); log.info(`Uploading ${images.length} image(s)...`); const { entries, uploaded, skipped } = await uploadImages( From bf53d5751f5c9eea83efcbaba3ba2f3998c30302 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 3 Jul 2026 10:45:14 +0000 Subject: [PATCH 5/5] fix(snapshots): resolve upload path vs cwd + objectstore timeouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Cursor (High): resolve the positional folder against the command's cwd before scanning — the walker requires an absolute path, so a relative `./screenshots` passed the stat check but would fail the walk. - Warden (High): add request timeouts to the objectstore HEAD (30s) and PUT (120s) via AbortSignal.timeout, so a stalled connection can't hang the CLI / a CI job forever. - Tests: relative-path resolution against cwd; HEAD/PUT carry an AbortSignal. --- src/commands/snapshots/upload.ts | 7 +++++-- src/lib/objectstore.ts | 7 +++++++ test/commands/snapshots/upload.test.ts | 11 +++++++++++ test/lib/objectstore.test.ts | 2 ++ 4 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/commands/snapshots/upload.ts b/src/commands/snapshots/upload.ts index bb7aa4d6db..145be2cd2b 100644 --- a/src/commands/snapshots/upload.ts +++ b/src/commands/snapshots/upload.ts @@ -7,6 +7,7 @@ */ import { readFile, stat } from "node:fs/promises"; +import { resolve } from "node:path"; import type { SentryContext } from "../../context.js"; import { type CreateSnapshotResponse, @@ -458,7 +459,9 @@ export const uploadCommand = buildCommand({ }, }, async *func(this: SentryContext, flags: UploadFlags, path: string) { - const info = await stat(path).catch(() => null); + // Resolve against the command's cwd; the walker requires an absolute path. + const dir = resolve(this.cwd, path); + const info = await stat(dir).catch(() => null); if (!info?.isDirectory()) { throw new ValidationError(`Path is not a directory: ${path}`, "path"); } @@ -480,7 +483,7 @@ export const uploadCommand = buildCommand({ } const vcs = collectVcs(flags, this.cwd, this.env); - const images = await collectImages(path); + const images = await collectImages(dir); if (images.length === 0) { yield new CommandOutput({ imagesFound: 0, diff --git a/src/lib/objectstore.ts b/src/lib/objectstore.ts index 81ebab0d07..8ba0355f57 100644 --- a/src/lib/objectstore.ts +++ b/src/lib/objectstore.ts @@ -26,6 +26,11 @@ const EXPIRATION_HEADER = "x-sn-expiration"; /** Matches one or more trailing slashes (for base-URL normalization). */ const TRAILING_SLASHES = /\/+$/; +/** Timeout for a HEAD existence check. */ +const HEAD_TIMEOUT_MS = 30_000; +/** Timeout for uploading a single object. */ +const PUT_TIMEOUT_MS = 120_000; + /** * Objectstore upload configuration, as returned (camelCase) by the Sentry * `snapshots/upload-options/` endpoint. @@ -80,6 +85,7 @@ export async function objectExists( const response = await customFetch(url, { method: "HEAD", headers: authHeaders(config), + signal: AbortSignal.timeout(HEAD_TIMEOUT_MS), }); if (response.status === 404) { return false; @@ -117,6 +123,7 @@ export async function putObject( [EXPIRATION_HEADER]: config.expirationPolicy, }, body, + signal: AbortSignal.timeout(PUT_TIMEOUT_MS), }); if (!response.ok) { throw new ApiError( diff --git a/test/commands/snapshots/upload.test.ts b/test/commands/snapshots/upload.test.ts index 22d8e3d2f3..a538b0ec6a 100644 --- a/test/commands/snapshots/upload.test.ts +++ b/test/commands/snapshots/upload.test.ts @@ -176,6 +176,17 @@ describe("snapshots upload", () => { ).rejects.toThrow(ValidationError); }); + test("resolves a relative folder path against the command cwd", async () => { + await writeShots(); // creates /shots + const harness = createContext(); // cwd === tmpDir + const func = await uploadCommand.loader(); + + await func.call(harness.context, { "app-id": "app" }, "shots"); + + expect(putSpy).toHaveBeenCalledTimes(2); + expect(createSpy).toHaveBeenCalledTimes(1); + }); + test("skips objects already present in objectstore", async () => { const dir = await writeShots(); // First image already exists, second does not. diff --git a/test/lib/objectstore.test.ts b/test/lib/objectstore.test.ts index a21cdc9d7e..d73077d696 100644 --- a/test/lib/objectstore.test.ts +++ b/test/lib/objectstore.test.ts @@ -53,6 +53,7 @@ describe("objectExists", () => { ); expect(init.method).toBe("HEAD"); expect(init.headers["x-os-auth"]).toBe("Bearer jwt-token"); + expect(init.signal).toBeInstanceOf(AbortSignal); }); test("returns false on a 404", async () => { @@ -89,6 +90,7 @@ describe("putObject", () => { expect(init.headers["x-os-auth"]).toBe("Bearer jwt-token"); expect(init.headers["x-sn-expiration"]).toBe("ttl:30d"); expect(init.body).toBe(body); + expect(init.signal).toBeInstanceOf(AbortSignal); }); test("throws on a non-2xx response", async () => {