Add the Trusted Server CLI by ChristianPavilonis · Pull Request #669 · IABTechLab/trusted-server

ChristianPavilonis · 2026-04-29T18:22:50Z

Reviewer quick links

Start here if you want to test the new CLI directly:

CLI guide: docs/guide/cli.md
Fastly provisioning map: docs/guide/fastly-provisioning.md
Fastly setup guide: docs/guide/fastly.md
Getting started guide: docs/guide/getting-started.md

The most useful review path is:

Read the CLI command reference.
Read the provisioning map to understand which trusted-server.toml changes map to Fastly resources.
Follow the manual test guide below against a disposable or test Fastly service.

Manual reviewer test plan

Use a disposable or test Fastly Compute service. Run the commands one by one so you can inspect each CLI step and its output.

1. Set up local CLI access

Install the host-target CLI from the checkout:

cargo install_cli

Confirm the command surface is available:

ts --help
ts config --help
ts audit --help
ts auth fastly --help
ts provision fastly --help

If you are not on Apple Silicon macOS, use the explicit host-target form from docs/guide/cli.md instead of cargo install_cli.

2. Authenticate to Fastly

For local review, store a Fastly API token in the host credential store:

ts auth fastly login

Inspect credential status in both human and JSON forms:

ts auth fastly status
ts auth fastly status --json

Expected result: effective_source is secure-storage, unless you set FASTLY_API_KEY, in which case it should be environment.

For CI-style review, skip login and use:

export FASTLY_API_KEY="your-fastly-token"
ts auth fastly status --json

3. Audit the test site

Create a temp directory so generated audit files do not overwrite your checkout files:

WORKDIR="$(mktemp -d)"
echo "$WORKDIR"

Run the browser-backed audit against the test site:

ts audit https://getpurpose.ai \
  --js-assets "$WORKDIR/js-assets.toml" \
  --config "$WORKDIR/trusted-server.toml" \
  --force

Inspect the generated files:

ls -la "$WORKDIR"
sed -n '1,220p' "$WORKDIR/js-assets.toml"
sed -n '1,260p' "$WORKDIR/trusted-server.toml"

Expected result: the audit writes js-assets.toml and a draft trusted-server.toml. The terminal summary should include the audited URL, page title, JS asset count, third-party asset count, detected integrations, and written paths.

4. Validate the generated config

Run validation in human-readable form:

ts config validate --config "$WORKDIR/trusted-server.toml"

Run validation in JSON form:

ts config validate --config "$WORKDIR/trusted-server.toml" --json

Expected result: JSON output has valid: true, a resolved path, a non-empty config_hash, and an empty errors array.

5. Preview Fastly provisioning

Set your test service ID:

export FASTLY_SERVICE_ID="svc_..."

Run a JSON provisioning plan:

ts provision fastly plan \
  --service-id "$FASTLY_SERVICE_ID" \
  --config "$WORKDIR/trusted-server.toml" \
  --json

Inspect these fields in the JSON output:

service_id
config_path
service_version.latest_version
service_version.target_version
service_version.clone_required
actions
warnings

Expected result: plan reports what would be created, updated, or bound without mutating the Fastly service.

6. Apply provisioning to the test service

Apply with JSON output and skip the confirmation prompt:

ts provision fastly apply \
  --service-id "$FASTLY_SERVICE_ID" \
  --config "$WORKDIR/trusted-server.toml" \
  --json \
  --yes

Inspect these fields in the JSON output:

completed_actions
warnings
failed_action
activated_version
service_version.target_version

Expected result: completed_actions lists the changes made. If resource bindings changed, activated_version should be true.

If request signing is enabled and api-keys/api_key is missing, provide exactly one runtime API token source before applying:

export FASTLY_RUNTIME_API_KEY="runtime-token"

For a disposable test service only, you can instead add:

--reuse-management-api-key

7. Re-run plan to check idempotency

Run the plan again:

ts provision fastly plan \
  --service-id "$FASTLY_SERVICE_ID" \
  --config "$WORKDIR/trusted-server.toml" \
  --json

Expected result: after a successful apply, actions should usually be empty unless a runtime token was intentionally not applied or the service changed externally.

8. Useful docs while reviewing output

Summary

Add the new ts host CLI as a dedicated Rust crate for Trusted Server config, dev, auth, audit, and Fastly provisioning workflows.
Reuse the existing runtime-config/core abstractions while splitting host-target CLI verification from the repo’s existing wasm-target verification lanes.
Upgrade ts audit from a static HTML fetcher to a Chromium-backed browser audit while preserving the existing audit artifact and draft-config outputs.
Add first-class CLI docs and a Fastly provisioning map for reviewers and operators.

Changes

File	Change
`Cargo.toml`	Added the new `trusted-server-cli` workspace crate and host-side CLI/browser dependencies.
`Cargo.lock`	Updated the lockfile for the new CLI, Fastly, and Chromiumoxide dependencies.
`.cargo/config.toml`	Added host-target Cargo aliases for CLI build, check, test, and install workflows.
`crates/trusted-server-cli/Cargo.toml`	Defined the `ts` binary crate and its dependencies.
`crates/trusted-server-cli/src/lib.rs`	Added the clap command surface and command dispatch for config, audit, dev, auth, and Fastly provisioning flows.
`crates/trusted-server-cli/src/main.rs`	Added the `ts` binary entrypoint.
`crates/trusted-server-cli/src/error.rs`	Added CLI-specific error types.
`crates/trusted-server-cli/src/output.rs`	Added shared human/JSON output helpers.
`crates/trusted-server-cli/src/config.rs`	Added config path resolution, starter template writing, and config validation using `trusted-server-core` runtime config loading.
`crates/trusted-server-cli/src/dev.rs`	Added `ts dev` and Rust-native local Fastly manifest rendering/launch behavior.
`crates/trusted-server-cli/src/fastly/auth.rs`	Added Fastly credential resolution, secure storage login/logout/status, and precedence handling for `FASTLY_API_KEY`.
`crates/trusted-server-cli/src/fastly/api.rs`	Added a host-side Fastly API client for provisioning resources and bindings.
`crates/trusted-server-cli/src/fastly/provision.rs`	Added Fastly plan/apply provisioning logic for config stores, secret stores, KV stores, and service bindings.
`crates/trusted-server-cli/src/fastly/mod.rs`	Wired Fastly auth/API/provision modules together.
`crates/trusted-server-cli/src/audit.rs`	Added the stable audit façade that preserves the current audit output contract.
`crates/trusted-server-cli/src/audit/collector.rs`	Added normalized collected-page models shared by the audit pipeline.
`crates/trusted-server-cli/src/audit/http_collector.rs`	Preserved the original static HTTP collector behind an internal seam.
`crates/trusted-server-cli/src/audit/browser_collector.rs`	Added Chromiumoxide-backed browser collection for rendered HTML, title, final URL, script tags, and browser resource entries.
`crates/trusted-server-cli/src/audit/analyzer.rs`	Added analyzer logic that merges DOM and browser-observed script evidence into the existing `AuditArtifact` schema.
`.github/workflows/test.yml`	Split Rust testing into wasm-target runtime lanes and a host-target CLI lane.
`.github/workflows/format.yml`	Split Rust clippy verification into wasm-target runtime lanes and a host-target CLI lane.
`.github/pull_request_template.md`	Updated the PR test checklist for the new host-target CLI verification commands.
`CLAUDE.md`	Documented the new split runtime/CLI verification workflow.
`README.md`	Added `ts` CLI usage examples and documented browser prerequisites for `ts audit`.
`docs/guide/cli.md`	Added the full CLI guide and command reference.
`docs/guide/fastly-provisioning.md`	Added the config-to-Fastly-resource provisioning map.
`docs/guide/getting-started.md`	Added `ts` config/dev/audit workflow examples and browser audit prerequisites.
`docs/guide/configuration.md`	Added `ts config init` / `ts config validate` guidance and linked provisioning behavior.
`docs/guide/fastly.md`	Added `ts auth fastly ...`, `ts provision fastly ...`, and provisioning-map links.

Closes

Closes #

Automated test plan

Checklist

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

aram356

Summary

New trusted-server-cli crate adds ts config|audit|dev|auth fastly|provision fastly commands and switches ts audit to a Chromium-driven collector. The shape is solid — clean module split, trait-seamed credential store, comprehensive MockFastlyApi for the provision flow — but several issues block merge: clippy fails on the host target with -D warnings, the new HTTP clients have no timeouts, ts audit accepts arbitrary URL schemes (local-file disclosure via headless Chromium), and the repo's CI workflows don't actually run for this PR.

Blocking

🔧 wrench

Clippy fails on host target: clippy::needless_pass_by_value in build_audit_outputs (crates/trusted-server-cli/src/audit.rs:70). Inline comment.
No request/connect timeouts on Fastly API client (crates/trusted-server-cli/src/fastly/api.rs:107-113). Inline comment.
No request/connect timeouts on audit HTTP collector (crates/trusted-server-cli/src/audit/http_collector.rs:11-15). Inline comment.
ts audit accepts arbitrary URL schemes — Url::parse lets file:// / data: through, and validate_navigation_response (crates/trusted-server-cli/src/audit/browser_collector.rs:213-216) returns Ok(()) for any non-http(s) scheme. ts audit file:///etc/passwd can navigate headless Chromium to local files and serialize the rendered HTML into the on-disk artifact. Restrict scheme at parse time. Inline comment on crates/trusted-server-cli/src/lib.rs:218.
No Rust CI runs for this PR: .github/workflows/format.yml:6-7 and .github/workflows/test.yml:9-10 trigger only on pull_request: branches: [main]. PR #669 targets feature/ts-config-in-config-store, so neither cargo test, cargo clippy, nor the new host-CLI lanes have run on this commit. The push runs are recorded as failed with "log not found" — they never executed. Either retarget onto main, expand the branches: filter to include the stack base branch, or land the stack first. As direct evidence: the clippy break above was not caught because no clippy job ever ran here.

❓ question

--reuse-management-api-key privilege escalation (crates/trusted-server-cli/src/lib.rs:336-375). The flag plants a credential with full Fastly management privileges into the runtime secret store; if the worker ever leaks it, an attacker can mutate Fastly resources, not just sign requests. Is this a smoke-test convenience or a supported production path? Should --help explicitly call out the risk, and should it require confirmation even with --yes? Inline comment.

Non-blocking

🤔 thinking

Dead http_collector module (crates/trusted-server-cli/src/audit/http_collector.rs:9-79) — entire file is #[allow(dead_code)] with no consumer. Either wire it as a fallback when no browser is found, or delete it. Inline comment.
analyze_html only used by tests (crates/trusted-server-cli/src/audit.rs:60 and crates/trusted-server-cli/src/audit/analyzer.rs:117) — gated #[cfg_attr(not(test), allow(dead_code))]. Move both under #[cfg(test)] or rewrite the tests around analyze_collected_page. Inline comment.
No log:: macros anywhere in the CLI — CLAUDE.md mandates log macros for instrumentation; the CLI only writes to stdout/stderr. Browser audit steps and Fastly API requests would benefit from log::debug! / log::trace! behind RUST_LOG. Today there is no way to introspect what ts provision fastly apply is doing beyond final stdout/stderr.

♻️ refactor

Race condition in env-var tests (crates/trusted-server-cli/src/fastly/auth.rs:166-196) — parallel mutation of FASTLY_API_KEY across threads. Use serial_test::serial or a per-suite Mutex. Inline comment.
Repeated regex compilation (crates/trusted-server-cli/src/audit/analyzer.rs:201, 224) — GTM-[A-Z0-9]+$ is rebuilt on every inline-script and recompiled in two functions. Hoist to a LazyLock<Regex>. Inline comment.
Trim API key before persisting (crates/trusted-server-cli/src/fastly/auth.rs:122-133) — pasted tokens commonly arrive with trailing whitespace/newlines. Inline comment.
format! allocations in classify_party (crates/trusted-server-cli/src/audit/analyzer.rs:158-170) — two allocations per call, run for every asset URL; an allocation-free version using strip_suffix is straightforward. Inline comment.

🌱 seedling

Pin chromiumoxide via workspace deps (crates/trusted-server-cli/Cargo.toml:27) — only direct version pin in this crate; everything else is workspace = true. Inline comment.
ts audit lacks --json — every other validate/status/plan/apply command supports --json; audit doesn't. Consider adding for tooling parity.

⛏ nitpick

config validate --json returns unstructured error blob (crates/trusted-server-cli/src/config.rs:97-99) — multi-line Report debug dump shoved into a single JSON string. Inline comment.
navigator.webdriver spoofing (crates/trusted-server-cli/src/audit/browser_collector.rs:62-71) — bot-evasion technique that's likely unnecessary for auditing sites the operator owns. Either drop or document the specific failure mode it mitigates. Inline comment.
Reused cache-shared-key across wasm and host lanes (.github/workflows/format.yml:30,55, .github/workflows/test.yml:29,65) — distinct keys per target are safer under concurrent runs. Inline comment.
Public items without doc comments — lib.rs:25,156,171, audit.rs:42,52, config.rs:13,19, dev.rs:13, error.rs:4, output.rs:8, fastly/auth.rs:57-75, fastly/api.rs:12-99. CLAUDE.md says every public item must have a doc comment. Most are effectively pub(crate) since the binary lives in the same crate — easiest fix is to downgrade visibility.

CI status

fmt: PASS (verified locally — workflow did not run on this commit)
clippy (workspace, wasm): not run
clippy (CLI, host): FAIL — see audit.rs:70 finding
rust tests (workspace): not run
rust tests (CLI, host): PASS locally (18/18) — workflow did not run on this commit
js tests: not run for this commit
analyze (javascript-typescript): PASS

aram356 · 2026-05-01T00:46:17Z

+}
+
+fn build_audit_outputs(
+    collected: collector::CollectedPage,


🔧 wrench — Clippy fails on host with -D warnings: clippy::needless_pass_by_value. The body only borrows collected (analyze_collected_page(&collected), collected.final_url()), so this should take a reference.

Fix:

fn build_audit_outputs( collected: &collector::CollectedPage, ) -> Result<AuditOutputs, Report<CliError>> {

And update the call site at audit.rs:67 to build_audit_outputs(&collected).

Reproduced locally:

cargo clippy -p trusted-server-cli --target $(rustc -vV | sed -n 's/^host: //p') --all-targets -- -D warnings error: this argument is passed by value, but not consumed in the function body --> crates/trusted-server-cli/src/audit.rs:70:16

aram356 · 2026-05-01T00:46:17Z

+            .build()
+            .change_context(CliError::FastlyApi)?;
+        Ok(Self { client, api_key })
+    }


🔧 wrench — No request/connect timeouts on the Fastly API client. reqwest::blocking::Client has no default timeout, so a stalled Fastly endpoint will wedge ts provision fastly plan/apply indefinitely with no recovery path other than SIGINT.

Fix:

let client = Client::builder() .user_agent("trusted-server-cli/0.1") .connect_timeout(std::time::Duration::from_secs(10)) .timeout(std::time::Duration::from_secs(60)) .build() .change_context(CliError::FastlyApi)?;

aram356 · 2026-05-01T00:46:17Z

+        .user_agent("trusted-server-cli/0.1")
+        .redirect(reqwest::redirect::Policy::limited(10))
+        .build()
+        .change_context(CliError::Audit)?;


🔧 wrench — Same as the Fastly client: no .timeout(...) / .connect_timeout(...). A slow target site will hang the audit. Add explicit timeouts.

aram356 · 2026-05-01T00:46:17Z

+    let url = url::Url::parse(&args.url).map_err(|error| {
+        Report::new(CliError::Arguments)
+            .attach(format!("invalid audit URL `{}`: {error}", args.url))
+    })?;


🔧 wrench — Url::parse accepts arbitrary schemes (file://, data:, chrome://, …). Combined with validate_navigation_response (browser_collector.rs:213-216) returning Ok(()) for any non-http(s) scheme, this lets ts audit file:///etc/passwd navigate Chromium to a local file and then serialize the rendered HTML into the audit artifact written to disk. Headless Chromium permits file:// unless explicitly disabled.

Fix — restrict scheme at parse time:

let url = url::Url::parse(&args.url).map_err(|error| { Report::new(CliError::Arguments) .attach(format!("invalid audit URL `{}`: {error}", args.url)) })?; if !matches!(url.scheme(), "http" | "https") { return Err(Report::new(CliError::Arguments).attach(format!( "`ts audit` only supports http/https URLs, got `{}`", url.scheme() ))); }

aram356 · 2026-05-01T00:46:17Z

+    }
+    if reuse_management_api_key {
+        return Ok(Some(management_api_key.to_string()));
+    }


❓ question — Reusing the management API key as the runtime API key means edge code stores a credential with full Fastly management privileges. If the runtime token leaks (logs, error paths, future bug in the worker), an attacker can mutate Fastly resources, not just sign requests.

Questions:

Is this intended only as a smoke-test convenience, or as a production-supported flow?

If smoke-test only, can --help for --reuse-management-api-key flag the privilege-escalation risk explicitly?

Should this require a confirmation prompt unless --yes is also set, so that operators can't accidentally promote their management token to the edge?

aram356 · 2026-05-01T00:46:17Z

+    if asset_host == page_host
+        || asset_host.ends_with(&format!(".{page_host}"))
+        || page_host.ends_with(&format!(".{asset_host}"))
+    {


♻️ refactor — Two format!(".{page_host}") allocations per call, and this runs once per asset URL. Allocation-free version:

fn host_matches(page_host: &str, asset_host: &str) -> bool { asset_host == page_host || asset_host .strip_suffix(page_host) .is_some_and(|prefix| prefix.ends_with('.')) || page_host .strip_suffix(asset_host) .is_some_and(|prefix| prefix.ends_with('.')) }

aram356 · 2026-05-01T00:46:17Z

+futures = { workspace = true }
+tokio = { workspace = true, features = ["rt-multi-thread", "time"] }
+which = { workspace = true }
+chromiumoxide = "0.9.1"


🌱 seedling — Every other dependency in this crate uses workspace = true; only chromiumoxide = "0.9.1" is pinned at the crate level. Move to [workspace.dependencies] in the root Cargo.toml for consistency and centralized version bumps.

aram356 · 2026-05-01T00:46:17Z

+                path: resolved_path,
+                config_hash: None,
+                errors: vec![format!("{error:?}")],
+            }


⛏ nitpick — vec![format!("{error:?}")] puts a multi-line error_stack::Report debug dump into a single JSON string. Anything parsing the --json output gets an unstructured blob. Consider one entry per attached context layer or a structured {stage, message} shape so downstream tooling can pattern-match.

aram356 · 2026-05-01T00:46:17Z

+            get: () => false,
+        });
+        "#,
+    )


⛏ nitpick — Overriding navigator.webdriver is bot-evasion. For an audit tool intended to run against sites the operator owns, this is unnecessary, and most sites don't gate solely on this flag. Either drop the override or add a // Why: comment explaining the specific failure mode it mitigates so future maintainers don't strip it as a security smell.

aram356 · 2026-05-01T00:46:17Z

+          toolchain: ${{ steps.rust-version-cli.outputs.rust-version }}
+          target: wasm32-wasip1
+          components: "clippy, rustfmt"
+          cache-shared-key: cargo-${{ runner.os }}


⛏ nitpick — Same cache-shared-key: cargo-${{ runner.os }} is reused across the wasm-target workspace lane and the host-target CLI lane (here, line 55, and test.yml lines 29 and 65). Concurrent jobs may contend on cache writes. Distinct keys per target (cargo-${{ runner.os }}-wasm vs -host) are safer.

aram356

Summary

Second-pass review against main. The 4 commits since the prior CHANGES_REQUESTED review are docs-only — none of the 15 prior code findings are addressed. Re-flagging the unresolved ones inline (many of them blocking) and adding new findings: a real CI failure caused by adding scraper to the workspace, a JSON-contract gap in provision apply, and several robustness gaps around ts dev and the headless browser.

Blocking

🔧 wrench

CI fail: shared-dep version mismatch. This PR adds scraper = "0.24.0" (and indirectly bumps derive_more resolution) to root [workspace.dependencies]. scripts/check-integration-dependency-versions.sh (run by the prepare integration artifacts job) fails:
```
Shared dependency version mismatch for derive_more: workspace=2.0.1 integration-tests=2.1.1
Shared dependency version mismatch for scraper: workspace=0.24.0 integration-tests=0.21.0
```
crates/integration-tests/Cargo.toml still pins scraper = "0.21" and resolves derive_more to 2.1.1 in its independent lockfile. Fix: bump the integration-tests crate to scraper = "0.24" and refresh its Cargo.lock so derive_more resolves to 2.0.1.
Clippy still fails on host with -D warnings (audit.rs:70) — see inline.
No timeouts on Fastly API client (api.rs:113) — see inline.
No timeouts on HTTP audit collector (http_collector.rs:15) — see inline.
URL scheme not restricted (lib.rs:218) — see inline.
ProvisionApplyJson.failed_action dead, partial completed_actions discarded on failure (provision.rs:69) — see inline.

❓ question

--reuse-management-api-key privilege escalation risk (lib.rs:370) — see inline.

Non-blocking

🤔 thinking

Format/test workflows are gated to branches: [main]. This PR targets feature/ts-config-in-config-store, so format-rust-cli-host, test-cli-host, the wasm clippy/test lanes, and the JS lanes are all skipped. The clippy failure above will not surface in CI until the parent branch is merged into main. Recommend either rebasing onto main, or temporarily widening the workflow trigger to include the parent feature branch.
Two parallel implementations of the local fastly manifest renderer. crates/trusted-server-cli/src/dev.rs::render_local_fastly_manifest (Rust) and scripts/render-fastly-local-config.py (Python, used by integration tests) emit the same [local_server.config_stores.ts_config_store] section with format = "inline-toml" and the ts-config key. They will drift. Pick one canonical source — e.g., have integration tests shell out to a ts dev --print-manifest mode, or have both paths share a fixture.
Hardcoded --env=local in ts dev (dev.rs:55) — see inline.
--skip-build Wasm path lookup ignores CARGO_TARGET_DIR (dev.rs:66) — see inline.
tokio rt-multi-thread feature unused (Cargo.toml:25) — see inline.
Race window between plan and clone in apply (provision.rs:135) — see inline.
Headless Chromium leak on Ctrl+C / panic (browser_collector.rs:42) — see inline.
Linux find_browser_executable fallback list is empty; error message misleading (browser_collector.rs:285) — see inline.
keyring sync-secret-service requires D-Bus on Linux (Cargo.toml:33) — see inline.
analyze_html is dead outside tests (audit.rs:59, audit/analyzer.rs:117) — see inline.
http_collector module is #[allow(dead_code)] and never called (audit/http_collector.rs:9) — see inline.

♻️ refactor

Race-prone set_var env tests (fastly/auth.rs:170) — see inline.
GTM regex compiled per call (audit/analyzer.rs:201) — see inline.
Token not trimmed before write (fastly/auth.rs:132) — see inline.
host_matches allocates twice per call (audit/analyzer.rs:162) — see inline.
Brittle relative path in STARTER_CONFIG_TEMPLATE (config.rs:11) — see inline.

🌱 seedling

chromiumoxide pinned at crate level instead of workspace (crates/trusted-server-cli/Cargo.toml:27) — see inline.

⛏ nitpick

Report debug dump as a single JSON string (config.rs:99) — see inline.
navigator.webdriver override is unjustified bot-evasion (browser_collector.rs:67) — see inline.
CI cache key collision across wasm and host lanes (.github/workflows/format.yml:30, plus format.yml:55, test.yml:29, test.yml:65) — see inline.
Dead non-http(s) early return in validate_navigation_response (browser_collector.rs:214) — see inline.

CI Status

fmt: not run (workflow gated to main-targeted PRs)
clippy (workspace, wasm): not run
clippy (CLI host): FAIL locally — see audit.rs:70
rust tests (workspace, wasm): not run
rust tests (CLI host): PASS locally (18 tests)
js tests / format / docs format: not run
prepare integration artifacts: FAIL (shared-dep version mismatch)

aram356 · 2026-05-08T17:32:50Z

+}
+
+fn build_audit_outputs(
+    collected: collector::CollectedPage,


🔧 wrench — Clippy fails on host with -D warnings (clippy::needless_pass_by_value). The body only borrows collected (analyze_collected_page(&collected), collected.final_url()), so this should take a reference. This was raised in the prior review and remains unaddressed.

Fix:

fn build_audit_outputs( collected: &collector::CollectedPage, ) -> Result<AuditOutputs, Report<CliError>> {

And update the call site at audit.rs:66 to build_audit_outputs(&collected).

Reproduced locally on claude-opus-4-7 host:

cargo clippy -p trusted-server-cli --target $(rustc -vV | sed -n 's/^host: //p') --all-targets -- -D warnings error: this argument is passed by value, but not consumed in the function body --> crates/trusted-server-cli/src/audit.rs:70:16

Note: the format-rust-cli-host workflow doesn't run for this PR because the workflow is gated to branches: [main] and the PR targets feature/ts-config-in-config-store. The failure is silent in CI but real.

aram356 · 2026-05-08T17:32:50Z

+            .build()
+            .change_context(CliError::FastlyApi)?;
+        Ok(Self { client, api_key })
+    }


🔧 wrench — No request/connect timeouts on the Fastly API client. reqwest::blocking::Client has no default timeout, so a stalled Fastly endpoint will wedge ts provision fastly plan/apply indefinitely. Re-flagging from prior review — unaddressed.

Fix:

let client = Client::builder() .user_agent("trusted-server-cli/0.1") .connect_timeout(std::time::Duration::from_secs(10)) .timeout(std::time::Duration::from_secs(60)) .build() .change_context(CliError::FastlyApi)?;

aram356 · 2026-05-08T17:32:50Z

+        .user_agent("trusted-server-cli/0.1")
+        .redirect(reqwest::redirect::Policy::limited(10))
+        .build()
+        .change_context(CliError::Audit)?;


🔧 wrench — Same as the Fastly client: no .timeout(...) / .connect_timeout(...). A slow target site will hang the audit. Re-flagging from prior review — unaddressed.

Note: this module is also currently #[allow(dead_code)] (see the separate finding above), but it ships with the PR and would hang on first use if wired up.

aram356 · 2026-05-08T17:32:50Z

+            .attach("nothing to do: both --no-js-assets and --no-config were set"));
+    }
+
+    let url = url::Url::parse(&args.url).map_err(|error| {


🔧 wrench — Url::parse accepts arbitrary schemes (file://, data:, chrome://, …). Combined with validate_navigation_response (browser_collector.rs:213-216) returning Ok(()) for any non-http(s) scheme, ts audit file:///etc/passwd would navigate Chromium to a local file and serialize the rendered HTML into the audit artifact written to disk. Headless Chromium permits file:// unless explicitly disabled. Re-flagging from prior review — unaddressed.

Fix — restrict scheme at parse time:

let url = url::Url::parse(&args.url).map_err(|error| { Report::new(CliError::Arguments) .attach(format!("invalid audit URL `{}`: {error}", args.url)) })?; if !matches!(url.scheme(), "http" | "https") { return Err(Report::new(CliError::Arguments).attach(format!( "`ts audit` only supports http/https URLs, got `{}`", url.scheme() ))); }

aram356 · 2026-05-08T17:32:50Z

+    if let Some(value) = env_runtime_api_key {
+        return Ok(Some(value));
+    }
+    if reuse_management_api_key {


❓ question — Reusing the management API key as the runtime API key means edge code stores a credential with full Fastly management privileges. If the runtime token leaks (logs, error paths, future bug in the worker), an attacker can mutate Fastly resources, not just sign requests. Re-flagging from prior review — unaddressed.

Questions:

Is this intended only as a smoke-test convenience, or as a production-supported flow?

If smoke-test only, can --help for --reuse-management-api-key flag the privilege-escalation risk explicitly?

Should this require a confirmation prompt independent of --yes, so operators can't accidentally promote their management token to the edge?

aram356 · 2026-05-08T17:32:51Z

+futures = { workspace = true }
+tokio = { workspace = true, features = ["rt-multi-thread", "time"] }
+which = { workspace = true }
+chromiumoxide = "0.9.1"


🌱 seedling — Every other dependency in this crate uses workspace = true; only chromiumoxide = "0.9.1" is pinned at the crate level. Move to [workspace.dependencies] in the root Cargo.toml for consistency and centralized version bumps. Re-flagging from prior review — unaddressed.

aram356 · 2026-05-08T17:32:51Z

+                path: resolved_path,
+                config_hash: None,
+                errors: vec![format!("{error:?}")],
+            }


⛏ nitpick — vec![format!("{error:?}")] puts a multi-line error_stack::Report debug dump into a single JSON string. Anything parsing the --json output gets an unstructured blob. Consider one entry per attached context layer or a structured {stage, message} shape so downstream tooling can pattern-match. Re-flagging from prior review — unaddressed.

aram356 · 2026-05-08T17:32:51Z

+            get: () => false,
+        });
+        "#,
+    )


⛏ nitpick — Overriding navigator.webdriver is bot-evasion. For an audit tool intended to run against sites the operator owns, this is unnecessary, and most sites don't gate solely on this flag. Either drop the override or add a // Why: comment explaining the specific failure mode it mitigates so future maintainers don't strip it as a security smell. Re-flagging from prior review — unaddressed.

aram356 · 2026-05-08T17:32:51Z

+    target_url: &Url,
+    navigation_response: ArcHttpRequest,
+) -> Result<(), Report<CliError>> {
+    if !matches!(target_url.scheme(), "http" | "https") {


⛏ nitpick — Once URL scheme is restricted upstream (see the lib.rs:218 blocking finding), this if !matches!(target_url.scheme(), "http" | "https") { return Ok(()); } becomes unreachable. Remove the early-return then so non-http(s) URLs can never reach validate_navigation_response.

aram356 · 2026-05-08T17:32:51Z

+          toolchain: ${{ steps.rust-version-cli.outputs.rust-version }}
+          target: wasm32-wasip1
+          components: "clippy, rustfmt"
+          cache-shared-key: cargo-${{ runner.os }}


⛏ nitpick — Same cache-shared-key: cargo-${{ runner.os }} is reused across the wasm-target workspace lane (line 30) and this new host-target CLI lane, plus test.yml lines 29 and 65. Concurrent jobs may contend on cache writes. Distinct keys per target (cargo-${{ runner.os }}-wasm vs -host) are safer. Re-flagging from prior review — unaddressed.

ChristianPavilonis added 2 commits April 28, 2026 16:02

Add trusted-server CLI provisioning workflow

74146e9

Add Chromium-backed ts audit collection

abd0c0d

ChristianPavilonis linked an issue Apr 29, 2026 that may be closed by this pull request

Trusted server CLI implementation #670

Open

ChristianPavilonis changed the base branch from main to feature/ts-config-in-config-store April 29, 2026 19:27

aram356 assigned ChristianPavilonis Apr 29, 2026

ChristianPavilonis changed the title ~~Add Chromium-backed ts audit collection~~ Add the Trusted Server CLI Apr 29, 2026

aram356 requested changes May 1, 2026

View reviewed changes

ChristianPavilonis added 3 commits May 5, 2026 10:47

Add CLI guide documentation

04c5e27

Document CLI Cargo aliases

de2202a

Add Fastly provisioning map docs

d6e056b

ChristianPavilonis requested review from jevansnyc and prk-Jr May 5, 2026 20:36

aram356 requested changes May 8, 2026

View reviewed changes

Conversation

ChristianPavilonis commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer quick links

Manual reviewer test plan

1. Set up local CLI access

2. Authenticate to Fastly

3. Audit the test site

4. Validate the generated config

5. Preview Fastly provisioning

6. Apply provisioning to the test service

7. Re-run plan to check idempotency

8. Useful docs while reviewing output

Summary

Changes

Closes

Automated test plan

Checklist

Uh oh!

aram356 left a comment

Choose a reason for hiding this comment

Summary

Blocking

🔧 wrench

❓ question

Non-blocking

🤔 thinking

♻️ refactor

🌱 seedling

⛏ nitpick

CI status

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aram356 left a comment

Choose a reason for hiding this comment

Summary

Blocking

🔧 wrench

❓ question

Non-blocking

🤔 thinking

♻️ refactor

🌱 seedling

⛏ nitpick

CI Status

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

ChristianPavilonis commented Apr 29, 2026 •

edited

Loading