From 051ee20b4a39172fc90689d2deda0ea305949504 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Fri, 18 Sep 2026 19:28:45 +0500 Subject: [PATCH 1/9] 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 7020e87..332f8a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,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 6569958..6d9ef3b 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 7654e75736129ef5fa97380ab246ecf5a09b87b1 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Fri, 18 Sep 2026 20:22:51 +0500 Subject: [PATCH 2/9] 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 332f8a8..3ca100b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,7 +61,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 6d9ef3b..5cd9877 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 @@ -289,7 +291,7 @@ still break, so keep `--allow-sri` off and check a deploy before trusting it. ``` bugsee-cli sourcemaps inject ... [--exclude ]... [--allow-sri] [--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] From 9e0026e44ea9495bed7cf9d5caab4fab8f2c2265 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Fri, 18 Sep 2026 22:19:02 +0500 Subject: [PATCH 3/9] fix(debug-files): type the strip's I/O errors, and stop the e2e check reading a zstd zip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two failures on #50. **The review's finding.** `stripped_copy_without_sources` read the map and wrote the copy through a bare `?` into `anyhow`, so an I/O failure there classified as exit 1 ("unexpected — you may fall back") while `sourcemap::identify` classifies one on the SAME file, moments earlier, as exit 10 ("input not found — do not"). Both now go through `Error::Io`, and the JSON re-serialization failure through `input_invalid`. The test drives the function directly, because `run_sourcemap_upload` reads the map through `identify` first — which is exactly why this could sit here unnoticed: an end-to-end test cannot reach the strip's own read. It covers the map vanishing (the TOCTOU window), an unreadable map, and an unwritable destination; both untyped-error mutants are caught. **The CI failure, which was mine.** The e2e strip check reads the uploaded zip with Python's `zipfile`, which cannot decompress zstd (method 93) before Python 3.14 — the macOS runner has 3.14, ubuntu and windows do not, so the flow passed locally and failed on three runners. That flow now uploads with `--no-zstd`; it is about `sourcesContent`, not compression, and `sourcemaps_upload` already covers the zstd path. The "could not verify" branch is also no longer a `[warn]`: a check that cannot run is a check that failed, and it reported as one. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- scripts/e2e_flows.py | 10 ++++-- src/cli/debug_files.rs | 72 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/scripts/e2e_flows.py b/scripts/e2e_flows.py index 429c26a..4c8b441 100644 --- a/scripts/e2e_flows.py +++ b/scripts/e2e_flows.py @@ -459,9 +459,14 @@ def main(): "sources": ["app.ts"], "sourcesContent": ["const secret = 1;"], "names": [], "mappings": "AAAA"}, f) before = open(sc_map, "rb").read() + # `--no-zstd` for THIS flow only: the assertion below reads the uploaded zip with Python's + # `zipfile`, which cannot decompress zstd (method 93) before Python 3.14 — the macOS runner has + # it, ubuntu and windows do not. The flow is about `sourcesContent`, not about compression, and + # `sourcemaps_upload` already covers the zstd path. results["sourcemaps_strip_sources_content"] = run( binpath, "sourcemaps_strip", - ["debug-files", "upload", "--type", "sourcemaps", "--strip-sources-content", sc_map] + v) + ["debug-files", "upload", "--type", "sourcemaps", "--strip-sources-content", "--no-zstd", + sc_map] + v) try: import zipfile as _zf uploaded = None @@ -477,7 +482,8 @@ def main(): 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) + # Not swallowed: a check that cannot run is a check that failed. + print(" [FAIL] 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"), diff --git a/src/cli/debug_files.rs b/src/cli/debug_files.rs index 9c6dd15..d471639 100644 --- a/src/cli/debug_files.rs +++ b/src/cli/debug_files.rs @@ -1816,20 +1816,28 @@ fn stripped_copy_without_sources( if !strip { return Ok(None); } - let bytes = std::fs::read(map_path)?; + // Typed, not a bare `?` into anyhow: an I/O failure on the map must classify the way + // `sourcemap::identify` classifies one on the SAME file moments earlier — exit 10, which + // integrators are documented not to fall back on — rather than exit 1 ("unexpected"). + let bytes = std::fs::read(map_path).map_err(crate::error::Error::Io)?; let Ok(serde_json::Value::Object(mut map)) = serde_json::from_slice(&bytes) else { return Ok(None); }; if !remove_sources_content(&mut map) { return Ok(None); } - let stripped = serde_json::to_vec(&map)?; + let stripped = serde_json::to_vec(&map).map_err(|e| { + input_invalid(format!( + "{}: could not re-serialize the map without its sources: {e}", + map_path.display() + )) + })?; 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)?; + std::fs::write(&out, &stripped).map_err(crate::error::Error::Io)?; tracing::info!( path = %map_path.display(), before_bytes = bytes.len(), @@ -3334,6 +3342,64 @@ mod sourcemap_upload_tests { assert_eq!(uploaded["debug_id"], "55555555-5555-5555-5555-555555555555"); } + /// The exit code is an integrator contract: 10 means "input not found, do not fall back", 1 + /// means "unexpected, you may fall back". Reading the map through a bare `?` into `anyhow` + /// classified an I/O failure as 1, unlike `sourcemap::identify` reading the very same file + /// moments earlier. Driven at the function, because `run_sourcemap_upload` reads the map through + /// `identify` FIRST — which is exactly why the misclassification could sit here unnoticed. + #[test] + fn a_strip_that_cannot_read_or_write_is_input_not_found() { + // As root, chmod 000 does not stop a read; there is nothing to assert. + if std::env::var_os("USER").is_some_and(|u| u == "root") { + return; + } + use std::os::unix::fs::PermissionsExt; + let tmp = tempfile::tempdir().unwrap(); + + // 1. The map vanished between `identify` and here (the TOCTOU window). + let missing = tmp.path().join("gone.js.map"); + let err = stripped_copy_without_sources(&missing, tmp.path(), true).unwrap_err(); + assert_eq!( + crate::error::classify(&err), + crate::exit_code::ExitCode::InputNotFound, + "{err:#}" + ); + + // 2. …or is unreadable. + let locked = tmp.path().join("locked.js.map"); + std::fs::write( + &locked, + br#"{"version":3,"sourcesContent":["x"],"mappings":""}"#, + ) + .unwrap(); + std::fs::set_permissions(&locked, std::fs::Permissions::from_mode(0o000)).unwrap(); + let err = stripped_copy_without_sources(&locked, tmp.path(), true).unwrap_err(); + std::fs::set_permissions(&locked, std::fs::Permissions::from_mode(0o644)).unwrap(); + assert_eq!( + crate::error::classify(&err), + crate::exit_code::ExitCode::InputNotFound, + "{err:#}" + ); + + // 3. …or the copy cannot be written. + let readable = tmp.path().join("ok.js.map"); + std::fs::write( + &readable, + br#"{"version":3,"sourcesContent":["x"],"mappings":""}"#, + ) + .unwrap(); + let out_dir = tmp.path().join("readonly"); + std::fs::create_dir(&out_dir).unwrap(); + std::fs::set_permissions(&out_dir, std::fs::Permissions::from_mode(0o500)).unwrap(); + let err = stripped_copy_without_sources(&readable, &out_dir, true).unwrap_err(); + std::fs::set_permissions(&out_dir, std::fs::Permissions::from_mode(0o700)).unwrap(); + assert_eq!( + crate::error::classify(&err), + crate::exit_code::ExitCode::InputNotFound, + "{err:#}" + ); + } + /// 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] From e6d9eff540b96390f59274c589d078e8b5f9a7a0 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Fri, 18 Sep 2026 22:27:07 +0500 Subject: [PATCH 4/9] fix(test,ci): gate the permission test to unix, and compile test code for Windows in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review caught a test that would not compile on Windows: `PermissionsExt` is Unix-only and my new `a_strip_that_cannot_read_or_write_is_input_not_found` used it with no `#[cfg(unix)]`, unlike `an_unreadable_map_fails_before_the_bundle_is_modified` right next door. Gated to match. No job could have caught it, which is the more interesting half. The `test` matrix is ubuntu + macOS, and the Windows job runs `cargo build --release` — the binary only, so `#[cfg(test)]` code is never compiled for that target. All 13 checks passed on a tree that broke `cargo test` for every Windows developer. That job now builds `--all-targets`, so test code compiles there too. Compiling is not running: the matrix above still owns behaviour, and this stays a build job. Running the suite on Windows would be the fuller fix — 535 tests that have never executed there — but that is its own change, with its own fallout, and not something to fold into a release. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- .github/workflows/ci.yml | 8 +++++++- src/cli/debug_files.rs | 4 ++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6bed328..8ca84f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,7 +64,13 @@ jobs: - uses: Swatinem/rust-cache@v2 # Build only: the `test` job above covers behaviour, and this job exists # to prove the target still compiles — ring/zstd-sys/zip/flate2 included. - - run: cargo build --release --target aarch64-pc-windows-msvc + # + # `--all-targets` so the TEST code compiles here too. Without it a test using a + # Unix-only API (`std::os::unix::fs::PermissionsExt`) without a `#[cfg(unix)]` guard + # passes every job and breaks anyone running `cargo test` on Windows — which happened, + # and which no job could have caught: the test matrix is ubuntu + macOS, and this job + # built only the binary. Compiling is not running; the matrix above still owns behaviour. + - run: cargo build --release --all-targets --target aarch64-pc-windows-msvc # One command per step ON PURPOSE. The default shell here is pwsh, where a # NATIVE command's non-zero exit does not abort the script — GitHub only # inspects $LASTEXITCODE after the whole thing runs. Both commands in one diff --git a/src/cli/debug_files.rs b/src/cli/debug_files.rs index d471639..42b6880 100644 --- a/src/cli/debug_files.rs +++ b/src/cli/debug_files.rs @@ -3347,6 +3347,10 @@ mod sourcemap_upload_tests { /// classified an I/O failure as 1, unlike `sourcemap::identify` reading the very same file /// moments earlier. Driven at the function, because `run_sourcemap_upload` reads the map through /// `identify` FIRST — which is exactly why the misclassification could sit here unnoticed. + /// + /// Unix-only, like `an_unreadable_map_fails_before_the_bundle_is_modified`: the permission + /// technique has no Windows equivalent, and `PermissionsExt` does not exist there. + #[cfg(unix)] #[test] fn a_strip_that_cannot_read_or_write_is_input_not_found() { // As root, chmod 000 does not stop a read; there is nothing to assert. From 94bc8ad9b42d48de67a047cad8bedb5c19762e6a Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Fri, 18 Sep 2026 22:27:21 +0500 Subject: [PATCH 5/9] ci: run the test suite on Windows, not just build it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suite has never executed on Windows: the matrix was ubuntu + macOS and the Windows leg built the binary only. This ships there, so it should be tested there. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- .github/workflows/ci.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8ca84f6..627e12e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -132,7 +132,11 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-latest] + # windows-latest runs the suite on the platform this ships to. It was absent for a long + # time, and the Windows leg was a build-only job, so 500+ tests had never executed there — + # a test using a Unix-only API passed every check while breaking `cargo test` for every + # Windows developer. + os: [ubuntu-latest, macos-latest, windows-latest] steps: - uses: actions/checkout@v7 - uses: dtolnay/rust-toolchain@stable From 97e5eccfbe874654ed8ad42242eb9cecea297a2a Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Fri, 18 Sep 2026 22:36:03 +0500 Subject: [PATCH 6/9] ci: run the full suite on BOTH Windows architectures, and fix what that found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit x64 and the native ARM64 runner now run `cargo test --all-targets`, not just a release build. The release ships both, and they differ in more than instruction set — `ring`'s ARM64 Windows assembly is why the release leg is pinned to a native runner rather than cross-compiled. The first run found a real break, and it was not from this PR: `tests/env_robustness.rs` defines a `cli()` helper used only by its `#[cfg(unix)]` tests, so on Windows it is dead code — and `RUSTFLAGS: -D warnings` makes that a hard error. `cargo test` had been failing on Windows for anyone who ran it, while every CI job stayed green, because no job compiled test code for the platform. Gated the helper to match its users. The ARM64 job drops the `--all-targets` I added a commit ago: with a real ARM test job it is back to what it says on the tin, proof that the release profile still links for the target it ships. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- .github/workflows/ci.yml | 15 +++++++-------- tests/env_robustness.rs | 4 ++++ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 627e12e..d85c1f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,13 +64,9 @@ jobs: - uses: Swatinem/rust-cache@v2 # Build only: the `test` job above covers behaviour, and this job exists # to prove the target still compiles — ring/zstd-sys/zip/flate2 included. - # - # `--all-targets` so the TEST code compiles here too. Without it a test using a - # Unix-only API (`std::os::unix::fs::PermissionsExt`) without a `#[cfg(unix)]` guard - # passes every job and breaks anyone running `cargo test` on Windows — which happened, - # and which no job could have caught: the test matrix is ubuntu + macOS, and this job - # built only the binary. Compiling is not running; the matrix above still owns behaviour. - - run: cargo build --release --all-targets --target aarch64-pc-windows-msvc + # The `test` matrix now RUNS the suite on windows-11-arm, so this job is back to what it + # says: proof that the release profile still links for the target it ships. + - run: cargo build --release --target aarch64-pc-windows-msvc # One command per step ON PURPOSE. The default shell here is pwsh, where a # NATIVE command's non-zero exit does not abort the script — GitHub only # inspects $LASTEXITCODE after the whole thing runs. Both commands in one @@ -136,7 +132,10 @@ jobs: # time, and the Windows leg was a build-only job, so 500+ tests had never executed there — # a test using a Unix-only API passed every check while breaking `cargo test` for every # Windows developer. - os: [ubuntu-latest, macos-latest, windows-latest] + # BOTH Windows architectures run the whole suite: x64 and the native ARM64 runner. The + # release ships both, and they differ in more than instruction set — `ring`'s ARM64 Windows + # assembly is why the release leg is pinned to a native runner rather than cross-compiled. + os: [ubuntu-latest, macos-latest, windows-latest, windows-11-arm] steps: - uses: actions/checkout@v7 - uses: dtolnay/rust-toolchain@stable diff --git a/tests/env_robustness.rs b/tests/env_robustness.rs index f1d69e3..4b5f7f1 100644 --- a/tests/env_robustness.rs +++ b/tests/env_robustness.rs @@ -6,6 +6,10 @@ use assert_cmd::Command; +// Only the Unix-gated tests below use this today (a non-UTF-8 env var is not constructible on +// Windows, where `OsString` is WTF-16), and `-D warnings` makes an unused helper a hard error — +// which is how `cargo test` was broken on Windows while every CI job stayed green. +#[cfg(unix)] fn cli() -> Command { let mut c = Command::cargo_bin("bugsee-cli").expect("compiled bugsee-cli binary"); c.env_clear(); From a70b6984a82e160da420421e4c3eb7608313b6a5 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Fri, 18 Sep 2026 22:42:12 +0500 Subject: [PATCH 7/9] test(env): gate the import too, not just the helper it feeds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gating `cli()` to unix left `use assert_cmd::Command;` unused there, which `-D warnings` also makes a hard error — the next layer of the same break. Both are gated now, with the reason recorded: the single test in this file needs a non-UTF-8 environment variable, which is not constructible on Windows. Swept the rest of the test surface for the same class: every other `os::unix` / `PermissionsExt` use is already inside a gated item or block (update_e2e.rs is Windows-aware by construction), and the two remaining greps are `is_symlink()`, which is cross-platform. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- tests/env_robustness.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/env_robustness.rs b/tests/env_robustness.rs index 4b5f7f1..b20e85f 100644 --- a/tests/env_robustness.rs +++ b/tests/env_robustness.rs @@ -4,11 +4,13 @@ //! may set a variable this CLI has never heard of. Reading the environment is //! not allowed to abort the run over one of them. +// Unix-only, both of them: the single test in this file needs a non-UTF-8 environment variable, +// which is not constructible on Windows (`OsString` is WTF-16 there). `-D warnings` makes an unused +// helper OR an unused import a hard error, which is how `cargo test` was failing on Windows while +// every CI job stayed green — no job compiled test code for that platform. +#[cfg(unix)] use assert_cmd::Command; -// Only the Unix-gated tests below use this today (a non-UTF-8 env var is not constructible on -// Windows, where `OsString` is WTF-16), and `-D warnings` makes an unused helper a hard error — -// which is how `cargo test` was broken on Windows while every CI job stayed green. #[cfg(unix)] fn cli() -> Command { let mut c = Command::cargo_bin("bugsee-cli").expect("compiled bugsee-cli binary"); From dea6d464c599cb4221e14b3df876b4f699c66a8e Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Fri, 18 Sep 2026 22:52:23 +0500 Subject: [PATCH 8/9] =?UTF-8?q?test:=20env=5Fclear()=20breaks=20the=20netw?= =?UTF-8?q?ork=20on=20Windows=20=E2=80=94=20put=20SystemRoot=20back?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Running the suite on Windows for the first time found a real defect in the tests, not the product. 519 unit tests pass there; `tests/elf_upload.rs` did not: WARN symbol metadata POST: transport error; backing off attempt=1 error=error sending request error: upload failed: symbol metadata POST: error sending request (exit 31, after 3 retries) The tests clear the child's environment so an ambient `BUGSEE_*` or CI variable cannot decide what the binary does. On Unix that is all `env_clear()` means. On Windows it also removes `SystemRoot`, and WinSock cannot initialise without it — so every request from the child fails, with nothing in the message to say why. `tests/update_e2e.rs` makes the same kind of call and passes precisely because it does not clear the environment. All three network-driving test files now share one `common::cli()` that clears the environment and then puts the OS's own variables back on Windows (`SystemRoot`, `windir`, `SystemDrive`, `TEMP`, `TMP`, `USERPROFILE`). Shared rather than repeated three times, so the next test written this way cannot repeat it; `xcode_post_action.rs` keeps its local `cli()` name as a thin wrapper, since its doc comment explains why that file in particular needs a hermetic environment. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- tests/common/mod.rs | 30 ++++++++++ tests/elf_upload.rs | 48 +++++++-------- tests/metadata_format.rs | 120 ++++++++++++++++++------------------- tests/xcode_post_action.rs | 6 +- 4 files changed, 116 insertions(+), 88 deletions(-) create mode 100644 tests/common/mod.rs diff --git a/tests/common/mod.rs b/tests/common/mod.rs new file mode 100644 index 0000000..70123d7 --- /dev/null +++ b/tests/common/mod.rs @@ -0,0 +1,30 @@ +//! Shared helpers for the integration tests that drive the COMPILED binary. + +use assert_cmd::Command; + +/// `bugsee-cli` with a cleared environment — plus, on Windows, the OS variables the child cannot +/// work without. +/// +/// The tests clear the environment so an ambient `BUGSEE_*` or CI variable cannot decide what the +/// binary does. On Unix that is all it means. On Windows, `env_clear()` also removes `SystemRoot`, +/// and WinSock cannot initialise without it: every request from the child then fails with +/// "error sending request", three retries and exit 31, with nothing to say why. Found the first time +/// the suite ran on Windows at all — `cargo test` had only ever run on ubuntu and macOS. +pub fn cli() -> Command { + let mut command = Command::cargo_bin("bugsee-cli").expect("compiled bugsee-cli binary"); + command.env_clear(); + #[cfg(windows)] + for key in [ + "SystemRoot", + "windir", + "SystemDrive", + "TEMP", + "TMP", + "USERPROFILE", + ] { + if let Some(value) = std::env::var_os(key) { + command.env(key, value); + } + } + command +} diff --git a/tests/elf_upload.rs b/tests/elf_upload.rs index 0cf65bc..b97723f 100644 --- a/tests/elf_upload.rs +++ b/tests/elf_upload.rs @@ -11,11 +11,12 @@ use std::io::Write; use std::path::{Path, PathBuf}; -use assert_cmd::Command; use serde_json::json; use wiremock::matchers::{body_string_contains, method, path as wm_path}; use wiremock::{Mock, MockServer, ResponseTemplate}; +mod common; + /// `file(1)` on the fixture: `BuildID[md5/uuid]=bca64abfec40dbb631bb8f1c37414472`. const FIXTURE_BUILD_ID: &str = "bca64abfec40dbb631bb8f1c37414472"; @@ -65,29 +66,28 @@ async fn elf_upload_registers_each_so_by_its_real_build_id() { let zip = zip_path.to_string_lossy().into_owned(); // assert_cmd is blocking; keep it off the async runtime so the mock serves. tokio::task::spawn_blocking(move || { - let mut c = Command::cargo_bin("bugsee-cli").unwrap(); - c.env_clear() - .args([ - "--endpoint", - &endpoint, - "--app-token", - "TKN", - "debug-files", - "upload", - "--type", - "elf", - "--version", - "1.0", - "--build", - "1", - // The build UUID is still accepted but must NOT become the - // symbol identity — the real build-id above proves that. - "--uuid", - "00000000-0000-0000-0000-000000000000", - &zip, - ]) - .assert() - .success(); + let mut c = common::cli(); + c.args([ + "--endpoint", + &endpoint, + "--app-token", + "TKN", + "debug-files", + "upload", + "--type", + "elf", + "--version", + "1.0", + "--build", + "1", + // The build UUID is still accepted but must NOT become the + // symbol identity — the real build-id above proves that. + "--uuid", + "00000000-0000-0000-0000-000000000000", + &zip, + ]) + .assert() + .success(); }) .await .unwrap(); diff --git a/tests/metadata_format.rs b/tests/metadata_format.rs index d5691a3..4c3768f 100644 --- a/tests/metadata_format.rs +++ b/tests/metadata_format.rs @@ -6,11 +6,12 @@ use std::path::PathBuf; -use assert_cmd::Command; use serde_json::json; use wiremock::matchers::{body_string_contains, method, path as wm_path}; use wiremock::{Mock, MockServer, ResponseTemplate}; +mod common; + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn proguard_upload_sends_format_mapping() { let server = MockServer::start().await; @@ -37,25 +38,24 @@ async fn proguard_upload_sends_format_mapping() { let endpoint = server.uri(); let path = mapping.to_string_lossy().into_owned(); tokio::task::spawn_blocking(move || { - let mut c = Command::cargo_bin("bugsee-cli").unwrap(); - c.env_clear() - .args([ - "--endpoint", - &endpoint, - "--app-token", - "TKN", - "debug-files", - "upload", - "--type", - "proguard", - "--version", - "1.0", - "--build", - "1", - &path, - ]) - .assert() - .success(); + let mut c = common::cli(); + c.args([ + "--endpoint", + &endpoint, + "--app-token", + "TKN", + "debug-files", + "upload", + "--type", + "proguard", + "--version", + "1.0", + "--build", + "1", + &path, + ]) + .assert() + .success(); }) .await .unwrap(); @@ -88,25 +88,24 @@ async fn sourcemap_upload_sends_format_sourcemap() { let endpoint = server.uri(); let path = map_path.to_string_lossy().into_owned(); tokio::task::spawn_blocking(move || { - let mut c = Command::cargo_bin("bugsee-cli").unwrap(); - c.env_clear() - .args([ - "--endpoint", - &endpoint, - "--app-token", - "TKN", - "debug-files", - "upload", - "--type", - "sourcemaps", - "--version", - "1.0", - "--build", - "1", - &path, - ]) - .assert() - .success(); + let mut c = common::cli(); + c.args([ + "--endpoint", + &endpoint, + "--app-token", + "TKN", + "debug-files", + "upload", + "--type", + "sourcemaps", + "--version", + "1.0", + "--build", + "1", + &path, + ]) + .assert() + .success(); }) .await .unwrap(); @@ -137,28 +136,27 @@ async fn il2cpp_linemap_upload_sends_format_il2cpp_linemap() { let endpoint = server.uri(); let path = root.to_string_lossy().into_owned(); tokio::task::spawn_blocking(move || { - let mut c = Command::cargo_bin("bugsee-cli").unwrap(); - c.env_clear() - .args([ - "--endpoint", - &endpoint, - "--app-token", - "TKN", - "debug-files", - "upload", - "--type", - "il2cpp-linemap", - "--version", - "1.0", - "--build", - "1", - "--uuid", - "deadbeefcafebabe", - "--force", - &path, - ]) - .assert() - .success(); + let mut c = common::cli(); + c.args([ + "--endpoint", + &endpoint, + "--app-token", + "TKN", + "debug-files", + "upload", + "--type", + "il2cpp-linemap", + "--version", + "1.0", + "--build", + "1", + "--uuid", + "deadbeefcafebabe", + "--force", + &path, + ]) + .assert() + .success(); }) .await .unwrap(); diff --git a/tests/xcode_post_action.rs b/tests/xcode_post_action.rs index 306d7db..4dae7c8 100644 --- a/tests/xcode_post_action.rs +++ b/tests/xcode_post_action.rs @@ -30,6 +30,8 @@ use serde_json::json; use wiremock::matchers::{method, path as wm_path}; use wiremock::{Mock, MockServer, ResponseTemplate}; +mod common; + /// Every flag added to `xcode post-action`. Used to pin the `--help` surface. const NEW_FLAGS: &[&str] = &[ "--enable-build-info", @@ -59,9 +61,7 @@ const NEW_FLAGS: &[&str] = &[ /// any in — each test then sets EXACTLY the vars it needs. The gate-out and /// token-missing code paths never shell out, so an otherwise-empty env is fine. fn cli() -> Command { - let mut c = Command::cargo_bin("bugsee-cli").expect("compiled bugsee-cli binary"); - c.env_clear(); - c + common::cli() } #[test] From cf298e39e93e36c65514d3fa3b6e93855bb57245 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Fri, 18 Sep 2026 23:04:35 +0500 Subject: [PATCH 9/9] test(update): the e2e harness only ever built the Unix release artifact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Next thing running the suite on Windows found. `src/cli/update.rs` asks for `bugsee-cli-.zip` on Windows and `.tar.xz` elsewhere (`artifact_urls`), and looks for `bugsee-cli.exe` inside — all correct. The TEST hard-coded the Unix half: it mounted `.tar.xz` and packed a binary named `bugsee-cli`, so the Windows run asked the mock for a file it had never mounted and read the 404 as a product failure. The harness now builds what this host's release would publish: a real ZIP containing `bugsee-cli-/bugsee-cli.exe` on Windows (via the `zip` crate already in the tree), the plain tar elsewhere, with the extension derived by the same split the product makes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- tests/update_e2e.rs | 83 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 67 insertions(+), 16 deletions(-) diff --git a/tests/update_e2e.rs b/tests/update_e2e.rs index 7f1265b..ec315f6 100644 --- a/tests/update_e2e.rs +++ b/tests/update_e2e.rs @@ -48,15 +48,55 @@ fn sha256_hex(bytes: &[u8]) -> String { h.finalize().iter().map(|b| format!("{b:02x}")).collect() } -/// Build a plain tar (system `tar` auto-detects "no compression" by content, so -/// the `.tar.xz` name is fine) containing `bugsee-cli-/bugsee-cli` with -/// `payload` as its bytes — the shape `install()` extracts with +/// The extension the release publishes for this host — the same split `artifact_urls` makes in +/// `src/cli/update.rs`. Windows gets a `.zip`; everything else a `.tar.xz`. Hard-coding `.tar.xz` +/// here meant the Windows run asked the mock for a file it never mounted and read a 404 as a +/// product failure. +fn artifact_ext() -> &'static str { + if cfg!(windows) { + "zip" + } else { + "tar.xz" + } +} + +/// The binary name inside the archive, which `install()` looks for after extraction. +fn archived_binary_name() -> &'static str { + if cfg!(windows) { + "bugsee-cli.exe" + } else { + "bugsee-cli" + } +} + +/// Build the archive this host's release would publish, containing +/// `bugsee-cli-/` with `payload` as its bytes — the shape `install()` extracts with /// `--strip-components=1`. -fn build_release_tar(dir: &Path, triple: &str, payload: &[u8]) -> Vec { +/// +/// A plain tar on Unix (system `tar` auto-detects "no compression" by content, so the `.tar.xz` name +/// is fine) and a real ZIP on Windows, which is what the release ships and what bsdtar extracts +/// there. +fn build_release_archive(dir: &Path, triple: &str, payload: &[u8]) -> Vec { let staging = dir.join("staging"); let wrapper = staging.join(format!("bugsee-cli-{triple}")); std::fs::create_dir_all(&wrapper).unwrap(); - std::fs::write(wrapper.join("bugsee-cli"), payload).unwrap(); + std::fs::write(wrapper.join(archived_binary_name()), payload).unwrap(); + + if cfg!(windows) { + let zip_path = dir.join("artefact.zip"); + let file = std::fs::File::create(&zip_path).unwrap(); + let mut writer = zip::ZipWriter::new(file); + writer + .start_file( + format!("bugsee-cli-{triple}/{}", archived_binary_name()), + zip::write::SimpleFileOptions::default(), + ) + .unwrap(); + std::io::Write::write_all(&mut writer, payload).unwrap(); + writer.finish().unwrap(); + return std::fs::read(&zip_path).unwrap(); + } + let tar_path = dir.join("artefact.tar.xz"); let ok = std::process::Command::new("tar") .args([ @@ -111,19 +151,23 @@ async fn update_self_replaces_to_a_newer_same_major_release() { // sentinel we can detect after the replace. let newer = format!("{}.99.0", current_major()); let sentinel = b"SENTINEL-NEW-BUGSEE-CLI-BINARY-vNEXT".to_vec(); - let tar = build_release_tar(tmp.path(), triple, &sentinel); - let sha = sha256_hex(&tar); + let artefact = build_release_archive(tmp.path(), triple, &sentinel); + let sha = sha256_hex(&artefact); let server = MockServer::start().await; mount_pointer(&server, &newer).await; Mock::given(method("GET")) - .and(wm_path(format!("/v{newer}/bugsee-cli-{triple}.tar.xz"))) - .respond_with(ResponseTemplate::new(200).set_body_bytes(tar.clone())) + .and(wm_path(format!( + "/v{newer}/bugsee-cli-{triple}.{}", + artifact_ext() + ))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(artefact.clone())) .mount(&server) .await; Mock::given(method("GET")) .and(wm_path(format!( - "/v{newer}/bugsee-cli-{triple}.tar.xz.sha256" + "/v{newer}/bugsee-cli-{triple}.{}.sha256", + artifact_ext() ))) .respond_with(ResponseTemplate::new(200).set_body_string(format!("{sha} x"))) .mount(&server) @@ -246,7 +290,10 @@ async fn update_download_failure_without_max_age_errors_and_keeps_binary() { let server = MockServer::start().await; mount_pointer(&server, &newer).await; Mock::given(method("GET")) - .and(wm_path(format!("/v{newer}/bugsee-cli-{triple}.tar.xz"))) + .and(wm_path(format!( + "/v{newer}/bugsee-cli-{triple}.{}", + artifact_ext() + ))) .respond_with(ResponseTemplate::new(500)) .mount(&server) .await; @@ -320,18 +367,22 @@ async fn update_readonly_dir_fails_safe_and_keeps_binary() { // in-place replace, because the directory is not writable. let newer = format!("{}.99.0", current_major()); let sentinel = b"SENTINEL-SHOULD-NEVER-LAND".to_vec(); - let tar = build_release_tar(tmp.path(), triple, &sentinel); - let sha = sha256_hex(&tar); + let artefact = build_release_archive(tmp.path(), triple, &sentinel); + let sha = sha256_hex(&artefact); let server = MockServer::start().await; mount_pointer(&server, &newer).await; Mock::given(method("GET")) - .and(wm_path(format!("/v{newer}/bugsee-cli-{triple}.tar.xz"))) - .respond_with(ResponseTemplate::new(200).set_body_bytes(tar)) + .and(wm_path(format!( + "/v{newer}/bugsee-cli-{triple}.{}", + artifact_ext() + ))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(artefact)) .mount(&server) .await; Mock::given(method("GET")) .and(wm_path(format!( - "/v{newer}/bugsee-cli-{triple}.tar.xz.sha256" + "/v{newer}/bugsee-cli-{triple}.{}.sha256", + artifact_ext() ))) .respond_with(ResponseTemplate::new(200).set_body_string(format!("{sha} x"))) .mount(&server)