[dhruv8sh]

Summary

• ts prebid client rewrote the operator's config with its own atomic writer, which reset a 0600 config to the umask default (usually 0644), skipped fsync, and could leave a partial .tmp-{pid} copy of the config behind on a failed write. It now uses the hardened writer from ts audit generate, moved into a shared fs_util module.
• TOML parse errors quoted the offending config line, which can hold values an operator keeps private (origin URL, store IDs, vendor account IDs) and lands in terminals and CI logs. Parse errors in ts prebid client and ts audit generate now report only the path plus line and column, through one shared toml_error_location helper.

Changes

File	Change
crates/trusted-server-cli/src/fs_util.rs	New module holding write_file_atomically (moved from the audit command), with a generic doc comment and a test that a failed rename leaves no temp file
crates/trusted-server-cli/src/lib.rs	Register fs_util
crates/trusted-server-cli/src/error.rs	Add shared toml_error_location helper, which maps the parser span to a line and column
crates/trusted-server-cli/src/prebid_bundle.rs	Replace write_atomic with write_file_atomically; stop echoing parser text in both config parse errors; document why the bundle-section deserialization error is safe to echo; add regression tests for permissions, temp-file cleanup and error redaction
crates/trusted-server-cli/src/commands/audit/generate/mod.rs	Import the shared writer instead of defining it
crates/trusted-server-cli/src/commands/audit/generate/slot_toml.rs	Stop echoing parser text when the target config fails to parse
crates/trusted-server-cli/src/commands/audit/mod.rs, crates/trusted-server-cli/src/commands/audit/generate/validate.rs	Use the shared toml_error_location helper instead of inline copies

Closes

Closes #1202

Test plan

• cargo test-fastly && cargo test-axum
• cargo clippy-fastly && cargo clippy-axum
• cargo fmt --all -- --check
• JS tests: cd crates/trusted-server-js/lib && npx vitest run
• JS format: cd crates/trusted-server-js/lib && npm run format
• Docs format: cd docs && npm run format
• WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
• Manual testing via fastly compute serve
• Other: cargo test-cloudflare, cargo clippy-cloudflare, cargo clippy-cli, and ./scripts/test-cli.sh (727 CLI tests pass, including the two failing tests from the issue)

Checklist

• Changes follow AGENTS.md conventions
• No unwrap() in production code — use expect("should ...")
• Uses tracing macros (not println!)
• New code has tests
• No secrets or credentials committed

[prk-Jr]

Summary

Moves the hardened atomic writer into a shared fs_util module, points ts prebid bundle at it, and stops echoing parser text in config parse errors. I ran the new permission and parse-error tests against main's implementation: both fail there and pass here, so they guard the regression from #1202. No blocking findings.

1 of the inline comments below carries a one-click GitHub suggestion.

Non-blocking

🏕 camp site

• No test for the patch_config_metadata parse-error redaction — see inline at crates/trusted-server-cli/src/prebid_bundle.rs:1916

⛏ nitpick

• patch_config_metadata_leaves_no_temp_files passes on main too — see inline at crates/trusted-server-cli/src/prebid_bundle.rs:1883

👍 praise

• Shared writer and documented echo exception — see inline at crates/trusted-server-cli/src/fs_util.rs:24

Cross-cutting / body-level findings

• 🏕 Two ts audit generate parse sites still echo {error} — managed_creative_projection (commands/audit/generate/mod.rs:1161, "failed to parse config for dry-run diff: {error}") and ensure_only_managed_fields_changed::unmanaged (commands/audit/generate/slot_toml.rs:705, "failed to validate updated config: {error}") both parse the operator's config and format the parser message. In practice neither is reachable with a malformed file, because creative_config (commands/audit/mod.rs:334) already parsed the same text with the same toml parser. But the PR description says parse errors in ts audit generate "now report only the path and byte offset". Routing both through toml_error_location keeps that true by construction, so a future reordering can't reintroduce the leak. Otherwise, narrow the claim in the description.
• 🌱 .ts-write-* temp files are not gitignored — the new writer removes its temp file on any error path, so issue item 3 is fixed for ordinary failures. A SIGKILL or power loss between create and rename can still leave a partial .ts-write-XXXXXX copy of the config next to it. It is now 0600, but git status lists it as untracked in a checkout and git add -A would pick it up. A one-line .ts-write-* entry in .gitignore, next to trusted-server.toml, would close that.

CI Status

• integration tests: PASS
• browser integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• CodeQL: PASS
• cargo test (ts CLI, native) (macos-latest): PASS
• cargo test (ts CLI, native) (ubuntu-latest): PASS
• cargo test (axum native): PASS
• cargo test (cross-adapter parity): PASS
• cargo test: PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo fmt: PASS
• CLAUDE.md symlink guard: PASS
• Analyze (rust): PASS
• Analyze (python): PASS
• Analyze (javascript-typescript): PASS
• Analyze (actions): PASS
• vitest: PASS
• format-typescript: PASS
• format-docs: PASS
• prepare integration artifacts: PASS

[ChristianPavilonis]

Approved. Sharing the hardened atomic writer and reporting TOML parse locations without config text addresses the permission-preservation and privacy concerns described in this PR.

[aram356]

Summary

Moves the hardened atomic writer into a shared fs_util module, points ts prebid client at it, and redacts TOML parse errors through one toml_error_location helper. One blocking test gap: the permission regression test can't detect the target's mode being dropped, which I proved by deleting the copy and watching every test pass. The other findings are non-blocking.

1 of the inline comments below carries a one-click GitHub suggestion. Use Commit suggestion to apply it as a commit on the PR branch. The remaining comments describe the change in prose because it touches several call sites or is a follow-up.

Blocking

🔧 wrench

• Permission test can't detect a dropped mode copy: see inline at crates/trusted-server-cli/src/prebid_bundle.rs:1864

Non-blocking

🤔 thinking

• Report line and column instead of a byte offset: see inline at crates/trusted-server-cli/src/error.rs:45

🌱 seedling

• The shared writer replaces symlinks and carries over mode bits only: see inline at crates/trusted-server-cli/src/fs_util.rs:38

Cross-cutting / body-level findings

• 📌 ts config validate / push / diff still print the offending config line: these load through edgezero-core's deserialize_app_config_with_options, whose AppConfigError Display is failed to parse {path} as TOML: {source}. The CLI's own app_config::load_settings (ts config ad-templates *, ts audit ad-templates verify) wraps the same error as failed to load {path}: {err}. Probe at this head, load_settings on a config whose third line is an unterminated origin_url string:

  failed to load <tmp>/trusted-server.toml: failed to parse <tmp>/trusted-server.toml as TOML: TOML parse error at line 3, column 58
    |
  3 | origin_url = "https://origin-private.example.com/internal
    |                                                          ^
  invalid basic string, expected `"`
  

  ts config push is the step most likely to run in CI, so the "lands in terminals and CI logs" scenario from the description is still open on that path. Out of scope for this PR; worth a follow-up issue covering edgezero's loader and the CLI wrapper.

• ⛏ The title names the retired ts prebid bundle spelling: the command is ts prebid client, and prebid_rejects_retired_bundle_command (run.rs:211) pins that ts prebid bundle no longer parses. The squash commit inherits the PR title, so Keep config permissions and hide config text in ts prebid client errors keeps git log searchable for the command operators actually run. The description's ts prebid bundle mentions need the same change.

CI Status

• integration tests: PASS
• browser integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• CodeQL: PASS
• cargo test (ts CLI, native) (macos-latest): PASS
• cargo test (axum native): PASS
• CLAUDE.md symlink guard: PASS
• Analyze (rust): PASS
• Analyze (python): PASS
• cargo fmt: PASS (required)
• Analyze (javascript-typescript): PASS
• vitest: PASS
• format-typescript: PASS (required)
• cargo test (ts CLI, native) (ubuntu-latest): PASS
• Analyze (javascript-typescript): PASS
• cargo test (cross-adapter parity): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• format-docs: PASS (required)
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• prepare integration artifacts: PASS
• cargo test: PASS (required)
• Analyze (actions): PASS

[dhruv8sh]

Filed #1396 for ts config validate, push and diff, which still echo the offending config line through edgezero-core's loader and app_config::load_settings. The title and description now say ts prebid client.

[dhruv8sh]

@prk-Jr both findings from your review are addressed in d61637a. Both ts audit generate sites now go through toml_error_location. managed_creative_projection reports failed to parse config for dry-run diff at line L, column C, and ensure_only_managed_fields_changed reports failed to validate updated config at line L, column C. Each has a test that a malformed config doesn't echo its source text, and both tests fail with the old {error} formatting. .gitignore now ignores .ts-write-* next to trusted-server.toml, so a copy left by a killed write isn't picked up by git add -A.

[aram356]

⛏ nitpick — splice_creative_slots is the one redacted parse site without a no-echo test. Restoring : {error} to the message at line 630 passes all 352 prebid_bundle, fs_util, error and audit tests, and so does passing "" instead of existing to toml_error_location (the error then says line 1, column 1). Same shape as the test above:

Suggested change

	}
	}
	#[test]
	fn splice_creative_slots_parse_error_does_not_echo_config_source() {
	let existing = "[publisher]\norigin_url = \"https://origin-private.example.com/internal\n";
	let error = splice_creative_slots(existing, &network_keys("222"), &header_rendered())
	.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}"
	);
	}

Verified in a scratch worktree: passes at this head, fails under both mutations, and cargo fmt --check, cargo clippy-cli and the full trusted-server-cli suite pass with it applied.

[aram356]

⛏ nitpick — Both prebid parse tests only check for at line , so they pass when the helper gets the wrong text. Passing "" instead of &contents at line 488 or line 761 reports line 1, column 1, and every test still passes. The audit tests pin at line 2, column ; this does the same for both tests in one click (the lines in between are unchanged):

Suggested change

	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 "),
	error.contains("at line 2, column "),
	"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 2, column "),

Verified in a scratch worktree: passes at this head, fails under both mutations, and cargo fmt --check, cargo clippy-cli and the full trusted-server-cli suite pass with it applied.

[aram356]

⛏ nitpick — Nothing pins that the column counts characters. Swapping tail.chars().count() for tail.len() passes every test, but would put the x in a = "é" x at column 10. The current code is right: I compared this helper with toml's own line, column on malformed inputs with multibyte, emoji, tab, CRLF and BOM content, and it matched on all of them. This keeps it that way:

Suggested change

	}
	}
	#[test]
	fn toml_error_location_counts_columns_in_characters() {
	// "é" is two bytes, so counting bytes would report column 10.
	let source = "a = \"é\" x\n";
	let location = toml_error_location(source, Some(9..10));
	assert_eq!(
	location, " at line 1, column 9",
	"should count the column in characters, not bytes"
	);
	}

Verified in a scratch worktree: passes at this head, fails with the byte-count mutation, and cargo fmt --check, cargo clippy-cli and the full trusted-server-cli suite pass with it applied.

[aram356]

🏕 camp site — This is the operator-visible fix, but CHANGELOG.md has no entry for it. The fix keeps a config's current mode, so a 0600 config that an earlier ts prebid client already reset to 0644 stays 0644; operators only learn to restrict it again if the release notes tell them. Suggested entry under ### Security in [Unreleased]:

• ts prebid client now keeps the config file's mode bits when it writes external_bundle_sha256 and external_bundle_sri; previously a 0600 config was reset to the umask default (usually 0644). Configs already rewritten by an earlier version keep the widened mode, so restrict them again (for example chmod 600 trusted-server.toml). The rewrite is fsynced and leaves no temporary copy behind on failure. TOML parse errors from ts prebid client and ts audit generate now name the file, line and column without quoting the offending config line (ts prebid bundle widens config file permissions and echoes config lines in parse errors #1202).

Apply manually: CHANGELOG.md isn't part of this diff, so this can't be a one-click suggestion.