diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6bed328..d85c1f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,6 +64,8 @@ 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. + # 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 @@ -126,7 +128,14 @@ 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. + # 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/CHANGELOG.md b/CHANGELOG.md index 7020e87..3ca100b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,21 @@ 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. 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. + ### 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..5cd9877 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,13 @@ 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` — 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. + `--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. @@ -228,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 @@ -282,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/scripts/e2e_flows.py b/scripts/e2e_flows.py index 834ec54..4c8b441 100644 --- a/scripts/e2e_flows.py +++ b/scripts/e2e_flows.py @@ -449,6 +449,42 @@ 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() + # `--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", "--no-zstd", + 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: + # 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"), "--uuid", "11111111-2222-3333-4444-555555555555"] + v) diff --git a/src/cli/debug_files.rs b/src/cli/debug_files.rs index a2eeae3..42b6880 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,30 @@ 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. + // + // 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), + }; // 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 +1551,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 +1730,7 @@ async fn run_sourcemap_upload( build, strategy, force, + strip_sources_content, }, &map_path, &identity, @@ -1739,6 +1784,78 @@ 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). +/// +/// 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, + strip: bool, +) -> anyhow::Result> { + if !strip { + return Ok(None); + } + // 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).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).map_err(crate::error::Error::Io)?; + 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 +2132,7 @@ mod sourcemap_upload_tests { None, false, false, + false, ) .await .unwrap(); @@ -2102,6 +2220,7 @@ mod sourcemap_upload_tests { None, false, false, + false, ) .await .unwrap_err(); @@ -2133,6 +2252,7 @@ mod sourcemap_upload_tests { None, false, true, + false, ) .await .unwrap(); @@ -2223,6 +2343,7 @@ mod sourcemap_upload_tests { concurrency: Option, allow_empty: bool, dry_run: bool, + strip_sources_content: bool, } async fn upload_paths( @@ -2242,6 +2363,7 @@ mod sourcemap_upload_tests { tweak.concurrency, tweak.allow_empty, tweak.dry_run, + tweak.strip_sources_content, ) .await } @@ -2378,6 +2500,7 @@ mod sourcemap_upload_tests { None, false, false, + false, ) .await .unwrap_err(); @@ -2414,6 +2537,7 @@ mod sourcemap_upload_tests { None, false, dry_run, + false, ) .await .unwrap_err(); @@ -2603,6 +2727,7 @@ mod sourcemap_upload_tests { None, false, false, + false, ) .await .unwrap(); @@ -2643,6 +2768,7 @@ mod sourcemap_upload_tests { None, false, false, + false, ) .await .unwrap(); @@ -2852,6 +2978,7 @@ mod sourcemap_upload_tests { Some(8), false, false, + false, ) .await .unwrap(); @@ -3019,9 +3146,299 @@ 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;"]) + ); + } + + /// 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"); + } + + /// 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. + /// + /// 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. + 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] + 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 +3552,7 @@ mod sourcemap_upload_tests { None, false, true, + false, ) .await .unwrap_err(); 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/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", + )); } } 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/env_robustness.rs b/tests/env_robustness.rs index f1d69e3..b20e85f 100644 --- a/tests/env_robustness.rs +++ b/tests/env_robustness.rs @@ -4,8 +4,14 @@ //! 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; +#[cfg(unix)] fn cli() -> Command { let mut c = Command::cargo_bin("bugsee-cli").expect("compiled bugsee-cli binary"); c.env_clear(); 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/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) 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]