Skip to content

fix(wasm): run leak scan on pre-injection headers in channel callbacks - #1377

Merged
ilblackdragon merged 5 commits into
nearai:stagingfrom
nick-stebbings:pr/wasm-channel-leak-scan-order
Apr 19, 2026
Merged

ilblackdragon merged 5 commits into
nearai:stagingfrom
nick-stebbings:pr/wasm-channel-leak-scan-order

Conversation

@nick-stebbings

Copy link
Copy Markdown
Contributor

Summary

Run the leak detector on original WASM-provided headers before credential injection in channel HTTP callbacks. This is the companion fix to #791 (merged) which fixed the same bug in the tools wrapper.

Problem

The WASM channel host's http_request handler calls inject_credentials() to replace placeholder values (e.g. {SLACK_BOT_TOKEN}) with real secrets, then runs the leak scan. Host-injected credentials trigger the leak detector, blocking legitimate channel callbacks like Slack chat.postMessage.

Fix

Scan the raw WASM-provided headers (pre-injection) instead of the post-injection headers. WASM never sees the real token values, so the pre-injection state is the correct input for leak detection. Matches the existing pattern in src/tools/wasm/wrapper.rs (PR #791).

Test plan

  • cargo test --lib passes (3150 tests)
  • cargo clippy --all --all-features clean
  • cargo fmt --check clean
  • Manual: Slack bot responds without leak scan false positive on xoxb- token

🤖 Generated with Claude Code

@github-actions github-actions Bot added scope: channel/wasm WASM channel runtime size: S 10-49 changed lines risk: medium Business logic, config, or moderate-risk modules labels Mar 18, 2026
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! 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 addresses an issue where the leak detector in WASM channel HTTP callbacks would incorrectly flag host-injected credentials as potential leaks. By adjusting the timing of the leak scan to occur on the raw, pre-injection headers, the change ensures that only WASM-provided values are checked, thereby eliminating false positives and allowing legitimate channel callbacks to function correctly.

Highlights

  • Prevented Leak Detector False Positives: The leak detector in WASM channel HTTP callbacks now scans the original headers provided by WASM before any host-side credential injection occurs. This resolves false positives caused by host-injected tokens (e.g., Slack bot tokens) triggering the detector.
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.

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. ↩

@github-actions github-actions Bot added the contributor: experienced 6-19 merged PRs label Mar 18, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses a potential security vulnerability in the WASM channel wrapper by ensuring that the leak detector scans the original WASM-provided headers before any credential injection occurs. This prevents false positives caused by host-injected credentials triggering the leak detector. The changes involve modifying the http_request function to extract and scan the raw headers before credential injection, aligning with the approach used in the tools wrapper. The test plan includes unit tests and manual testing to verify the fix.

Comment thread src/channels/wasm/wrapper.rs Outdated
Comment thread src/channels/wasm/wrapper.rs Outdated
Comment thread src/channels/wasm/wrapper.rs Outdated
@nick-stebbings
nick-stebbings force-pushed the pr/wasm-channel-leak-scan-order branch from f9c2dae to b28f65d Compare March 19, 2026 19:46
@github-actions github-actions Bot added scope: docs Documentation scope: dependencies Dependency updates labels Mar 19, 2026
@nick-stebbings
nick-stebbings force-pushed the pr/wasm-channel-leak-scan-order branch from b28f65d to ec4267a Compare March 19, 2026 21:16
@nick-stebbings

Copy link
Copy Markdown
Contributor Author

Addressed Gemini review feedback in ec4267a:

  • Malformed headers JSON: Replaced unwrap_or_default() with unwrap_or_else that logs a warning before falling back to empty headers. Malformed JSON from WASM is now visible in logs.
  • Scope block: Kept intentionally — scopes the raw headers lifetime before credential injection to make the scan-before-inject ordering clear.

All checks pass (clippy, fmt, tests).

@nick-stebbings
nick-stebbings force-pushed the pr/wasm-channel-leak-scan-order branch from 0b00bfa to e578ced Compare March 21, 2026 21:51
@nick-stebbings
nick-stebbings force-pushed the pr/wasm-channel-leak-scan-order branch 2 times, most recently from aa28202 to 75b08ee Compare March 28, 2026 07:40
@github-actions github-actions Bot added size: M 50-199 changed lines and removed size: S 10-49 changed lines labels Mar 28, 2026
@nick-stebbings
nick-stebbings force-pushed the pr/wasm-channel-leak-scan-order branch from 75b08ee to 24c25f7 Compare March 28, 2026 07:46

@ilblackdragon ilblackdragon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: fix(wasm): run leak scan on pre-injection headers in channel callbacks

Must-fix

  1. Eliminate double-parse of headers_json — the code parses headers_json twice: once at line 340-341 for the existing flow, and again in the new block for the leak scan. The tools wrapper (src/tools/wasm/wrapper.rs:296-328) avoids this by scanning raw_headers BEFORE consuming it with into_iter(). Restructure to match:
let raw_headers = serde_json::from_str(&headers_json).unwrap_or_default();
// Leak scan on raw WASM-provided values
leak_detector.scan_http_request(&url, &header_vec, body.as_deref())?;
// THEN inject credentials
let headers = raw_headers.into_iter()
    .map(|(k, v)| (k.clone(), self.inject_credentials(&v, ...)))
    .collect();
  1. Misleading comment — says "ORIGINAL WASM-provided values (before ANY credential injection)" but the URL is post-template-injection (injected_url). Fix to say "before host credential injection" or "pre-host-injection."

Should-fix

  1. Migrate import — per CLAUDE.md, "new code should import from ironclaw_safety directly." Both the production code and test use crate::safety::LeakDetector. Migrate to ironclaw_safety::LeakDetector.

  2. Remove unnecessary block scope — the { ... } around the leak scan provides no scoping benefit.

  3. Remove raw_url_for_scan alias — it's just &url with no transformation.

nick-stebbings and others added 3 commits March 31, 2026 22:00
The WASM channel host's http_request handler was scanning request headers
AFTER inject_credentials() replaced placeholder values (e.g. {SLACK_BOT_TOKEN})
with real secrets. This caused the leak detector to flag host-injected
credentials as potential leaks, blocking legitimate WASM channel callbacks.

Run the leak scan on the original WASM-provided headers (before any
credential injection) so host-injected tokens never appear in the scan.
WASM never sees the real values, so scanning the pre-injection state is
correct. Matches the existing pattern in src/tools/wasm/wrapper.rs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Proves that scanning post-injection headers triggers a false positive
on host-injected xoxb- tokens, confirming the fix must scan WASM-provided
headers before credential injection.
…igrate import

- Eliminate double-parse of headers_json: parse once, scan raw headers,
  then inject credentials (matches tools wrapper pattern)
- Fix misleading comment: URL has template substitution but not yet
  host credential injection (was "before ANY credential injection")
- Migrate import to ironclaw_safety::LeakDetector per CLAUDE.md
- Remove unnecessary block scope around leak scan
- Remove raw_url_for_scan alias (just use &url directly)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nick-stebbings
nick-stebbings force-pushed the pr/wasm-channel-leak-scan-order branch from 24c25f7 to c594003 Compare March 31, 2026 20:05
@nick-stebbings

Copy link
Copy Markdown
Contributor Author

Addressed all review feedback from @ilblackdragon in c594003, rebased onto latest staging (78e448df):

Must-fix (both resolved)

  1. Double-parse eliminated — headers_json is now parsed once into raw_headers. Leak scan runs on the raw values via .iter(), then raw_headers.into_iter() feeds credential injection. Matches the tools wrapper pattern.

  2. Comment fixed — Now reads "URL has template substitution applied (injected_url) but not yet host credential injection" (was incorrectly saying "before ANY credential injection").

Should-fix (all resolved)

  1. Import migrated — Both production (line 54) and test (line 4798) imports changed from crate::safety::LeakDetector to ironclaw_safety::LeakDetector.

  2. Block scope removed — Flat code, no unnecessary { }.

  3. raw_url_for_scan alias removed — Uses &url directly.

All checks pass (cargo check, clippy, fmt, tests).

zmanian
zmanian previously approved these changes Mar 31, 2026

@zmanian zmanian left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review -- APPROVE

Security fix is correct. Leak scan now runs on pre-injection headers, preventing false positives from host-injected credentials (e.g., Slack xoxb- tokens).

All of ilblackdragon's feedback addressed:

  1. Double-parse eliminated -- single parse into raw_headers, consumed by into_iter() after scan
  2. Comment updated to accurately describe pre/post-injection ordering
  3. Import migrated to ironclaw_safety::LeakDetector
  4. Block scope and URL alias removed
  5. unwrap_or_default replaced with unwrap_or_else + tracing::warn

Single fix point in http_request host function correctly covers all WASM HTTP egress (other callbacks invoke guest code which calls back into http_request).

Minor suggestion (non-blocking)

Consider whether malformed headers_json should be a hard error rather than falling back to empty headers -- a WASM module producing unparseable JSON may be worth failing fast on.

serrrfirat
serrrfirat previously approved these changes Apr 13, 2026

@serrrfirat serrrfirat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. I resolved the staging merge conflict and re-checked the final diff against origin/staging; it stays scoped to the WASM channel wrapper leak-scan ordering. The rerun checks on head 7db495c3 are green, including formatting, clippy default/libsql/all-features, cargo-deny, no-panics, and regression enforcement.

@serrrfirat
serrrfirat requested a review from zmanian April 13, 2026 11:19
This was referenced Apr 22, 2026
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
nearai#1377)

* fix(wasm): run leak scan on pre-injection headers in channel callbacks

The WASM channel host's http_request handler was scanning request headers
AFTER inject_credentials() replaced placeholder values (e.g. {SLACK_BOT_TOKEN})
with real secrets. This caused the leak detector to flag host-injected
credentials as potential leaks, blocking legitimate WASM channel callbacks.

Run the leak scan on the original WASM-provided headers (before any
credential injection) so host-injected tokens never appear in the scan.
WASM never sees the real values, so scanning the pre-injection state is
correct. Matches the existing pattern in src/tools/wasm/wrapper.rs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: regression test for pre-injection leak scan ordering

Proves that scanning post-injection headers triggers a false positive
on host-injected xoxb- tokens, confirming the fix must scan WASM-provided
headers before credential injection.

* fix: address review feedback — eliminate double-parse, fix comment, migrate import

- Eliminate double-parse of headers_json: parse once, scan raw headers,
  then inject credentials (matches tools wrapper pattern)
- Fix misleading comment: URL has template substitution but not yet
  host credential injection (was "before ANY credential injection")
- Migrate import to ironclaw_safety::LeakDetector per CLAUDE.md
- Remove unnecessary block scope around leak scan
- Remove raw_url_for_scan alias (just use &url directly)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: serrrfirat <f@nuff.tech>
Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: experienced 6-19 merged PRs risk: medium Business logic, config, or moderate-risk modules scope: channel/wasm WASM channel runtime scope: dependencies Dependency updates scope: docs Documentation size: M 50-199 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants