From 8e072c225f21aae6f47a42c5a6022e97132f2169 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Fri, 18 Sep 2026 19:28:45 +0500 Subject: [PATCH 1/2] feat(debug-files): --strip-sources-content, for teams who do not want their source uploaded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit §8: a source map's `sourcesContent` embeds the original source verbatim, and it rides into the upload with the map. That is how a symbolicated crash shows source lines — and also means the customer's code leaves the build machine. There was no way to say no. `--strip-sources-content` (`--type sourcemaps`) removes it from the COPY that is uploaded. File, line and column symbolication is unaffected; what is lost is the snippet shown beside a frame. - The map on disk is never modified. It is the caller's build output and their own debugging, and the bundler plugins delete these maps after a successful upload anyway. - The declared `hash` describes the stripped bytes, not the file that was read — the wire field is supposed to describe what was uploaded. - The debug-id is carried over untouched: it is the key, and re-deriving it would break the pair with the bundle `sourcemaps inject` stamped. - A map with no `sourcesContent`, or one that is not the JSON object we expect, is uploaded byte-for-byte unchanged. This is a privacy preference, not a validator. - Rejected with exit 20 for every other `--type`, like the other sourcemap-only flags: no other symbol format embeds source. Measured on a real esbuild bundle: 253 → 181 bytes uploaded, local map untouched. Five mutants caught (flag ignored, always-on, nothing-to-strip still re-serialized, wrong hash, flag accepted for other types) — the third needed the test strengthened to assert BYTE equality, not just the absence of the key. Two new e2e flows unzip the captured PUT and check both the upload and the file on disk. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- CHANGELOG.md | 13 ++ README.md | 7 + scripts/e2e_flows.py | 30 +++++ src/cli/debug_files.rs | 268 +++++++++++++++++++++++++++++++++++++ tests/debug_files_flags.rs | 11 ++ 5 files changed, 329 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74f1334..3457639 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,19 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). carrying an unresolvable debug-id `missing_sym`, which is what prompts an upload — an unstamped bundle is silently unsymbolicated instead. +- **`debug-files upload --strip-sources-content`** (`--type sourcemaps`) — upload each map without + its embedded original source. `sourcesContent` carries your code verbatim, and it is what lets a + symbolicated crash show source lines; stripping it keeps file/line/column resolution and drops the + snippet, for teams who would rather their source did not leave the build machine. + + The map on disk is never modified — only the copy that is uploaded — and the declared `hash` + describes the stripped bytes rather than the file that was read. A map that carries no + `sourcesContent` (or is not the JSON object we expect) is uploaded byte-for-byte unchanged: this + is a privacy preference, not a validator. Rejected (exit 20) for every other `--type`, since no + other symbol format embeds source. + + Measured on a real esbuild bundle: 253 → 181 bytes uploaded, local map untouched. + ### Fixed - **`debug-files upload --type sourcemaps --dry-run` no longer fails on a map that has no debug-id.** A dry run sends nothing, so an un-keyed map cannot register the unfindable symbol the diff --git a/README.md b/README.md index 6959e80..4771b69 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,7 @@ bugsee-cli debug-files upload ... \ [--force] # re-upload even if the server already has it (dsym/pdb/rust/il2cpp-linemap/sourcemaps) [--concurrency N] # sourcemaps only: ceiling on uploads in flight, 1..=32 (default: scaled) [--allow-empty] # sourcemaps only: "nothing to upload" is success, not exit 10 + [--strip-sources-content] # sourcemaps only: upload maps without the embedded source [--dry-run] ``` @@ -221,6 +222,12 @@ rather than fatal there (`unkeyed` in the completion log) — the whole flow can freshly built directory, where `sourcemaps inject --dry-run` has deliberately written nothing yet. A REAL run still refuses such a map (exit 11): uploading it would register a symbol nothing can find. +`--strip-sources-content` uploads each map WITHOUT its `sourcesContent`, for teams who would rather +their source did not leave the build machine. Symbolication still resolves file, line and column; +what is lost is the source snippet shown beside a crash frame. The map on disk is never modified — +only the copy that is uploaded — and the declared `hash` describes the stripped bytes. A map that +carries no `sourcesContent` is uploaded byte-for-byte unchanged. + `--allow-empty` turns "nothing to upload" into success (exit 0) instead of exit 10 — a monorepo package built without maps, or a framework whose server output has none, is a legitimate no-op rather than a reason to fail the build. diff --git a/scripts/e2e_flows.py b/scripts/e2e_flows.py index 834ec54..429c26a 100644 --- a/scripts/e2e_flows.py +++ b/scripts/e2e_flows.py @@ -449,6 +449,36 @@ def main(): os.path.join(fix, "mapping.txt")] + v) results["sourcemaps_upload"] = run(binpath, "sourcemaps", ["debug-files", "upload", "--type", "sourcemaps", os.path.join(fix, "dist", "app.js.map")] + v) + + # --strip-sources-content: the uploaded copy must not carry the source, and the file on disk + # must be untouched — it is the user's build output. + sc_map = os.path.join(fix, "with-sources", "app.js.map") + os.makedirs(os.path.dirname(sc_map), exist_ok=True) + with open(sc_map, "w") as f: + json.dump({"version": 3, "debug_id": "77777777-7777-7777-7777-777777777777", + "sources": ["app.ts"], "sourcesContent": ["const secret = 1;"], + "names": [], "mappings": "AAAA"}, f) + before = open(sc_map, "rb").read() + results["sourcemaps_strip_sources_content"] = run( + binpath, "sourcemaps_strip", + ["debug-files", "upload", "--type", "sourcemaps", "--strip-sources-content", sc_map] + v) + try: + import zipfile as _zf + uploaded = None + for name in os.listdir(STATE["cap"]): + if name.startswith("sourcemaps_strip__") and name.endswith(".bin"): + with _zf.ZipFile(os.path.join(STATE["cap"], name)) as z: + uploaded = json.loads(z.read(z.namelist()[0])) + results["sourcemaps_strip_uploads_no_source"] = ( + uploaded is not None + and "sourcesContent" not in uploaded + and uploaded.get("mappings") == "AAAA" + and open(sc_map, "rb").read() == before) + if not results["sourcemaps_strip_uploads_no_source"]: + print(f" [warn] uploaded={uploaded!r}") + except Exception as e: + print(" [warn] could not verify the strip flow:", e) + results["sourcemaps_strip_uploads_no_source"] = False results["elf"] = run(binpath, "elf", ["debug-files", "upload", "--type", "elf", os.path.join(fix, "native-debug-symbols.zip"), "--uuid", "11111111-2222-3333-4444-555555555555"] + v) diff --git a/src/cli/debug_files.rs b/src/cli/debug_files.rs index a2eeae3..cb592fd 100644 --- a/src/cli/debug_files.rs +++ b/src/cli/debug_files.rs @@ -117,6 +117,15 @@ pub enum DebugFilesCommand { #[arg(long)] allow_empty: bool, + /// Upload source maps WITHOUT their embedded original source. `--type sourcemaps` only. + /// + /// A map's `sourcesContent` carries your source verbatim, which is how a symbolicated crash + /// shows source lines. Stripping it keeps file/line/column symbolication and drops the + /// snippet, for teams who would rather their code did not leave the build machine. The map + /// on disk is NOT modified — only the copy that is uploaded. + #[arg(long)] + strip_sources_content: bool, + /// Dry-run — discover and pack files but skip the HTTP upload. #[arg(long)] dry_run: bool, @@ -210,6 +219,7 @@ pub async fn dispatch( force, concurrency, allow_empty, + strip_sources_content, dry_run, } => { let kind = r#type.unwrap_or(DebugFileType::Proguard); @@ -260,6 +270,11 @@ pub async fn dispatch( "--allow-empty is only valid for --type sourcemaps — every other type treats an empty input as a configuration mistake", )); } + if kind != DebugFileType::Sourcemaps && strip_sources_content { + return Err(config_invalid( + "--strip-sources-content is only valid for --type sourcemaps — no other symbol format embeds source", + )); + } if kind != DebugFileType::Sourcemaps && concurrency.is_some() { return Err(config_invalid( "--concurrency is only valid for --type sourcemaps — no other type uploads a batch of independent files", @@ -370,6 +385,7 @@ pub async fn dispatch( concurrency.map(usize::from), allow_empty, dry_run, + strip_sources_content, ) .await; } @@ -1399,6 +1415,9 @@ struct SourcemapUploadCtx<'a> { build: &'a str, strategy: Strategy, force: bool, + /// Drop `sourcesContent` from the COPY that is uploaded. The file on disk is the caller's build + /// output and is never rewritten. + strip_sources_content: bool, } /// Pack ONE `.map` and register + PUT it. `None` when this was a dry run (packed, never sent). @@ -1420,6 +1439,15 @@ async fn upload_one_sourcemap( .file_name() .and_then(|n| n.to_str()) .unwrap_or("bundle.js.map"); + + // `--strip-sources-content`: pack a COPY without the embedded original source. The file on disk + // is the caller's build output — rewriting it would take away their own debugging, and the + // bundler plugins delete these maps after a successful upload anyway. + let stripped = stripped_copy_without_sources(map_path, tmpdir.path(), ctx)?; + let (map_path, identity) = match stripped.as_ref() { + Some((path, id)) => (path.as_path(), id), + None => (map_path, identity), + }; // zstd-11 over a source map is real CPU work (~6 ms for a typical map, ~22 ms for a 1.6 MB // one). All these futures are polled by ONE task, so packing inline would serialise across // concurrent uploads and stall the in-flight requests' I/O while it ran. @@ -1508,6 +1536,7 @@ async fn run_sourcemap_upload( concurrency: Option, allow_empty: bool, dry_run: bool, + strip_sources_content: bool, ) -> anyhow::Result<()> { // A path that does not exist is a typo, not an empty build: it must not be swallowed by // `--allow-empty`, and naming it beats the "no .map files found under …" it used to produce. @@ -1686,6 +1715,7 @@ async fn run_sourcemap_upload( build, strategy, force, + strip_sources_content, }, &map_path, &identity, @@ -1739,6 +1769,53 @@ fn is_stylesheet_or_declaration_map(path: &Path) -> bool { .any(|suffix| name.ends_with(suffix)) } +/// A copy of `map_path` with `sourcesContent` removed, plus its recomputed identity — or `None` +/// when nothing needs stripping (flag off, no `sourcesContent`, or the map is not the JSON object we +/// expect, which is a privacy preference's business to ignore rather than to fail over). +/// +/// The identity is recomputed because the declared `hash` must describe what is UPLOADED. The +/// debug-id is carried over untouched: it is the key, and re-deriving it here would break the pair +/// with the bundle that `sourcemaps inject` stamped. +fn stripped_copy_without_sources( + map_path: &Path, + tmpdir: &Path, + ctx: SourcemapUploadCtx<'_>, +) -> anyhow::Result> { + if !ctx.strip_sources_content { + return Ok(None); + } + let bytes = std::fs::read(map_path)?; + let Ok(serde_json::Value::Object(mut map)) = serde_json::from_slice(&bytes) else { + return Ok(None); + }; + if map.remove("sourcesContent").is_none() { + return Ok(None); + } + let stripped = serde_json::to_vec(&map)?; + let name = map_path + .file_name() + .map(Path::new) + .unwrap_or_else(|| Path::new("bundle.js.map")); + let out = tmpdir.join(name); + std::fs::write(&out, &stripped)?; + tracing::info!( + path = %map_path.display(), + before_bytes = bytes.len(), + after_bytes = stripped.len(), + "stripped sourcesContent from the uploaded copy" + ); + let identity = sourcemap::SourcemapIdentity { + debug_id: crate::inject::read_debug_id(&out)?, + content_sha1_hex: { + use sha1::Digest; + let digest: [u8; 20] = sha1::Sha1::digest(&stripped).into(); + hex::encode(digest) + }, + size_bytes: stripped.len() as u64, + }; + Ok(Some((out, identity))) +} + /// Pack a `.dSYM` bundle's entries into a temp zip with the chosen strategy. /// Returns the tempdir guard (keep it alive until the PUT finishes), the zip /// path, and the zip size. Called only once the server has confirmed it wants @@ -2015,6 +2092,7 @@ mod sourcemap_upload_tests { None, false, false, + false, ) .await .unwrap(); @@ -2102,6 +2180,7 @@ mod sourcemap_upload_tests { None, false, false, + false, ) .await .unwrap_err(); @@ -2133,6 +2212,7 @@ mod sourcemap_upload_tests { None, false, true, + false, ) .await .unwrap(); @@ -2223,6 +2303,7 @@ mod sourcemap_upload_tests { concurrency: Option, allow_empty: bool, dry_run: bool, + strip_sources_content: bool, } async fn upload_paths( @@ -2242,6 +2323,7 @@ mod sourcemap_upload_tests { tweak.concurrency, tweak.allow_empty, tweak.dry_run, + tweak.strip_sources_content, ) .await } @@ -2378,6 +2460,7 @@ mod sourcemap_upload_tests { None, false, false, + false, ) .await .unwrap_err(); @@ -2414,6 +2497,7 @@ mod sourcemap_upload_tests { None, false, dry_run, + false, ) .await .unwrap_err(); @@ -2603,6 +2687,7 @@ mod sourcemap_upload_tests { None, false, false, + false, ) .await .unwrap(); @@ -2643,6 +2728,7 @@ mod sourcemap_upload_tests { None, false, false, + false, ) .await .unwrap(); @@ -2852,6 +2938,7 @@ mod sourcemap_upload_tests { Some(8), false, false, + false, ) .await .unwrap(); @@ -3019,9 +3106,189 @@ mod sourcemap_upload_tests { None, false, true, + false, + ) + .await + .unwrap(); + } + + /// What we actually PUT: the single `.map` entry, decompressed out of the uploaded zip. The + /// only way to assert on the bytes that leave the machine rather than on the ones on disk. + async fn uploaded_map_bytes(server: &MockServer) -> Vec { + let requests = server.received_requests().await.unwrap(); + let put = requests + .iter() + .find(|r| r.method.as_str() == "PUT") + .expect("no PUT"); + let mut archive = + zip::ZipArchive::new(std::io::Cursor::new(put.body.clone())).expect("not a zip"); + assert_eq!(archive.len(), 1, "one entry: just the map"); + let mut entry = archive.by_index(0).unwrap(); + let mut body = Vec::new(); + std::io::Read::read_to_end(&mut entry, &mut body).unwrap(); + body + } + + async fn uploaded_map_json(server: &MockServer) -> serde_json::Value { + let requests = server.received_requests().await.unwrap(); + let put = requests + .iter() + .find(|r| r.method.as_str() == "PUT") + .expect("no PUT"); + let mut archive = + zip::ZipArchive::new(std::io::Cursor::new(put.body.clone())).expect("not a zip"); + assert_eq!(archive.len(), 1, "one entry: just the map"); + let mut entry = archive.by_index(0).unwrap(); + let mut body = String::new(); + std::io::Read::read_to_string(&mut entry, &mut body).unwrap(); + serde_json::from_str(&body).expect("uploaded map is not JSON") + } + + /// `sourcesContent` embeds the ORIGINAL SOURCE in the map, which is how symbolication shows + /// source lines — and also means the upload ships the customer's code to the backend. Teams who + /// would rather it did not can strip it; they keep file/line/column symbolication and lose the + /// source snippet. + #[tokio::test] + async fn strip_sources_content_removes_the_source_from_what_is_uploaded() { + let tmp = tempfile::tempdir().unwrap(); + let original = br#"{"version":3,"file":"app.js","debug_id":"11111111-1111-1111-1111-111111111111","sources":["src/app.ts"],"sourcesContent":["const secret = 'hunter2';\n"],"names":[],"mappings":"AAAA"}"#; + write(tmp.path(), "app.js.map", original); + let server = collector(&[]).await; + + run_sourcemap_upload( + &[tmp.path().to_path_buf()], + &server.uri(), + "TKN", + "1.0", + "1", + None, + Strategy::Zstd(11), + false, + None, + false, + false, + true, + ) + .await + .unwrap(); + + let uploaded = uploaded_map_json(&server).await; + assert!( + uploaded.get("sourcesContent").is_none(), + "sourcesContent must not leave the machine: {uploaded}" + ); + // Everything symbolication needs is still there. + assert_eq!(uploaded["mappings"], "AAAA"); + assert_eq!(uploaded["sources"], serde_json::json!(["src/app.ts"])); + assert_eq!(uploaded["debug_id"], "11111111-1111-1111-1111-111111111111"); + + // The user's own file is untouched — it is their build output, and their local debugging. + let on_disk = std::fs::read(tmp.path().join("app.js.map")).unwrap(); + assert_eq!(on_disk, original, "the map on disk must not be rewritten"); + } + + /// The declared `hash` must describe what was UPLOADED, not the file we read. + #[tokio::test] + async fn stripping_updates_the_declared_hash() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "app.js.map", + br#"{"version":3,"debug_id":"22222222-2222-2222-2222-222222222222","sources":["a.ts"],"sourcesContent":["x"],"names":[],"mappings":"AAAA"}"#, + ); + let server = collector(&[]).await; + + run_sourcemap_upload( + &[tmp.path().to_path_buf()], + &server.uri(), + "TKN", + "1.0", + "1", + None, + Strategy::Zstd(11), + false, + None, + false, + false, + true, + ) + .await + .unwrap(); + + let requests = server.received_requests().await.unwrap(); + let post: serde_json::Value = serde_json::from_slice( + &requests + .iter() + .find(|r| r.method.as_str() == "POST") + .unwrap() + .body, + ) + .unwrap(); + let uploaded = uploaded_map_json(&server).await; + let expected = { + use sha1::Digest; + let digest: [u8; 20] = + sha1::Sha1::digest(serde_json::to_vec(&uploaded).unwrap()).into(); + hex::encode(digest) + }; + assert_eq!(post["hash"].as_str().unwrap(), expected); + } + + /// Without the flag, the source rides along exactly as before — that is what makes the + /// symbolicated frame show a source line. + #[tokio::test] + async fn sources_content_is_kept_by_default() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "app.js.map", + br#"{"version":3,"debug_id":"33333333-3333-3333-3333-333333333333","sources":["a.ts"],"sourcesContent":["const x = 1;"],"names":[],"mappings":"AAAA"}"#, + ); + let server = collector(&[]).await; + + upload_dir(tmp.path(), &server.uri()).await.unwrap(); + + assert_eq!( + uploaded_map_json(&server).await["sourcesContent"], + serde_json::json!(["const x = 1;"]) + ); + } + + /// A map that carries no `sourcesContent` — or is not the JSON we expect — uploads unchanged + /// rather than failing: the flag is a privacy preference, not a validator. + #[tokio::test] + async fn stripping_a_map_with_no_sources_content_changes_nothing() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "app.js.map", + br#"{"version":3,"debug_id":"44444444-4444-4444-4444-444444444444","sources":["a.ts"],"names":[],"mappings":"AAAA"}"#, + ); + let server = collector(&[]).await; + + run_sourcemap_upload( + &[tmp.path().to_path_buf()], + &server.uri(), + "TKN", + "1.0", + "1", + None, + Strategy::Zstd(11), + false, + None, + false, + false, + true, ) .await .unwrap(); + + // BYTE-identical: with nothing to strip there is no reason to re-serialize the map, and + // doing so would change the bytes (and the hash) of a file we were asked to leave alone. + assert_eq!( + uploaded_map_bytes(&server).await, + std::fs::read(tmp.path().join("app.js.map")).unwrap() + ); } /// A build step that legitimately produces no maps — a monorepo package built without them, a @@ -3135,6 +3402,7 @@ mod sourcemap_upload_tests { None, false, true, + false, ) .await .unwrap_err(); diff --git a/tests/debug_files_flags.rs b/tests/debug_files_flags.rs index c7f65e3..5f119e2 100644 --- a/tests/debug_files_flags.rs +++ b/tests/debug_files_flags.rs @@ -171,6 +171,17 @@ fn sourcemap_only_flags_are_rejected_for_other_types() { "--concurrency is only valid for --type sourcemaps — no other type uploads a \ batch of independent files", )); + + upload( + kind, + &["--strip-sources-content", tmp.path().to_str().unwrap()], + ) + .assert() + .code(CONFIG_INVALID) + .stderr(contains( + "--strip-sources-content is only valid for --type sourcemaps — no other symbol format \ + embeds source", + )); } } From 9dc7c140115f72d24d1cdd6d9ded3374bb007b85 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Fri, 18 Sep 2026 20:22:51 +0500 Subject: [PATCH 2/2] fix(debug-files): --strip-sources-content missed an indexed map's source entirely MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A review of the whole 0.7.11 delta found the flag's promise failing silently on the one map shape it could not see. An INDEXED map (spec §Index-Map, `{"version":3,"sections":[{"offset":…,"map":{…}}]}`) keeps its source inside each section, not at the top level — so `map.remove("sourcesContent")` found nothing, the map uploaded verbatim WITH the source, and the run reported success without even a log line. A team turning this on precisely so their code never leaves the build machine got the opposite, quietly. `sourcesContent` is now removed wherever a map can carry it: the top level and every `sections[].map`, recursively (sections nest). Verified against a capturing mock — the PUT body no longer contains the source, while sections, offsets, mappings and the debug-id survive. Also from the same review: the strip's JSON round-trip moved to `spawn_blocking`. Parsing and re-serializing a multi-megabyte map costs more than the zstd that already moved there, and every upload future is polled by one task. Docs: README's synopsis was missing `--strip-sources-content`, its "both flags" line had grown to three, and neither document mentioned indexed maps. Three mutants caught (sections branch skipped, recursion dropped, top-level removal dropped). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- CHANGELOG.md | 4 +- README.md | 12 +++--- src/cli/debug_files.rs | 88 ++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 94 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3457639..7384498 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,9 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). The map on disk is never modified — only the copy that is uploaded — and the declared `hash` describes the stripped bytes rather than the file that was read. A map that carries no `sourcesContent` (or is not the JSON object we expect) is uploaded byte-for-byte unchanged: this - is a privacy preference, not a validator. Rejected (exit 20) for every other `--type`, since no + is a privacy preference, not a validator. An INDEXED map (spec §Index-Map) keeps its source inside + `sections[].map`, and those are stripped too — removing only the top-level key would have shipped + the source while reporting success. Rejected (exit 20) for every other `--type`, since no other symbol format embeds source. Measured on a real esbuild bundle: 253 → 181 bytes uploaded, local map untouched. diff --git a/README.md b/README.md index 4771b69..0efa85b 100644 --- a/README.md +++ b/README.md @@ -222,8 +222,9 @@ rather than fatal there (`unkeyed` in the completion log) — the whole flow can freshly built directory, where `sourcemaps inject --dry-run` has deliberately written nothing yet. A REAL run still refuses such a map (exit 11): uploading it would register a symbol nothing can find. -`--strip-sources-content` uploads each map WITHOUT its `sourcesContent`, for teams who would rather -their source did not leave the build machine. Symbolication still resolves file, line and column; +`--strip-sources-content` uploads each map WITHOUT its `sourcesContent` — including the copies an +indexed map keeps inside `sections[].map` — for teams who would rather their source did not leave +the build machine. Symbolication still resolves file, line and column; what is lost is the source snippet shown beside a crash frame. The map on disk is never modified — only the copy that is uploaded — and the declared `hash` describes the stripped bytes. A map that carries no `sourcesContent` is uploaded byte-for-byte unchanged. @@ -235,8 +236,9 @@ A path that does not exist is an error regardless (`path does not exist:

`, exit 10) — including when other paths do hold maps — so a typo or a build that never ran cannot half-upload a build's symbols. -Both flags apply to `--type sourcemaps` only, and are rejected (exit 20) for any -other type rather than accepted and ignored. +`--concurrency`, `--allow-empty` and `--strip-sources-content` apply to +`--type sourcemaps` only, and are rejected (exit 20) for any other type rather +than accepted and ignored. `--exclude ` (repeatable) keeps `inject` out of part of a build output — `--exclude '**/node_modules/**'` leaves vendored third-party code inside a server bundle untouched, `--exclude @@ -254,7 +256,7 @@ unsymbolicated instead. Measured on a stock `next build` with browser source map ``` bugsee-cli sourcemaps inject ... [--exclude ]... [--dry-run] bugsee-cli debug-files upload --type sourcemaps ... --version --build \ - [--concurrency N] [--allow-empty] + [--concurrency N] [--allow-empty] [--strip-sources-content] ``` ### `xcode upload-dsyms` diff --git a/src/cli/debug_files.rs b/src/cli/debug_files.rs index cb592fd..9c6dd15 100644 --- a/src/cli/debug_files.rs +++ b/src/cli/debug_files.rs @@ -1443,7 +1443,22 @@ async fn upload_one_sourcemap( // `--strip-sources-content`: pack a COPY without the embedded original source. The file on disk // is the caller's build output — rewriting it would take away their own debugging, and the // bundler plugins delete these maps after a successful upload anyway. - let stripped = stripped_copy_without_sources(map_path, tmpdir.path(), ctx)?; + // + // On the blocking pool for the same reason the packing is: parsing and re-serializing a + // multi-megabyte map costs more than the zstd does, and every upload future is polled by ONE + // task. + let stripped = { + let (owned_path, tmp_owned, strip) = ( + map_path.to_path_buf(), + tmpdir.path().to_path_buf(), + ctx.strip_sources_content, + ); + tokio::task::spawn_blocking(move || { + stripped_copy_without_sources(&owned_path, &tmp_owned, strip) + }) + .await + .map_err(|e| anyhow::anyhow!("stripping {} failed to run: {e}", map_path.display()))?? + }; let (map_path, identity) = match stripped.as_ref() { Some((path, id)) => (path.as_path(), id), None => (map_path, identity), @@ -1769,6 +1784,23 @@ fn is_stylesheet_or_declaration_map(path: &Path) -> bool { .any(|suffix| name.ends_with(suffix)) } +/// Remove every `sourcesContent` a source map can carry; `true` when one was there. +/// +/// Not just the top-level key: an INDEXED map (spec §Index-Map) keeps its source inside +/// `sections[i].map`, and those sections nest. Stripping only the top level left the source in the +/// upload while reporting success — the flag's whole promise, failing silently. +fn remove_sources_content(map: &mut serde_json::Map) -> bool { + let mut removed = map.remove("sourcesContent").is_some(); + if let Some(serde_json::Value::Array(sections)) = map.get_mut("sections") { + for section in sections { + if let Some(serde_json::Value::Object(inner)) = section.get_mut("map") { + removed |= remove_sources_content(inner); + } + } + } + removed +} + /// A copy of `map_path` with `sourcesContent` removed, plus its recomputed identity — or `None` /// when nothing needs stripping (flag off, no `sourcesContent`, or the map is not the JSON object we /// expect, which is a privacy preference's business to ignore rather than to fail over). @@ -1779,16 +1811,16 @@ fn is_stylesheet_or_declaration_map(path: &Path) -> bool { fn stripped_copy_without_sources( map_path: &Path, tmpdir: &Path, - ctx: SourcemapUploadCtx<'_>, + strip: bool, ) -> anyhow::Result> { - if !ctx.strip_sources_content { + if !strip { return Ok(None); } let bytes = std::fs::read(map_path)?; let Ok(serde_json::Value::Object(mut map)) = serde_json::from_slice(&bytes) else { return Ok(None); }; - if map.remove("sourcesContent").is_none() { + if !remove_sources_content(&mut map) { return Ok(None); } let stripped = serde_json::to_vec(&map)?; @@ -3254,6 +3286,54 @@ mod sourcemap_upload_tests { ); } + /// An INDEXED map (spec §Index-Map: `{"version":3,"sections":[{"offset":…,"map":{…}}]}`) keeps + /// its source inside each section, not at the top level. Removing only the top-level key left + /// the source in the upload while reporting success — the flag's entire promise, failing + /// silently. Webpack emits these for `devtool: 'source-map'` with certain plugins, and the + /// worker symbolicates them. + #[tokio::test] + async fn strip_reaches_inside_an_indexed_map() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "app.js.map", + br#"{"version":3,"debug_id":"55555555-5555-5555-5555-555555555555","sections":[ + {"offset":{"line":0,"column":0},"map":{"version":3,"sources":["a.ts"],"sourcesContent":["const password = 'hunter2';"],"names":[],"mappings":"AAAA"}}, + {"offset":{"line":9,"column":0},"map":{"version":3,"sources":["b.ts"],"sourcesContent":["const other = 2;"],"names":[],"mappings":"AACA"}} + ]}"#, + ); + let server = collector(&[]).await; + + run_sourcemap_upload( + &[tmp.path().to_path_buf()], + &server.uri(), + "TKN", + "1.0", + "1", + None, + Strategy::Zstd(11), + false, + None, + false, + false, + true, + ) + .await + .unwrap(); + + let body = String::from_utf8(uploaded_map_bytes(&server).await).unwrap(); + assert!( + !body.contains("hunter2") && !body.contains("sourcesContent"), + "source must not leave the machine in a section either: {body}" + ); + // …and the map is still usable: sections, offsets, mappings and the key survive. + let uploaded: serde_json::Value = serde_json::from_str(&body).unwrap(); + assert_eq!(uploaded["sections"].as_array().unwrap().len(), 2); + assert_eq!(uploaded["sections"][1]["offset"]["line"], 9); + assert_eq!(uploaded["sections"][0]["map"]["mappings"], "AAAA"); + assert_eq!(uploaded["debug_id"], "55555555-5555-5555-5555-555555555555"); + } + /// A map that carries no `sourcesContent` — or is not the JSON we expect — uploads unchanged /// rather than failing: the flag is a privacy preference, not a validator. #[tokio::test]