ts prebid bundle widens config file permissions and echoes config lines in parse errors

[aram356]

Description

ts prebid bundle writes external_bundle_sha256 and external_bundle_sri into the operator's config (patch_config_metadata, crates/trusted-server-cli/src/prebid_bundle.rs:743-783). It uses its own write_atomic (prebid_bundle.rs:798-830). The same crate already has a safer writer, write_file_atomically in commands/audit/generate/mod.rs:38-75, and a non-disclosing parse error in commands/audit/mod.rs:427-438. Compared with those, the bundle command has four problems:

1. Permissions. It writes a new temp file with fs::write, which uses the process umask, then renames it over the config. A 0600 config comes back 0644, readable by every local user under the usual 022 umask.

2. Durability. It does not fsync the temp file before the rename or the directory after it. The audit writer does both, and its doc comment explains why.

3. Leftover temp file. When the temp write fails partway (a full disk, for example), .{filename}.tmp-{pid} is left behind with part of the config in it. Only the rename-failure path removes it. .gitignore covers trusted-server.toml but not that name, so the partial copy shows up as an untracked file in a checkout.

4. Disclosure in errors. A TOML syntax error prints the parser's snippet of the operator file: invalid TOML in {path}: {error} (prebid_bundle.rs:480-485). The same applies to failed to parse config {path} for metadata update: {error} (prebid_bundle.rs:750-755), although that path is reached only if the file changes between the two reads or the toml and toml_edit parsers disagree. The audit command avoids this on purpose:

   // Parser messages can quote literal secrets from the operator's config.
   // Keep the path and location, but never include the source or error text.

The CLI itself can produce a 0600 config. ts audit generate writes a new draft config through write_file_atomically, and tempfile creates the temp file 0600. With no existing target, the file keeps that mode (generate/mod.rs:46-47, 60-68). Operators may also restrict the file by hand. ts config init writes with fs::write (commands/config/init.rs:51), so its output follows the umask.

What the file holds: secret fields contain secret-store key names, not secret values (the list is in TrustedServerAppConfig::secret_fields, crates/trusted-server-core/src/config.rs:134-234). The file does hold values an operator may keep private, such as publisher.origin_url (the origin behind the CDN), request_signing store IDs, auction provider endpoints and account IDs, and vendor account IDs such as Lockr's app_id and Permutive's organization_id and workspace_id. A typo on one of those lines prints it to the terminal and to any CI log that captures it.

write_atomic and the parse message came in with #743 (2026-07-07). The audit writer and its non-disclosing errors came later, in #823 (2026-09-21).

Steps to reproduce

As failing tests

In crates/trusted-server-cli/src/prebid_bundle.rs, add this import at the top of mod tests:

#[cfg(unix)]
use std::os::unix::fs::PermissionsExt as _;

and these tests to the same module:

#[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 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}"
    );
}

Run:

cargo test -p trusted-server-cli --lib --target "$(rustc -vV | sed -n 's/host: //p')" -- \
  prebid_bundle::tests::patch_config_metadata_preserves_config_permissions \
  prebid_bundle::tests::load_bundle_config_parse_error_does_not_echo_config_source

Observed (420 is 0o644, 384 is 0o600; temp path shortened):

test prebid_bundle::tests::load_bundle_config_parse_error_does_not_echo_config_source ... FAILED
test prebid_bundle::tests::patch_config_metadata_preserves_config_permissions ... FAILED

should not echo config source text: invalid TOML in <tmp>/trusted-server.toml: TOML parse error at line 2, column 58
  |
2 | origin_url = "https://origin-private.example.com/internal
  |                                                          ^
invalid basic string, expected `"`

assertion `left == right` failed: should keep the operator's config permissions
  left: 420
 right: 384

End to end with the real binary

This script replaces only the Node build with a stub npm that writes the manifest the command reads. Everything else is the real ts prebid bundle. Run it from the repository root:

#!/bin/bash
set -u
HOST=$(rustc -vV | sed -n 's/host: //p')
cargo build -q -p trusted-server-cli --target "$HOST"
TS="${CARGO_TARGET_DIR:-$PWD/target}/$HOST/debug/ts"
WORK=$(mktemp -d)

mkdir -p "$WORK/bin"
cat > "$WORK/bin/npm" <<'STUB'
#!/bin/bash
while [ $# -gt 0 ]; do [ "$1" = "--out" ] && out="$2"; shift; done
mkdir -p "$out"
sha=$(printf probe | shasum -a 256 | cut -d' ' -f1)
printf '/* stub */\n' > "$out/trusted-prebid-$sha.js"
printf '{"schemaVersion":1,"sha256":"%s","sri":"sha384-cHJvYmU=","filename":"trusted-prebid-%s.js"}\n' "$sha" "$sha" > "$out/manifest.json"
STUB
chmod +x "$WORK/bin/npm"

# The shipped template with the bundle inputs uncommented.
sed -e 's/^# \[integrations.prebid.bundle.modules\]/[integrations.prebid.bundle.modules]/' \
    -e 's/^# bidder = \["rubiconBidAdapter"\]/bidder = ["rubiconBidAdapter"]/' \
    trusted-server.example.toml > "$WORK/trusted-server.toml"

echo "== 1. permissions"
chmod 600 "$WORK/trusted-server.toml"
ls -l "$WORK/trusted-server.toml" | cut -c1-10
PATH="$WORK/bin:$PATH" "$TS" prebid bundle --config "$WORK/trusted-server.toml" --out "$WORK/dist" > /dev/null
ls -l "$WORK/trusted-server.toml" | cut -c1-10

echo "== 2. parse error"
sed -i.bak -e 's#^origin_url = "https://origin.example.com"#origin_url = "https://origin-private.example.com/internal#' "$WORK/trusted-server.toml"
PATH="$WORK/bin:$PATH" "$TS" prebid bundle --config "$WORK/trusted-server.toml" --out "$WORK/dist" 2>&1 | sed "s#$WORK#<work>#"
mv "$WORK/trusted-server.toml.bak" "$WORK/trusted-server.toml"

echo "== 3. failed temp write"
# Cap file size at 8 KiB and ignore SIGXFSZ, so the temp write fails with EFBIG
# the way a full disk fails with ENOSPC.
( trap '' XFSZ; ulimit -f 8; PATH="$WORK/bin:$PATH" exec "$TS" prebid bundle --config "$WORK/trusted-server.toml" --out "$WORK/dist" ) 2>&1 | sed "s#$WORK#<work>#"
ls -la "$WORK" | grep 'tmp-' | awk '{print $1, $5, $NF}'

Observed on macOS with umask 0022:

== 1. permissions
-rw-------
-rw-r--r--
== 2. parse error
[ts] invalid TOML in <work>/trusted-server.toml: TOML parse error at line 61, column 58
   |
61 | origin_url = "https://origin-private.example.com/internal
   |                                                          ^
invalid basic string, expected `"`

== 3. failed temp write
[ts] failed to write temporary config <work>/.trusted-server.toml.tmp-88420: File too large (os error 27)
-rw-r--r--@ 8192 .trusted-server.toml.tmp-88420

In the repository, the temp name is not ignored:

$ git check-ignore -v --no-index .trusted-server.toml.tmp-88420 || echo "not ignored"
not ignored
$ git check-ignore -v --no-index trusted-server.toml
.gitignore:25:trusted-server.toml	trusted-server.toml

The two writers side by side

A standalone program with verbatim copies of both functions (error types changed only), tempfile 3.27.0 as locked, umask 0022:

audit temp file mode while open: 600
audit temp file exists after drop without persist: false
prebid bundle write_atomic : target before=absent after=644
prebid bundle write_atomic : target before=   600 after=644
prebid bundle write_atomic : target before=   640 after=644
audit write_file_atomically: target before=absent after=600
audit write_file_atomically: target before=   600 after=600
audit write_file_atomically: target before=   640 after=640

Expected behavior

• The patched config keeps its previous mode. A new file is created 0600, as the audit writer does.
• The replacement is durable: the temp file is fsynced before the rename and the directory after it.
• A failed write leaves no temp file behind.
• Parse errors name the path and the byte offset but do not print source text.

Actual behavior

• A 0600 config becomes 0644 after one run.
• No fsync.
• A failed temp write leaves a world-readable partial copy of the config next to it, which git status lists as untracked.
• Parse errors print the offending line of the config.

Root cause

write_atomic (prebid_bundle.rs:798-830):

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);
    // ...
})?;

fs::write creates the temp file with the default mode and the rename replaces the target inode, so the old mode is lost. There is no sync_all. The write-error branch returns without removing tmp_path.

load_bundle_config (prebid_bundle.rs:480-485) formats the toml error with {error}, and its Display includes a source excerpt. patch_config_metadata does the same with the toml_edit error (prebid_bundle.rs:750-755).

For comparison, write_file_atomically (commands/audit/generate/mod.rs:54-75) uses a tempfile in the same directory, which is 0600 and removed on drop. It calls sync_all, copies the target's permissions when the target exists, persists with a rename, then fsyncs the directory. The audit parser error (commands/audit/mod.rs:429-438) reports only the path and error.span().

Impact

• Operators who run ts prebid bundle against a config they restricted, including one produced by ts audit generate, end up with a config other local users can read. On a single-user workstation this matters little. On a shared build host or CI runner it exposes origin hosts, store IDs and endpoints.
• The echoed source line reaches terminals and CI logs. Secret values are not in the file by design, so this exposes the same class of private-but-not-secret values. It would expose a real secret only if one was pasted into the file by mistake.
• The leftover temp file needs a failed write, such as a full disk, or a crash between the write and the rename. It is rare, but when it happens the partial config can be committed with git add -A.
• Missing fsync can matter after a power loss or a kernel crash right after the command. It is rare.

Proposed fix

1. Move write_file_atomically out of commands/audit/generate/mod.rs into a small shared module in trusted-server-cli, and call it from patch_config_metadata. Delete write_atomic.
2. Make both parse errors in prebid_bundle.rs non-disclosing: the path plus error.span() byte offset, and no {error}. Follow creative_config in commands/audit/mod.rs:423-452.
3. Add the two tests above, plus one that a failed write leaves no temp file behind, and keep patch_config_metadata_writes_hash_and_sri.

Steps 1 and 2 were tried on a copy of a4e01eb, by making write_file_atomically pub(crate) and pointing write_atomic at it. Both new tests pass, and so do all 507 trusted-server-cli lib tests and the CLI integration test binaries.

The patched file's mode changes only for configs that were not 0644 before. For configs that were, nothing visible changes.

Done when

• patch_config_metadata preserves the target's mode, fsyncs, and leaves no temp file on failure, with a regression test for each.
• load_bundle_config and patch_config_metadata parse errors print the path and byte offset only, with a test that asserts no source text appears.
• ts prebid bundle and ts audit share one atomic writer.
• cargo clippy-cli and ./scripts/test-cli.sh pass.

Affected area

CI / Tooling

Version

main at a4e01eb

Related

• Prebid external first-party bundle loading #743 (merged): introduced ts prebid bundle, write_atomic and the parse error text.
• Add ts CLI ad-template config diagnostics and browser audit #823 (merged): introduced write_file_atomically and the non-disclosing parse errors in ts audit.
• Add Prebid fork source CLI design spec #1185 (draft spec): would extend ts prebid bundle with fork sources. It relies on the same config patch step.
• Define integrations crate split and ordered configuration #1194 (draft spec): says the bundle command keeps its ability to patch the config atomically. Worth fixing the writer before that work builds on it.