You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
add a shared, static configuration-audit library for Codex CLI, the VS Code agent host, and GitHub Copilot for VS Code
expose human-readable and versioned JSON CLI reports with target aliases, layered configuration provenance, redacted evidence, and CI failure thresholds
add the native Config Audit dashboard with findings, inventory, manual checks, limitations, and raw-report views
document the report schema, rule coverage, usage, exit codes, and read-only execution boundary
dashboard TypeScript type-check and Vite production build
Playwright Chromium smoke test using real CLI-generated audit bundles through a Tauri IPC mock: merged VS Code/Copilot inventory rendered without [object Object], target switching to Codex succeeded, and no page exceptions or failed requests occurred
Notes
The audit is read-only and does not execute agents, MCP servers, hooks, skills, plugins, extensions, or command rules.
Reviewed the full diff against a local checkout of feature/codex-config-audit. Overall this is well-built: rule IDs are stable, evidence is redacted before serialization, the fingerprint hashing is length-prefixed, per-setting provenance genuinely resolves the layer that won, and the read-only boundary holds (inventory_does_not_execute_mcp_commands is a good guard). cargo clippy -p gensee-crate-config-audit --all-targets -- -D warnings is clean.
Two things look blocking.
1. Tests fail on macOS — 5 of 28, not 28 passing
cargo test -p gensee-crate-config-audit
test result: FAILED. 23 passed; 5 failed
codex::tests::project_provider_key_is_reported_as_ignored
codex::tests::effective_findings_use_the_setting_layer_that_won
codex::tests::unreadable_discovered_source_makes_assessment_partial
vscode::tests::effective_findings_use_per_setting_provenance
vscode::tests::discovered_control_plane_read_and_parse_errors_are_reported
All five come from canonical_or_original() resolving $TMPDIR (/var/folders/… → /private/var/folders/…). .github/workflows/ci.yml is ubuntu-only, so CI won't catch this, but every macOS dev will.
2. project_is_trusted can never match a non-canonical workspace path (high)
let candidates = [display_path(workspace),
workspace.to_string_lossy().to_string(),];
display_path(p)isp.to_string_lossy().to_string() — the two candidates are always the identical string. And workspace was already canonicalized at codex.rs:54, so the only form ever compared against the [projects."…"] keys is the canonical one.
Codex records the trust key using the path the user launched from, so on macOS (/tmp/x → /private/tmp/x), a symlinked home, or a symlinked checkout, a trusted .codex/config.toml is silently treated as untrusted: its settings are never merged, never evaluated, and the report emits a misleading "not applied because the workspace is not explicitly trusted" limitation. Under-reporting is the worst failure mode for an auditor.
This is also the root cause of two of the five test failures — project_provider_key_is_reported_as_ignored is an accurate repro. Fix by passing both options.workspace and the canonicalized form into the candidate list. (The other three failures are tests comparing pre-canonical paths against canonical report paths.)
Other findings
mutable_package_launcher fires on correctly pinned servers — vscode.rs:1684-1703. Every non-flag argument is treated as a package spec, so npx -y @scope/server@1.2.3 /Users/me/projects is flagged VSC-MCP-007 (High) because "projects" has no @. codex.rs:2207 guards this with looks_like_path; the VS Code copy dropped it. npm/yarn/pnpm are also in the launcher list, so subcommands like exec/run/start trigger it. Suggest inspecting only the first non-flag argument, plus the looks_like_path guard.
Duplicate extension rows in the merged dashboard view — vscode.rs:253-255 keeps Copilot extensions in the Copilot leaf's inventory while the agent-host leaf keeps all of them; mergeReports in dashboards/src/pages/ConfigAudit.tsx flatMaps both into a <Table rowKey="path">. When Copilot is installed that is a duplicate React key and a double-counted "Extensions" stat. Sources and findings are already target-prefixed on merge; extensions/skills/MCP are not.
Exit code 2 is too easy to trigger — crate/gensee-crate-cli/src/config_audit.rs:131-144 treats any source error as an incomplete audit. read_limited_text errors on any file over 256 KiB, and discover_instruction_files walks the whole workspace for AGENTS.md. A single large instruction file anywhere in a monorepo turns a successful audit into an operational failure in CI. Worth separating "a layer feeding the effective config failed" from "one discovered artifact was skipped" — the latter reads better as a limitation.
CAX-ENV-002 doesn't know about shell_environment_policy.filters — the current config reference lists filters as canonical and include_only/exclude as legacy. A config using inherit = "all" with a filters allowlist gets a false positive at codex.rs:620. (Checked the live reference for this section — the ignore_default_excludes default of true that CAX-ENV-001 assumes is correct as written.)
Docs describe a check that isn't implemented — docs/config-audit.md:104 lists "on Windows, the workspace VS Code setting that selects WSL execution" under the Codex CLI target; nothing in codex.rs implements it. Separately, CONFIG_REFERENCE in codex.rs:16 points at developers.openai.com/codex/config-reference/, which now 308s to learn.chatgpt.com/docs/config-file/config-reference (what the docs page itself links). Emitted references should use the destination.
Hardcoded inventory fields render as determinations — vscode.rs:827-833 always sets enabled: true and has_tool_allowlist: false, and review_state is universally "unknown". The dashboard renders these as "YES" / "UNSCOPED" columns, which reads as a finding about the server rather than "not modeled for this provider". Either omit the columns for VS Code targets or make the not-modeled state explicit.
Reuse — roughly 200 lines are duplicated verbatim between codex.rs and vscode.rs: make_finding, finding_fingerprint, hash_fingerprint_part, sort_findings, summarize, source_for, read_limited_text, display_path, hash_bytes, canonical_or_original, normalize_secret_key, secret_key_name, the loopback/insecure_remote_http pair, collect_json_commands, frontmatter_value, find_named_files, and both MAX_* constants. They have already drifted (the pinning check above; only codex.rs skips .git/node_modules/target during traversal). A shared common.rs before a third provider adapter lands would stop the next divergence.
Minor — bare gensee audit and gensee audit config both default to the codex target only, which the usage text ("Usage: gensee audit config") reads as broader than it is; and vscode_user_data isn't directory-validated in the Tauri command the way codex_home is.
Reviewed 34b708c against its parent. Tests now pass on macOS — 31 passed, 0 failed — and cargo clippy -p gensee-crate-config-audit --all-targets -- -D warnings is clean.
What's fixed
project_is_trusted now takes both the original and canonical workspace (codex.rs:335), with a direct unit test covering the /tmp ↔ /private/tmp pair.
mutable_package_launcher is rewritten to look only at the package position, strip the scope before the @ check, and gate npm/pnpm/yarn on exec/dlx/x. Traced it against the cases from the earlier review — npx -y @scope/server@1.2.3 /Users/me/projects no longer fires, @scope/server still does.
Exit code 2 is scoped to source kinds that actually determine the effective config (config_audit.rs:156), with a test proving an oversized AGENTS.md no longer turns into an operational failure.
Merged dashboard inventory dedupes skills/MCP/extensions by key, so the duplicate-rowKey case is gone.
Hardcoded VS Code inventory fields are now hidden behind modelsInventoryState, with an explicit "not modeled" alert rather than rendering assumptions as data. Good call.
Docs WSL claim removed, CONFIG_REFERENCE repointed to learn.chatgpt.com, vscode_user_data validated in the Tauri command.
New issues
1. Canonicalization was removed entirely, not just for the trust key (medium, regression).audit_codex and audit_vscode_scope now carry the raw option paths through every derived path and report field. Verified:
Same underlying config, audited via absolute vs. relative --workspace/--codex-home, produces a different target.workspace, different evidence sources, and different fingerprints. That undercuts the documented promise that fingerprints let automation track findings across runs. The narrower fix is to keep canonical_workspace for every derived path and report field, and use the raw options.workspaceonly as an extra trust-key candidate.
2. Exclude-only filters now suppress CAX-ENV-002 (low–medium, new false negative).has_environment_filter at codex.rs:620 is satisfied by the mere presence of filters. Verified — inherit = "all" with filters = { AWS_SECRET_ACCESS_KEY = "exclude" } emits nothing, even though the full environment minus one variable is still inherited. Per the reference, only include entries create an allowlist; the check should require at least one.
Relatedly, the new test canonical_environment_filters_prevent_full_inheritance_warning uses filters = ["PATH", "HOME"] — an array, not the documented map<string, include | exclude>. It passes only because the code checks existence, so it never exercises the real shape.
3. hook_commands and friends switched from sum to Math.max (low). Correct today only because the Copilot leaf zeroes its own inventory. If a future bundle resolves to two leaves that each count distinct artifacts, max silently under-reports — a max of counts isn't a union. Since the collections next to it are now deduped by key, deriving the counts from the deduped lists would be more robust.
4. looks_like_path is now duplicated with divergent semantics (low). The new one in vscode.rs:1729 checks ~ and X: but not a bare path separator; codex.rs:2337 checks the separator but not ~. And executable_dependency_is_unpinned in codex.rs did not get the positional-argument fix, so npx -y pkg@1.2.3 --root data still false-positives on the Codex side. The two package-pinning checks have diverged again, just in the other direction now.
5. Reuse is only partly addressed.common.rs picked up four helpers plus MAX_TEXT_FILE_BYTES, but make_finding, finding_fingerprint, hash_fingerprint_part, sort_findings, summarize, source_for, normalize_secret_key, secret_key_name, insecure_remote_http, collect_json_commands, and now looks_like_path still exist in both modules.
6. Unrelated tclone.rs refactor in this commit (nit). 112/109 lines of pure clippy cleanup (needless_range_loop, type_complexity). It reads as behavior-preserving — the clones.len() != copies guard above makes .take(copies) equivalent to the old index loop — but it's unrelated to the review feedback and makes the commit harder to review or revert on its own.
Everything from the previous round checks out (36 tests pass on macOS, clippy clean, fingerprint stability verified against the symlink/relative-path repro). Two things left.
1. Scheme-less MCP url values bypass credential redaction (low–medium, pre-existing).vscode::url_has_credentials returns false whenever Url::parse fails, and the endpoint is then copied verbatim into inventory.mcp_servers[].endpoint. Verified against the serialized report:
The secret lands in the JSON report and in the dashboard's Endpoint column. codex::endpoint_contains_secret has a fallback for unparsable input, but it keys on "://", so it misses the same cases. Since redaction is the property this feature stakes its credibility on, it should fail closed — treat an unparsable endpoint as potentially credentialed and redact it, in one shared helper.
2. Nit:codex::is_loopback_url(&str) still shadows common::url_host_is_loopback(&Url), which is private. Exposing a single is_loopback_url from common would finish the consolidation.
The core fix is right: endpoint_contains_secret now lives in common.rs, fails closed on unparsable input, and is_loopback_url / url_has_credentials are consolidated. 40 tests pass on macOS, clippy clean, and both regression tests reproduce the exact strings from the previous round.
Two new problems, both from that fail-closed value being reused for more than redaction.
1. VSC-MCP-005 now fires on a missing scheme, with no credentials present (high).vscode.rs:824 uses endpoint_contains_secret for endpoint_has_credentials, which drives both the redaction and the finding. A plain typo:
There are no credentials anywhere in that config. The finding asserts as confirmed that credentials are embedded, tells the user they "can leak through source control, logs, backups," and fails CI under --fail-on high. Redaction should fail closed; the claim shouldn't inherit that. Two options: split into endpoint_must_be_redacted() (fail-closed, drives redaction) and endpoint_has_credentials() (proven, drives the finding), or keep one helper and emit VSC-MCP-005 as Potential with a "the endpoint could not be parsed" description when it is inferred rather than proven.
2. endpoint_key matches user-controlled server IDs, over-redacting unrelated evidence (medium).codex.rs:2157 tests normalize_secret_key(key).contains("url") against the whole dotted path, and the path embeds the MCP server ID. With a server named url-fetcher:
CAX-MCP-004 | key="mcp_servers.url-fetcher.command" value="<redacted>" (should be "bash")
CAX-MCP-001 | key="mcp_servers.url-fetcher.enabled_tools" value="<redacted>" (should be "<unset>")
Not a leak — it is over-redaction — but it strips the evidence that makes the finding actionable, and any server whose name contains url (url-fetcher, curl-proxy) triggers it. Matching the final segment instead of the full path fixes it and still covers every current call site: mcp_servers.{id}.url → url ✓, otel.exporter.endpoint → endpoint ✓, mcp_oauth_callback_url (no dots) ✓, mcp_servers.url-fetcher.command → command ✗.
fnendpoint_key(key:&str) -> bool{let last = key.rsplit('.').next().unwrap_or(key);let last = normalize_secret_key(last);
last.contains("url") || last.contains("endpoint")}
Both items from the last round are fixed exactly as suggested (42 tests pass on macOS, clippy clean):
endpoint_key now matches only the final dotted segment, so a server named url-fetcher no longer poisons unrelated evidence — the new test asserts CAX-MCP-004 shows "bash" and CAX-MCP-001 shows "<unset>".
The signal is split into endpoint_must_be_redacted (fail-closed, serialization) and endpoint_has_credentials (proven, findings), with a doc comment stating which is for what. VSC-MCP-005 no longer fires on a parse failure, and the old test that asserted the false positive was flipped to assert its absence.
Three small leftovers, all in the same "don't assert what you haven't proven" vein.
1. The placeholder still claims credentials (low). Redaction is now fail-closed but the label isn't. Same input, both providers:
The finding was correctly downgraded to nothing, but the string the user actually reads in the dashboard's Endpoint column still says "credential". Use the neutral <redacted-url> when redaction was fail-closed, and reserve <redacted-credential-url> for the case endpoint_has_credentials proved — which also makes the two providers agree.
2. A malformed MCP endpoint now produces no finding at all (low). Before this commit it produced a wrong High; now it produces silence, and the user just sees a redacted placeholder with no explanation. An info/Potential finding along the lines of "MCP endpoint could not be parsed; transport, host, and credential posture were not evaluated" would close the loop — and it is substantively useful, since an unparsable url also means the TLS and loopback checks silently did not run on that server.
3. Nit: shadowing in inspect_mcp_server (vscode.rs:824-831).
let endpoint_must_be_redacted = raw_endpoint.as_deref().is_some_and(endpoint_must_be_redacted);let endpoint_has_credentials = raw_endpoint.as_deref().is_some_and(endpoint_has_credentials);
Correct as written — the imported fn is still in scope on the right-hand side — but each local then shadows its function for the rest of the body. Worth renaming the locals (redact_endpoint / has_credentials) so the two concepts stay visually distinct in the very function that exists to keep them apart.
8c78597 — all leftovers closed, one optional enhancement
42 tests pass on macOS, clippy clean. All three items from the last round are resolved:
endpoint_display_value centralizes the label choice — <redacted-credential-url> only when endpoint_has_credentials proved it, neutral <redacted-url> when redaction merely failed closed. Both providers share it now, so the Codex/VS Code divergence is gone.
CAX-MCP-009 / VSC-MCP-010 (Info, Potential) report the unparsable endpoint with the wording "transport, host, and credential posture were not evaluated," and both are in the docs rule tables.
Shadowing renamed to redact_endpoint / has_credentials, and the insecure_remote_http evidence now picks its label from has_credentials rather than the fail-closed flag.
The sanitize_evidence_value restructure into if secret_like … else if endpoint_key … is a real improvement too — endpoint evidence now gets the specific label instead of a generic <redacted>.
One residual, worth considering as an enhancement
A genuine embedded credential written without a scheme is now reported only as Info. Verified:
Nothing leaks, so this is not a defect in what the last two commits set out to fix — but a plaintext credential sitting in mcp.json now surfaces as "could not be parsed" rather than as the High-severity credential exposure it actually is. That is the flip side of correctly refusing to assert credentials on parse failure.
A scheme-recovery parse would let endpoint_has_credentials prove these without asserting anything false:
That is: when Url::parse fails and the value carries no scheme, retry once against https://{value.trim_start_matches('/')}; if that parses and shows credentials, it is proven. The third case stays Info, which is the right outcome. Both leak examples from the original report are exactly these two recoverable forms, so this would close the detection gap that motivated the sub-thread.
Otherwise everything raised across these rounds is resolved.
42 tests pass, clippy clean. The recovery is implemented carefully: has_explicit_url_scheme prevents mangling values that already declare a scheme, the <redacted-*> sentinel short-circuit keeps endpoint_display_value idempotent, and the doc table now distinguishes "not a valid absolute URL" from "credentials undetectable." The two target forms work:
The password goes into inventory.mcp_servers[].endpoint and the serialized report unredacted, with no finding at all — not even the Info one.
Cause: Url::parsesucceeds on that string (scheme admin, cannot-be-a-base, host() == None), so endpoint_must_be_redacted returns false and parse_endpoint_for_credential_detection never reaches the recovery branch. has_explicit_url_scheme would also refuse it, since admin is a syntactically valid scheme. This is the same leak class the sub-thread started on, and dropping the // is arguably the more natural way to typo it.
host().is_none() is a clean discriminator — an MCP url with no host is never valid — so gating on "parsed but host-less" rather than "failed to parse" closes it:
fnparse_endpoint_for_credential_detection(value:&str) -> Option<Url>{ifletOk(url) = Url::parse(value){if url.host().is_some(){returnSome(url);}// parsed as an opaque/cannot-be-a-base URL: fall through to recovery}
...
}
endpoint_must_be_redacted and endpoint_is_parseable need the same host check, otherwise redaction still will not fire.
2. VS Code input variables are reported as embedded credentials (medium)
${input:...} is the documented way to keep secrets out of mcp.json — VSC-MCP-005's own remediation says "Use VS Code input variables or OAuth instead of URL credentials." So the rule fires on the fix it recommends, at High/Confirmed, and the inventory hides a harmless placeholder behind a credential label.
parsed_url_has_credentials looks only at the presence of a username/password/secret-named query key, never at the value. json_contains_secret already solves this with secret_literal(), which excludes ${, env:, and input: — applying that same check to the userinfo and query values would fix it. This one predates the commit for scheme-ful URLs, but the recovery now extends it to scheme-less forms too.
Both go through the shared endpoint_display_value / endpoint_has_credentials in common.rs, so neither is VS Code-specific — Codex mcp_servers.<id>.url behaves the same.
8c94a20 — userinfo is still evaluated as one string
45 tests pass, clippy clean. Both items from the last round are fixed:
endpoint_is_parseable now requires a host, and parse_endpoint_for_credential_detection falls through to recovery on a parsed-but-hostless URL — so admin:do-not-leak@internal.example/mcp is redacted and flagged, with a test row added for it.
secret_value_is_literal / runtime_secret_reference exclude ${…}, env:, input: (case-insensitively, an improvement over the old secret_literal), applied to percent-decoded userinfo and per-pair query values. secret_literal moved to common.rs so json_contains_secret shares it.
The new percent-encoding dependency adds no package to Cargo.lock — it was already in the tree under url — so it is just a direct edge.
Two problems remain in url_userinfo_has_literal_credentials, both from folding username and password into one string.
1. A literal password leaks when the username is a placeholder (medium–high)
do-not-leak is a real literal password and it lands verbatim in the report with no finding. The concatenation format!("{}:{password}", username) means a ${ anywhere in the userinfo suppresses detection of a genuine literal in the other half — the placeholder shields the secret sitting next to it.
2. A bare username is reported as embedded credentials (low–medium)
Userinfo with no password contains no secret. High/Confirmed "MCP endpoint embeds credentials" overclaims, and redacting the value hides the host, which is legitimate inventory data.
Both fall out of evaluating the components separately
That gives: ${input:user}:do-not-leak → literal password → flagged and redacted; admin:${input:token} → placeholder password → clean (already correct today); admin@host → no password → no credential claim. If you do want bare usernames surfaced, a separate Low/Potential "endpoint embeds a username" rule keeps the VSC-MCP-005 claim accurate rather than stretching it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
cargo test -p gensee-crate-config-audit— 28 passedcargo test -p gensee-crate-cli— 386 passedcargo clippy -p gensee-crate-config-audit --all-targets -- -D warningscargo fmt --all -- --checkgit diff --check[object Object], target switching to Codex succeeded, and no page exceptions or failed requests occurredNotes
The audit is read-only and does not execute agents, MCP servers, hooks, skills, plugins, extensions, or command rules.