Reject image optimizer names that collide after trimming

[ChristianPavilonis]

Summary

• Reject image profile names and profile-set names that collide after trimming whitespace. Previously, names such as medium and " medium" silently overwrote one another, with the surviving value depending on map iteration order.
• Check the original keys before normalization discards them, and report both conflicting keys with their configuration path.
• Preserve existing behavior for valid configuration, including trimmed values and references, and discarded empty keys.

Changes

File	Change
crates/trusted-server-core/src/settings.rs	Validate both sets of names before rebuilding the maps, propagate configuration errors through settings loading, and test TOML/JSON rejection and compatibility.
crates/trusted-server-core/src/config.rs	Propagate normalization errors through CLI app-config deserialization and test collision rejection and valid normalization.

Scope

This is a configuration-loading fix confined to two core files. Most added lines are regression tests. No dependencies, adapter code, or template-cache fingerprint logic change; ambiguous configuration is rejected instead of choosing a winner.

Closes

Closes #1211

Test plan

The focused image-optimizer test run failed before the production change with three regression failures, then passed all 25 tests afterward. Cases cover both collision sites, leading/trailing whitespace, equal values, error context, and non-colliding configuration through TOML, JSON, and CLI app-config loading.

• 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, all 1,155 tests passed
• JS format: cd crates/trusted-server-js/lib && npm run format
• Docs format: cd docs && npm run format, passed after installing missing dependencies with npm ci
• WASM release build: not run separately; Fastly tests and Clippy passed on wasm32-wasip1
• Manual testing via fastly compute serve: not run; regression checks exercise configuration loading directly
• Other: neighboring settings/config tests, cargo test-cloudflare, cargo test-spin, ./scripts/test-cli.sh, cross-adapter parity tests, and host-target OpenRTB codegen tests
• Other: cargo clippy-cloudflare, cargo clippy-cloudflare-wasm, cargo clippy-spin-native, cargo clippy-spin-wasm, cargo clippy-cli, and cargo clippy-codegen
• Other: JS build with node build-all.mjs and final git diff --check

Six existing core tests remain ignored. Docs dependency installation reported 17 audit vulnerabilities; dependency changes are outside this fix.

Checklist

• Changes follow AGENTS.md conventions
• No new unwrap() calls in production code
• No new logging or print calls
• New behavior has regression tests
• No secrets or credentials committed

[prk-Jr]

Summary

Approve. The fix rejects profile and profile-set names that collide after trimming before rebuilding either image map, retains the original keys in useful error messages, and propagates failures through runtime and CLI configuration loading. No blocking defects found; the two recommendations below are non-blocking.

Non-blocking recommendations

• 🤔 Make error selection deterministic across profile sets — see inline at crates/trusted-server-core/src/settings.rs:1036–1043.

Cross-cutting recommendation

🏕 Document the configuration compatibility change

Add the following beside the profile-table documentation in docs/guide/configuration.md, with a short entry under CHANGELOG.md → Unreleased → Fixed:

Profile and profile-set names are trimmed. Names that become identical
after trimming are rejected, even when their values match. Empty names
continue to be discarded.

This gives operators a clear explanation when previously accepted ambiguous configuration is rejected.

Apply manually: these documentation files are outside this PR's diff. This is a prose-only recommendation.

Local validation

• cargo test-fastly image_optimizer --lib --offline: 25 focused image-optimizer tests passed under Viceroy.
• cargo fmt --all -- --check: passed.
• git diff --check: passed.
• A temporary diagnostic probe confirmed varying first-error selection when two sets contain collisions. The probe was removed, and the reviewer worktree is clean.

CI Status

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

[dhruv8sh]

Summary

Rejects profile and profile-set names that collide after trimming, before either image map is rebuilt, and passes the error through runtime and CLI config loading. The fix is correct and well tested; nothing blocking.

2 of the inline comments below carry a one-click GitHub suggestion — use Commit suggestion (or Add suggestion to batch for several at once) to apply them as commits on the PR branch.

Non-blocking

🤔 thinking / ⛏ nitpick

• Errors are not deterministic when several profile sets collide (+1 to the earlier comment) — see inline at crates/trusted-server-core/src/settings.rs:1036
• New assertions have no messages — see inline at crates/trusted-server-core/src/config.rs:864-865

👍 praise

• Validation runs before the maps are rebuilt, and the error is deterministic and clear — see inline at crates/trusted-server-core/src/settings.rs:1066-1094

Cross-cutting / body-level findings

• 🏕 Docs/CHANGELOG note — +1 to @prk-Jr's earlier request to document that profile and profile-set names which become identical after trimming are now rejected (in docs/guide/configuration.md and under CHANGELOG.md → Unreleased → Fixed), so operators whose previously accepted config now fails to load have an explanation.

CI Status

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

[ChristianPavilonis]

Rebased onto main at 2a0cb04 and pushed the review follow-up in ce4aace.

Addressed the worthwhile nits: stable collision-error selection across profile sets with a repeated TOML/JSON regression, descriptive assertion messages, clearer normalization documentation, and dotted TOML paths in errors. @prk-Jr @dhruv8sh: added the compatibility note beside the profile-table documentation in docs/guide/configuration.md and under Unreleased / Fixed in CHANGELOG.md. The suggested shared map-normalization refactor is deferred to keep this fix scoped. Replied to and resolved all seven review threads.

Checks passed:

• cargo test-fastly image_optimizer --lib --offline: 26 tests
• cargo test-fastly settings::tests --lib --offline: 178 tests
• cargo test-fastly config::tests --lib --offline: 54 tests
• CARGO_NET_OFFLINE=true cargo clippy-fastly
• cargo fmt --all -- --check
• Prettier check for CHANGELOG.md and docs/guide/configuration.md
• git diff --check

The new regression failed before sorting and passed afterward. Full cross-adapter and browser suites were not rerun locally; CI is running against the pushed revision.