From 1f9ab095253d5a6a9dee4b4b612ef46fc6af0a30 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Mon, 12 Oct 2026 19:25:57 +0530 Subject: [PATCH 1/3] Keep config permissions and hide config text in ts prebid bundle errors Move the audit atomic writer into a shared fs_util module and use it for the prebid bundle metadata patch, so the config keeps its permissions, is fsynced, and leaves no temp file behind on failure. Report TOML parse errors by byte offset instead of echoing the parser snippet, including in ts audit generate, through one shared toml_error_location helper. Closes #1202. Signed-off-by: dhruv8sh --- .../src/commands/audit/generate/mod.rs | 40 +----- .../src/commands/audit/generate/slot_toml.rs | 5 +- .../src/commands/audit/generate/validate.rs | 8 +- .../src/commands/audit/mod.rs | 7 +- crates/trusted-server-cli/src/error.rs | 10 ++ crates/trusted-server-cli/src/fs_util.rs | 71 +++++++++++ crates/trusted-server-cli/src/lib.rs | 2 + .../trusted-server-cli/src/prebid_bundle.rs | 116 +++++++++++------- 8 files changed, 168 insertions(+), 91 deletions(-) create mode 100644 crates/trusted-server-cli/src/fs_util.rs diff --git a/crates/trusted-server-cli/src/commands/audit/generate/mod.rs b/crates/trusted-server-cli/src/commands/audit/generate/mod.rs index 6532a7117a..7c901416d8 100644 --- a/crates/trusted-server-cli/src/commands/audit/generate/mod.rs +++ b/crates/trusted-server-cli/src/commands/audit/generate/mod.rs @@ -29,51 +29,13 @@ use crate::commands::audit::generate::slot_toml::{ }; use crate::commands::config::init::EXAMPLE_CONFIG; use crate::error::{CliResult, cli_error, report_error}; +use crate::fs_util::write_file_atomically; use analyzer::{analyze_collected_page, extract_gtm_container_id}; pub(crate) use browser_collector::DeviceProfile; pub(crate) use crawl_plan::CrawlBudget; -/// Writes `contents` to `path` atomically: a same-directory temp file is -/// written and fsynced, then renamed over the target, then the directory entry -/// is fsynced. -/// -/// A plain `fs::write` truncates the destination before writing, so a full disk -/// or an interrupted run would leave an operator's `trusted-server.toml` empty -/// or half-written. `rename` within a directory is atomic, so a reader sees -/// either the old file or the complete new one. -/// -/// The target's existing permissions are carried onto the replacement, since -/// the temp file is created 0600 and the config may intentionally be broader. -/// -/// # Errors -/// -/// Returns the underlying I/O error when the temp file cannot be created, -/// written, synced, or renamed over `path`. -fn write_file_atomically(path: &Path, contents: &str) -> std::io::Result<()> { - let directory = path - .parent() - .filter(|parent| !parent.as_os_str().is_empty()) - .unwrap_or_else(|| Path::new(".")); - - let mut temp = tempfile::Builder::new() - .prefix(".ts-audit-") - .tempfile_in(directory)?; - temp.write_all(contents.as_bytes())?; - temp.as_file().sync_all()?; - if let Ok(metadata) = fs::metadata(path) { - temp.as_file().set_permissions(metadata.permissions())?; - } - temp.persist(path).map_err(|error| error.error)?; - - // Best-effort durability for the rename itself. Opening a directory handle - // is not portable (Windows rejects it), and the content is already safely - // on disk either way, so a failure here is not worth failing the command. - let _ = fs::File::open(directory).and_then(|handle| handle.sync_all()); - Ok(()) -} - /// Arguments for `ts audit generate ` — bootstraps draft Trusted Server /// config and JavaScript asset audit files from a live page (issue #800). #[derive(Debug, clap::Args)] diff --git a/crates/trusted-server-cli/src/commands/audit/generate/slot_toml.rs b/crates/trusted-server-cli/src/commands/audit/generate/slot_toml.rs index c5d1627d85..47d5ce2fb2 100644 --- a/crates/trusted-server-cli/src/commands/audit/generate/slot_toml.rs +++ b/crates/trusted-server-cli/src/commands/audit/generate/slot_toml.rs @@ -12,7 +12,7 @@ use trusted_server_core::creative_opportunities::{ #[cfg(test)] use crate::commands::audit::generate::gpt_slots; -use crate::error::{CliResult, cli_error, report_error}; +use crate::error::{CliResult, cli_error, report_error, toml_error_location}; /// A slot ready to render — the union of discovered and existing fields, without /// the core type's `pub(crate)` compiled-pattern cache. @@ -627,7 +627,8 @@ pub(super) fn splice_creative_slots( ) -> CliResult { let mut document = existing.parse::().map_err(|error| { report_error(format!( - "failed to parse target config before updating slots: {error}" + "failed to parse target config{} before updating slots", + toml_error_location(error.span()) )) })?; let had_section = document.get("creative_opportunities").is_some(); diff --git a/crates/trusted-server-cli/src/commands/audit/generate/validate.rs b/crates/trusted-server-cli/src/commands/audit/generate/validate.rs index 4334cb398d..c8373362f0 100644 --- a/crates/trusted-server-cli/src/commands/audit/generate/validate.rs +++ b/crates/trusted-server-cli/src/commands/audit/generate/validate.rs @@ -9,7 +9,7 @@ use trusted_server_core::config::TrustedServerAppConfig; -use crate::error::{CliResult, cli_error}; +use crate::error::{CliResult, cli_error, toml_error_location}; /// Validates the candidate config text the generator is about to persist. /// @@ -54,9 +54,9 @@ pub(super) fn check_candidate(candidate: &str, baseline: &str) -> CliResult CliResult<()> { let config: TrustedServerAppConfig = toml::from_str(source).map_err(|error| { - error.span().map_or_else( - || "config could not be loaded".to_string(), - |span| format!("config could not be loaded at byte offset {}", span.start), + format!( + "config could not be loaded{}", + toml_error_location(error.span()) ) })?; TrustedServerAppConfig::new(config.into_settings()) diff --git a/crates/trusted-server-cli/src/commands/audit/mod.rs b/crates/trusted-server-cli/src/commands/audit/mod.rs index 6da0171b96..4cc758c4dc 100644 --- a/crates/trusted-server-cli/src/commands/audit/mod.rs +++ b/crates/trusted-server-cli/src/commands/audit/mod.rs @@ -17,7 +17,7 @@ use clap::{Args, Subcommand}; use crate::app_config::AppConfigArgs; use crate::commands::audit::collector::{BrowserOpts, GenerateBrowserOpts}; use crate::commands::audit::page::PageAuditArgs; -use crate::error::{CliResult, cli_error}; +use crate::error::{CliResult, cli_error, toml_error_location}; use crate::run::RunOutcome; /// Parses and validates an `http`/`https` URL, rejecting all other schemes. @@ -427,10 +427,7 @@ fn creative_config( // Parser messages can quote literal secrets from the operator's config. // Keep the path and location, but never include the source or error text. let value = toml::from_str::(document).map_err(|error| { - let location = error - .span() - .map(|span| format!(" at byte offset {}", span.start)) - .unwrap_or_default(); + let location = toml_error_location(error.span()); format!( "failed to parse {}{location} before generating slots; fix the TOML syntax and re-run", path.display() diff --git a/crates/trusted-server-cli/src/error.rs b/crates/trusted-server-cli/src/error.rs index 1ca36f0c4b..3b30dff885 100644 --- a/crates/trusted-server-cli/src/error.rs +++ b/crates/trusted-server-cli/src/error.rs @@ -36,3 +36,13 @@ pub enum CliError { } impl Error for CliError {} + +/// Formats a TOML error's location without the parser's message. +/// +/// Parser and deserializer messages quote the offending source line or value, +/// which can hold data an operator keeps private, so only the byte offset of +/// the error is reported. +pub(crate) fn toml_error_location(span: Option>) -> String { + span.map(|span| format!(" at byte offset {}", span.start)) + .unwrap_or_default() +} diff --git a/crates/trusted-server-cli/src/fs_util.rs b/crates/trusted-server-cli/src/fs_util.rs new file mode 100644 index 0000000000..58a94d8ffe --- /dev/null +++ b/crates/trusted-server-cli/src/fs_util.rs @@ -0,0 +1,71 @@ +//! Filesystem helpers shared by `ts` subcommands. + +use std::fs; +use std::io::Write as _; +use std::path::Path; + +/// Writes `contents` to `path` atomically: a same-directory temp file is +/// written and fsynced, then renamed over the target, then the directory entry +/// is fsynced. +/// +/// A plain `fs::write` truncates the destination before writing, so a full disk +/// or an interrupted run would leave the target empty or half-written. `rename` +/// within a directory is atomic, so a reader sees either the old file or the +/// complete new one. If any step fails, the temp file is removed on drop. +/// +/// The temp file is created 0600. When the target already exists, its +/// permissions are carried onto the replacement so the write neither widens +/// nor narrows access; a new file stays 0600. +/// +/// # Errors +/// +/// Returns the underlying I/O error when the temp file cannot be created, +/// written, synced, or renamed over `path`. +pub(crate) fn write_file_atomically(path: &Path, contents: &str) -> std::io::Result<()> { + let directory = path + .parent() + .filter(|parent| !parent.as_os_str().is_empty()) + .unwrap_or_else(|| Path::new(".")); + + let mut temp = tempfile::Builder::new() + .prefix(".ts-write-") + .tempfile_in(directory)?; + temp.write_all(contents.as_bytes())?; + temp.as_file().sync_all()?; + if let Ok(metadata) = fs::metadata(path) { + temp.as_file().set_permissions(metadata.permissions())?; + } + temp.persist(path).map_err(|error| error.error)?; + + // Best-effort durability for the rename itself. Opening a directory handle + // is not portable (Windows rejects it), and the content is already safely + // on disk either way, so a failure here is not worth failing the command. + let _ = fs::File::open(directory).and_then(|handle| handle.sync_all()); + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn write_file_atomically_removes_temp_file_when_rename_fails() { + let temp = tempfile::TempDir::new().expect("should create temp dir"); + let target = temp.path().join("occupied"); + fs::create_dir(&target).expect("should create directory at target"); + fs::write(target.join("child"), "x").expect("should populate target directory"); + + write_file_atomically(&target, "contents") + .expect_err("should fail to rename over a non-empty directory"); + + let entries: Vec<_> = fs::read_dir(temp.path()) + .expect("should list temp dir") + .map(|entry| entry.expect("should read entry").file_name()) + .collect(); + assert_eq!( + entries, + vec![std::ffi::OsString::from("occupied")], + "should leave no temp file behind after a failed write" + ); + } +} diff --git a/crates/trusted-server-cli/src/lib.rs b/crates/trusted-server-cli/src/lib.rs index 24e001d853..7f6b9e6e73 100644 --- a/crates/trusted-server-cli/src/lib.rs +++ b/crates/trusted-server-cli/src/lib.rs @@ -5,6 +5,8 @@ mod app_config; #[cfg(not(target_arch = "wasm32"))] mod error; #[cfg(not(target_arch = "wasm32"))] +mod fs_util; +#[cfg(not(target_arch = "wasm32"))] mod prebid_bundle; #[cfg(not(target_arch = "wasm32"))] mod run; diff --git a/crates/trusted-server-cli/src/prebid_bundle.rs b/crates/trusted-server-cli/src/prebid_bundle.rs index 609c950531..c92af3d85b 100644 --- a/crates/trusted-server-cli/src/prebid_bundle.rs +++ b/crates/trusted-server-cli/src/prebid_bundle.rs @@ -8,6 +8,9 @@ use std::process::{Command, Stdio}; use serde::{Deserialize, Deserializer, Serialize}; use toml_edit::{DocumentMut, Item, table, value}; +use crate::error::toml_error_location; +use crate::fs_util::write_file_atomically; + pub(crate) type CliResult = Result; const NODE_MODULES_MISSING_HELP: &str = "Prebid bundling dependencies are missing. Run `cd crates/trusted-server-js/lib && npm ci`, then retry `ts prebid client`."; @@ -480,8 +483,9 @@ pub(crate) fn load_bundle_config(config_path: &Path) -> CliResult CliResult CliResu })?; let mut document = contents.parse::().map_err(|error| { report_error(format!( - "failed to parse config {} for metadata update: {error}", - config_path.display() + "failed to parse config {}{} for metadata update", + config_path.display(), + toml_error_location(error.span()) )) })?; @@ -779,7 +787,12 @@ fn patch_config_metadata(config_path: &Path, sha256: &str, sri: &str) -> CliResu prebid.insert("external_bundle_sha256", value(sha256)); prebid.insert("external_bundle_sri", value(sri)); - write_atomic(config_path, &document.to_string()) + write_file_atomically(config_path, &document.to_string()).map_err(|error| { + report_error(format!( + "failed to write config {}: {error}", + config_path.display() + )) + }) } fn table_like_mut<'a>( @@ -795,46 +808,13 @@ fn table_like_mut<'a>( }) } -fn write_atomic(path: &Path, contents: &str) -> CliResult<()> { - let parent = path - .parent() - .filter(|parent| !parent.as_os_str().is_empty()) - .unwrap_or_else(|| Path::new(".")); - fs::create_dir_all(parent).map_err(|error| { - report_error(format!( - "failed to create config parent directory {}: {error}", - parent.display() - )) - })?; - - let filename = path - .file_name() - .and_then(|name| name.to_str()) - .unwrap_or("trusted-server.toml"); - let tmp_path = parent.join(format!(".{filename}.tmp-{}", std::process::id())); - - fs::write(&tmp_path, contents).map_err(|error| { - report_error(format!( - "failed to write temporary config {}: {error}", - tmp_path.display() - )) - })?; - fs::rename(&tmp_path, path).map_err(|error| { - let _ = fs::remove_file(&tmp_path); - report_error(format!( - "failed to replace config {} with {}: {error}", - path.display(), - tmp_path.display() - )) - })?; - - Ok(()) -} - #[cfg(test)] mod tests { use super::*; + #[cfg(unix)] + use std::os::unix::fs::PermissionsExt as _; + fn managed_config(managed: &str, user_id: &str) -> String { format!( r#" @@ -1880,4 +1860,58 @@ real_time_data = ["exampleRtdProvider"] "error should instruct npm ci: {error:?}" ); } + + #[cfg(unix)] + #[test] + fn patch_config_metadata_preserves_config_permissions() { + let (_temp, path) = write_config(&valid_config()); + fs::set_permissions(&path, fs::Permissions::from_mode(0o600)) + .expect("should restrict config permissions"); + + patch_config_metadata(&path, &"a".repeat(64), "sha384-abc") + .expect("should patch config metadata"); + + let mode = fs::metadata(&path) + .expect("should stat patched config") + .permissions() + .mode() + & 0o777; + assert_eq!(mode, 0o600, "should keep the operator's config permissions"); + } + + #[test] + fn patch_config_metadata_leaves_no_temp_files() { + let (temp, path) = write_config(&valid_config()); + + patch_config_metadata(&path, &"a".repeat(64), "sha384-abc") + .expect("should patch config metadata"); + + let entries: Vec<_> = fs::read_dir(temp.path()) + .expect("should list config directory") + .map(|entry| entry.expect("should read entry").file_name()) + .collect(); + assert_eq!( + entries, + vec![std::ffi::OsString::from("trusted-server.toml")], + "should leave only the config file behind" + ); + } + + #[test] + fn load_bundle_config_parse_error_does_not_echo_config_source() { + let (_temp, path) = write_config( + "[publisher]\norigin_url = \"https://origin-private.example.com/internal\n", + ); + + let error = load_bundle_config(&path).expect_err("should reject invalid TOML"); + + assert!( + !error.contains("origin-private.example.com"), + "should not echo config source text: {error}" + ); + assert!( + error.contains("at byte offset"), + "should report the error location: {error}" + ); + } } From 8b8445e582e690f19a7cae383694a76c378cb5a1 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Thu, 15 Oct 2026 14:53:05 +0530 Subject: [PATCH 2/3] Report TOML error line and column and tighten config write tests Map redacted TOML error spans to line and column, prove mode preservation with a 0640 config, cover the patch_config_metadata parse error, and narrow the atomic writer doc to the mode bits it actually keeps. Signed-off-by: dhruv8sh --- .../src/commands/audit/generate/slot_toml.rs | 2 +- .../src/commands/audit/generate/validate.rs | 13 ++-- .../src/commands/audit/mod.rs | 7 +-- crates/trusted-server-cli/src/error.rs | 45 ++++++++++++-- crates/trusted-server-cli/src/fs_util.rs | 8 ++- .../trusted-server-cli/src/prebid_bundle.rs | 62 ++++++++++++++----- 6 files changed, 98 insertions(+), 39 deletions(-) diff --git a/crates/trusted-server-cli/src/commands/audit/generate/slot_toml.rs b/crates/trusted-server-cli/src/commands/audit/generate/slot_toml.rs index 47d5ce2fb2..51b6bcb5f2 100644 --- a/crates/trusted-server-cli/src/commands/audit/generate/slot_toml.rs +++ b/crates/trusted-server-cli/src/commands/audit/generate/slot_toml.rs @@ -628,7 +628,7 @@ pub(super) fn splice_creative_slots( let mut document = existing.parse::().map_err(|error| { report_error(format!( "failed to parse target config{} before updating slots", - toml_error_location(error.span()) + toml_error_location(existing, error.span()) )) })?; let had_section = document.get("creative_opportunities").is_some(); diff --git a/crates/trusted-server-cli/src/commands/audit/generate/validate.rs b/crates/trusted-server-cli/src/commands/audit/generate/validate.rs index c8373362f0..a12e135100 100644 --- a/crates/trusted-server-cli/src/commands/audit/generate/validate.rs +++ b/crates/trusted-server-cli/src/commands/audit/generate/validate.rs @@ -49,14 +49,14 @@ pub(super) fn check_candidate(candidate: &str, baseline: &str) -> CliResult CliResult<()> { let config: TrustedServerAppConfig = toml::from_str(source).map_err(|error| { format!( "config could not be loaded{}", - toml_error_location(error.span()) + toml_error_location(source, error.span()) ) })?; TrustedServerAppConfig::new(config.into_settings()) @@ -93,10 +93,7 @@ mod tests { error.contains("refusing to write"), "should explain refusal" ); - assert!( - error.contains("byte offset"), - "should retain parser location" - ); + assert!(error.contains("at line "), "should retain parser location"); } #[test] @@ -111,7 +108,7 @@ mod tests { "should not disclose source bytes" ); assert!( - warnings[0].contains("byte offset"), + warnings[0].contains("at line "), "should retain parser location" ); } @@ -158,7 +155,7 @@ mod tests { "should cover schema as well as syntax errors" ); assert!( - error.contains("byte offset"), + error.contains("at line "), "should keep the parser location" ); } diff --git a/crates/trusted-server-cli/src/commands/audit/mod.rs b/crates/trusted-server-cli/src/commands/audit/mod.rs index 4cc758c4dc..1791566a0a 100644 --- a/crates/trusted-server-cli/src/commands/audit/mod.rs +++ b/crates/trusted-server-cli/src/commands/audit/mod.rs @@ -427,7 +427,7 @@ fn creative_config( // Parser messages can quote literal secrets from the operator's config. // Keep the path and location, but never include the source or error text. let value = toml::from_str::(document).map_err(|error| { - let location = toml_error_location(error.span()); + let location = toml_error_location(document, error.span()); format!( "failed to parse {}{location} before generating slots; fix the TOML syntax and re-run", path.display() @@ -524,10 +524,7 @@ mod tests { error.contains("fix the TOML syntax and re-run"), "should retain repair guidance, got {error}" ); - assert!( - error.contains("byte offset"), - "should retain parser location" - ); + assert!(error.contains("at line "), "should retain parser location"); assert!(!error.contains('\n'), "should not include a source excerpt"); } diff --git a/crates/trusted-server-cli/src/error.rs b/crates/trusted-server-cli/src/error.rs index 3b30dff885..6706cda7aa 100644 --- a/crates/trusted-server-cli/src/error.rs +++ b/crates/trusted-server-cli/src/error.rs @@ -40,9 +40,44 @@ impl Error for CliError {} /// Formats a TOML error's location without the parser's message. /// /// Parser and deserializer messages quote the offending source line or value, -/// which can hold data an operator keeps private, so only the byte offset of -/// the error is reported. -pub(crate) fn toml_error_location(span: Option>) -> String { - span.map(|span| format!(" at byte offset {}", span.start)) - .unwrap_or_default() +/// which can hold data an operator keeps private, so only the 1-based line and +/// column of the error within `source` are reported. +pub(crate) fn toml_error_location(source: &str, span: Option>) -> String { + let Some(start) = span.map(|span| span.start) else { + return String::new(); + }; + let before = source.get(..start).unwrap_or(source); + let line = before.matches('\n').count() + 1; + let column = before + .rsplit('\n') + .next() + .map_or(0, |tail| tail.chars().count()) + + 1; + format!(" at line {line}, column {column}") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn toml_error_location_reports_line_and_column() { + let source = "a = 1\nb = \"x\n"; + + let location = toml_error_location(source, Some(10..11)); + + assert_eq!( + location, " at line 2, column 5", + "should map offset to line and column" + ); + } + + #[test] + fn toml_error_location_is_empty_without_span() { + assert_eq!( + toml_error_location("a = 1", None), + "", + "should omit location without span" + ); + } } diff --git a/crates/trusted-server-cli/src/fs_util.rs b/crates/trusted-server-cli/src/fs_util.rs index 58a94d8ffe..36424e4f88 100644 --- a/crates/trusted-server-cli/src/fs_util.rs +++ b/crates/trusted-server-cli/src/fs_util.rs @@ -13,9 +13,11 @@ use std::path::Path; /// within a directory is atomic, so a reader sees either the old file or the /// complete new one. If any step fails, the temp file is removed on drop. /// -/// The temp file is created 0600. When the target already exists, its -/// permissions are carried onto the replacement so the write neither widens -/// nor narrows access; a new file stays 0600. +/// The temp file is created 0600. When the target already exists, its mode +/// bits are carried onto the replacement; a new file stays 0600. Because the +/// target is replaced by rename, its owner, group, ACLs and hard links are not +/// preserved, and a symlink at `path` is replaced by a regular file rather than +/// updating the file it points to. /// /// # Errors /// diff --git a/crates/trusted-server-cli/src/prebid_bundle.rs b/crates/trusted-server-cli/src/prebid_bundle.rs index c92af3d85b..98bc7d7450 100644 --- a/crates/trusted-server-cli/src/prebid_bundle.rs +++ b/crates/trusted-server-cli/src/prebid_bundle.rs @@ -485,7 +485,7 @@ pub(crate) fn load_bundle_config(config_path: &Path) -> CliResult CliResu report_error(format!( "failed to parse config {}{} for metadata update", config_path.display(), - toml_error_location(error.span()) + toml_error_location(&contents, error.span()) )) })?; @@ -1864,23 +1864,32 @@ real_time_data = ["exampleRtdProvider"] #[cfg(unix)] #[test] fn patch_config_metadata_preserves_config_permissions() { - let (_temp, path) = write_config(&valid_config()); - fs::set_permissions(&path, fs::Permissions::from_mode(0o600)) - .expect("should restrict config permissions"); - - patch_config_metadata(&path, &"a".repeat(64), "sha384-abc") - .expect("should patch config metadata"); - - let mode = fs::metadata(&path) - .expect("should stat patched config") - .permissions() - .mode() - & 0o777; - assert_eq!(mode, 0o600, "should keep the operator's config permissions"); + // The temp file is created 0600, so 0640 is what proves the target's mode + // is copied onto the replacement rather than the temp file's default kept. + for mode in [0o600, 0o640] { + let (_temp, path) = write_config(&valid_config()); + fs::set_permissions(&path, fs::Permissions::from_mode(mode)) + .expect("should set config permissions"); + + patch_config_metadata(&path, &"a".repeat(64), "sha384-abc") + .expect("should patch config metadata"); + + let patched = fs::metadata(&path) + .expect("should stat patched config") + .permissions() + .mode() + & 0o777; + assert_eq!( + patched, mode, + "should keep the operator's config permissions {mode:o}" + ); + } } + // Success path only; the failed-rename cleanup is covered by + // `fs_util::tests::write_file_atomically_removes_temp_file_when_rename_fails`. #[test] - fn patch_config_metadata_leaves_no_temp_files() { + fn patch_config_metadata_leaves_only_config_on_success() { let (temp, path) = write_config(&valid_config()); patch_config_metadata(&path, &"a".repeat(64), "sha384-abc") @@ -1910,7 +1919,26 @@ real_time_data = ["exampleRtdProvider"] "should not echo config source text: {error}" ); assert!( - error.contains("at byte offset"), + error.contains("at line "), + "should report the error location: {error}" + ); + } + + #[test] + fn patch_config_metadata_parse_error_does_not_echo_config_source() { + let (_temp, path) = write_config( + "[publisher]\norigin_url = \"https://origin-private.example.com/internal\n", + ); + + let error = patch_config_metadata(&path, &"a".repeat(64), "sha384-abc") + .expect_err("should reject invalid TOML"); + + assert!( + !error.contains("origin-private.example.com"), + "should not echo config source text: {error}" + ); + assert!( + error.contains("at line "), "should report the error location: {error}" ); } From d61637a07f7d415c23ae2472fdc1fabd97738b18 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Mon, 19 Oct 2026 14:23:39 +0530 Subject: [PATCH 3/3] Hide config text in remaining ts audit generate parse errors Route the dry-run diff and managed-field validation parse errors through toml_error_location, add regression tests, and gitignore .ts-write-* temp files left by an interrupted config write. Signed-off-by: dhruv8sh --- .gitignore | 2 ++ .../src/commands/audit/generate/mod.rs | 23 +++++++++++++++-- .../src/commands/audit/generate/slot_toml.rs | 25 +++++++++++++++++-- 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 24b9e06aa1..5e3da5b116 100644 --- a/.gitignore +++ b/.gitignore @@ -23,6 +23,8 @@ # env .env* trusted-server.toml +# partial copies left if the ts CLI config writer is killed mid-write +.ts-write-* js-assets.toml # backup diff --git a/crates/trusted-server-cli/src/commands/audit/generate/mod.rs b/crates/trusted-server-cli/src/commands/audit/generate/mod.rs index db6a6a4fa8..57a1ff76f1 100644 --- a/crates/trusted-server-cli/src/commands/audit/generate/mod.rs +++ b/crates/trusted-server-cli/src/commands/audit/generate/mod.rs @@ -28,7 +28,7 @@ use crate::commands::audit::generate::slot_toml::{ render_slots, replace_key_in_section, resolve_network_id, splice_creative_slots, toml_string, }; use crate::commands::config::init::EXAMPLE_CONFIG; -use crate::error::{CliResult, cli_error, report_error}; +use crate::error::{CliResult, cli_error, report_error, toml_error_location}; use crate::fs_util::write_file_atomically; use analyzer::{analyze_collected_page, extract_gtm_container_id}; @@ -1159,7 +1159,10 @@ pub(crate) fn run_update_slots( /// unrelated operator configuration from dry-run output. fn managed_creative_projection(document: &str) -> CliResult { let value = toml::from_str::(document).map_err(|error| { - report_error(format!("failed to parse config for dry-run diff: {error}")) + report_error(format!( + "failed to parse config for dry-run diff{}", + toml_error_location(document, error.span()) + )) })?; let creative = value .get("creative_opportunities") @@ -4338,4 +4341,20 @@ mod tests { "the concurrent edit must not be overwritten" ); } + + #[test] + fn managed_creative_projection_parse_error_does_not_echo_config_source() { + let source = "[publisher]\norigin_url = \"https://origin-private.example.com/internal\n"; + + let error = managed_creative_projection(source).expect_err("should reject invalid TOML"); + + assert!( + !error.contains("origin-private.example.com"), + "should not echo config source text: {error}" + ); + assert!( + error.contains("at line 2, column "), + "should report the error location: {error}" + ); + } } diff --git a/crates/trusted-server-cli/src/commands/audit/generate/slot_toml.rs b/crates/trusted-server-cli/src/commands/audit/generate/slot_toml.rs index 51b6bcb5f2..2ecb2e4f2a 100644 --- a/crates/trusted-server-cli/src/commands/audit/generate/slot_toml.rs +++ b/crates/trusted-server-cli/src/commands/audit/generate/slot_toml.rs @@ -701,8 +701,12 @@ pub(super) fn splice_creative_slots( /// Verifies that the structural update changed only generator-managed fields. fn ensure_only_managed_fields_changed(before: &str, after: &str) -> CliResult<()> { fn unmanaged(document: &str) -> CliResult { - let mut value = toml::from_str::(document) - .map_err(|error| report_error(format!("failed to validate updated config: {error}")))?; + let mut value = toml::from_str::(document).map_err(|error| { + report_error(format!( + "failed to validate updated config{}", + toml_error_location(document, error.span()) + )) + })?; if let Some(root) = value.as_table_mut() { let remove_empty = if let Some(creative) = root .get_mut("creative_opportunities") @@ -2467,4 +2471,21 @@ slot_id = "sidebar" toml::from_str::(&doc).expect("exotic targeting key renders as valid TOML"); } + + #[test] + fn ensure_only_managed_fields_changed_parse_error_does_not_echo_config_source() { + let source = "[publisher]\norigin_url = \"https://origin-private.example.com/internal\n"; + + let error = ensure_only_managed_fields_changed(source, source) + .expect_err("should reject invalid TOML"); + + assert!( + !error.contains("origin-private.example.com"), + "should not echo config source text: {error}" + ); + assert!( + error.contains("at line 2, column "), + "should report the error location: {error}" + ); + } }