fix(wasm): run leak scan before credential injection in tools wrapper

[nick-stebbings]

Summary

The tools WASM wrapper runs LeakDetector::scan_http_request() on HTTP request
headers after inject_host_credentials() has inserted real secrets (e.g.
xoxb- Slack bot tokens). This causes the leak detector to flag the tool's own
legitimate outbound API calls as secret exfiltration.

• Move leak scan to run on raw_headers before any credential injection
• Add regression test test_leak_scan_runs_before_credential_injection

This is the same class of bug fixed in #421 (dc7d9cc), which corrected the
ordering in channels/wasm/wrapper.rs — the tools wrapper was missed.

BEFORE: parse → inject_credentials → inject_host_credentials → scan (WRONG)
AFTER:  parse → scan → inject_credentials → inject_host_credentials (CORRECT)

Fixes false-positive leak blocks on any WASM tool using host-based credential injection.

Test plan

• cargo test --lib — 2757 tests pass
• cargo clippy --all --tests — zero warnings
• cargo fmt -- --check — clean
• New test: test_leak_scan_runs_before_credential_injection

🤖 Generated with Claude Code

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a critical issue in the WASM tools wrapper where the leak detector was incorrectly flagging legitimate outbound API calls as secret exfiltration. The problem stemmed from the leak scan occurring after sensitive host credentials, such as Slack bot tokens, had been injected into the HTTP request headers. By reordering the process to perform the leak scan on the raw headers before any credential injection, the change eliminates these false positives, ensuring that WASM tools can reliably make authenticated API calls without being erroneously blocked.

Highlights

• Reordered Leak Scan: The leak detection process in the WASM tools wrapper was reordered to run on raw HTTP request headers before any host credentials are injected.
• Prevented False Positives: This change resolves false-positive secret leak blocks that occurred when legitimate outbound API calls were flagged due to host-injected secrets (e.g., Slack bot tokens).
• Added Regression Test: A new test, test_leak_scan_runs_before_credential_injection, was introduced to ensure the correct ordering of leak scanning and credential injection.

Changelog

• src/tools/wasm/wrapper.rs
  • Moved the LeakDetector::scan_http_request call to occur before inject_host_credentials.
  • Removed the redundant LeakDetector::scan_http_request call that was previously executed after credential injection.
  • Added a new test test_leak_scan_runs_before_credential_injection to validate the fix.

Activity

• No human activity has been recorded on this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request correctly fixes a bug where the leak detector was running after credential injection, which caused false positives. The change moves the leak scan to run on the raw headers before any host credentials are injected, which is the correct approach. A good regression test has also been added to verify this behavior. I've suggested a performance improvement to avoid unnecessary allocations during the leak scan.

[nick-stebbings]

Fixed cargo fmt CI failure in 68b3b5b (closure formatting in leak scan loop).

[zmanian]

Review: fix(wasm): run leak scan before credential injection

Clean, well-scoped security fix. The ordering change is correct -- scanning pre-injection headers prevents false positives from host-injected tokens (like xoxb- Slack tokens).

Strengths:

• Good commit progression: fix -> perf optimization (inline scan) -> fmt cleanup
• Regression test clearly demonstrates both the pre-injection pass and post-injection block
• Using scan_and_clean() inline avoids unnecessary Vec allocation per HTTP request
• Consistent with the channels wrapper fix from #421

Minor notes:

• The injected_url variable name is slightly confusing in the pre-injection context (it's the URL that will be injected into, not one that has been injected). Consider renaming to raw_url or just url in a follow-up, but not blocking.

LGTM -- approve.

[zmanian]

Re-review: fix(wasm): run leak scan before credential injection

The ordering fix is correct. Approving.

Ordering analysis

The tools wrapper has a two-phase credential injection pipeline:

1. inject_credentials() -- resolves WASM-declared placeholders (e.g., {TELEGRAM_BOT_TOKEN}) that the WASM module explicitly templated into its request. The WASM module knows about these by design.
2. inject_host_credentials() -- injects host-side secrets (Bearer tokens, API keys) that the WASM module never sees. These are resolved by the host based on the target URL's hostname.

The leak scan now sits between phases 1 and 2, which is the correct position:

• Phase 1 credentials are WASM-declared, so scanning after phase 1 catches any exfiltration attempt where the WASM module smuggles a known secret into an unexpected field (e.g., encoding a token into a query param to a different domain).
• Phase 2 credentials are host-injected and invisible to WASM -- scanning these would produce the false positives described in the PR.

This matches the architecture documented in leak_detector.rs (line 22: Allowlist -> Leak Scan -> Credential Injector) and is consistent with the channels wrapper fix in #421.

Code quality

• The inline scan_and_clean() approach avoids the Vec allocation from the original code and provides more specific error messages (URL vs header vs body). Good improvement over the Gemini-suggested version that was adopted.
• Comment explains the "why" clearly.

Test coverage

The regression test is well-structured: it demonstrates that pre-injection headers pass the scan while post-injection headers (with a real xoxb- token) are blocked. This directly validates the ordering invariant.

Note: the test uses scan_http_request() with Vec allocation (the old API), not the inline scan_and_clean() pattern used in production. This is fine for a regression test -- it's testing the detector's behavior, not the allocation strategy.

Minor note (non-blocking, carry forward from previous review)

The variable injected_url at the scan site is the URL after inject_credentials() (phase 1) but before inject_host_credentials() (phase 2). The name "injected" refers to WASM-placeholder injection, which is accurate but could be confusing in the context of the leak scan comment that says "before credential injection." Consider renaming to url_with_wasm_creds or similar in a follow-up for clarity.

Verdict

The security fix is correct, the test coverage is adequate, and the code quality is good. No unwrap/expect in the changed production paths (the existing unwrap_or_default() on line 280 is pre-existing and is a safe default for header parsing failure).

LGTM.