feat(web): migrate web worker from TypeScript to Rust - #295
Merged
Conversation
…olation Add functions/fetch.rs handler (raw Value input, invalid_payload on parse/validate failure, catch_unwind → transport_error envelope) and functions/mod.rs with register_all. Wire pub mod functions into lib.rs.
- ssrf.rs: fix stale comment (IPv6 bracket stripping happens in
parse_target, not host_str); move mid-file SocketAddr import to
top-level use std::net::{..., SocketAddr}
- schemas.rs: fix module doc "(for request_format)" → "(for request_schema)"
- harness/src/web removed (untracked TS leftover, no git change needed)
- cargo build + test (54 passed) + clippy -D warnings: all green
Move the CPU-bound depth-check and HTML-to-markdown/text conversion into tokio::task::spawn_blocking so large transforms never block an async runtime worker thread. shape_response is now async; the call site in execute_fetch is updated to .await it. Behavior is identical: depth guard, transform failure, and closure panics all fall back to raw body with transformed unset; truncation marker only appended on successful transform.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 54 minutes and 53 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughIntroduces an entirely new Changesweb worker: SSRF-guarded HTTP fetch
Sequence Diagram(s)sequenceDiagram
rect rgba(135, 206, 235, 0.5)
Note over main,configuration: Startup
main->>configuration: register_config (schema + optional seed)
main->>configuration: fetch_config
configuration-->>main: WebConfig
main->>SharedState: apply_config (ArcSwap store)
main->>IIIBus: register web::fetch tool + configuration:updated trigger
end
rect rgba(144, 238, 144, 0.5)
Note over Agent,shape_response: web::fetch request
Agent->>execute_fetch: FetchPayload
execute_fetch->>ssrf_check_target: parse_target + DNS resolve + check_ip all addrs
ssrf_check_target-->>execute_fetch: ResolvedTarget (pinned IP) or blocked_host error
execute_fetch->>reqwest: request to pinned SocketAddr (SNI = hostname)
reqwest-->>execute_fetch: HTTP Response
execute_fetch->>shape_response: bytes + content_type + format + cfg
shape_response->>convert: html_to_markdown / extract_text (spawn_blocking)
convert-->>shape_response: transformed text
shape_response-->>Agent: JSON envelope {ok, status, body/json/content}
end
rect rgba(255, 200, 100, 0.5)
Note over configuration,SharedState: Hot-reload
configuration->>on_config_change: configuration:updated event
on_config_change->>configuration: fetch_config (with retry)
configuration-->>on_config_change: WebConfig
on_config_change->>SharedState: apply_config (ArcSwap swap)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
skill-check — worker0 verified, 23 skipped (no docs/).
Four for four. Nicely done. |
sergiofilhowz
approved these changes
Jun 19, 2026
CI runs `cargo fmt --all -- --check`, which was failing on rustfmt drift across the crate (notably the test files). Normalize formatting. Claude-Session: https://claude.ai/code/session_018YytfzKvsyBiTHi6b2u1rn
The per-worker PR validation requires a non-empty README.md. Document the web::fetch request/response envelope, configuration keys, page-reading mode, the SSRF guard, and local dev commands. Claude-Session: https://claude.ai/code/session_018YytfzKvsyBiTHi6b2u1rn
Every sibling Rust worker commits its Cargo.lock; the web crate was missing it. Add it for reproducible builds and convention parity. Claude-Session: https://claude.ai/code/session_018YytfzKvsyBiTHi6b2u1rn
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
web/skills/index.md (1)
109-109: 💤 Low valueSimplify "very large pages" to reduce intensifier redundancy.
The static analysis tool flags "very" as an over-used intensifier here. Since "large pages" is already clear in context, consider shortening to: "For large pages, lower
max_bytes…" to tighten the prose.📝 Proposed edit
-For very large pages, lower `max_bytes` — conversion runs on the capped body. +For large pages, lower `max_bytes` — conversion runs on the capped body.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/skills/index.md` at line 109, Remove the word "very" from the phrase "very large pages" in the documentation for the format parameter. The sentence currently reads "For very large pages, lower max_bytes..." and should be changed to "For large pages, lower max_bytes..." to eliminate redundant intensifier usage while maintaining clarity and tightening the prose.web/README.md (1)
82-82: 💤 Low valueClarify
max_bytesdefault more explicitly.The current phrasing "
max_response_bytes(default_response_bytesinformatmode)" is harder to parse than it could be. For consistency withskills/index.md(line 61) and user clarity, consider restating as: "max_bytes| 5 MiB (256 KiB informatmode)".📝 Proposed rewording
-| `max_bytes` | `max_response_bytes` (`default_response_bytes` in `format` mode) | over-cap body is truncated, not errored | +| `max_bytes` | 5 MiB (256 KiB in `format` mode) | over-cap body is truncated, not errored |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/README.md` at line 82, The documentation in the README currently uses unclear phrasing for the max_bytes parameter default value. Locate the section describing max_response_bytes and its default behavior in format mode, and replace the current phrasing with the clearer format: "max_bytes | 5 MiB (256 KiB in format mode)". This change makes the default values more explicit and aligns the terminology and presentation with the documentation in skills/index.md for consistency across the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/config.yaml.example`:
- Line 9: The allow_loopback setting in the example configuration file is
currently set to true, which creates an insecure default that enables localhost
access for SSRF attacks by default. Change the allow_loopback value from true to
false to establish a secure-by-default posture, ensuring that operators who use
this example configuration file without modification will have the more
restrictive and safer setting enabled.
In `@web/src/fetch.rs`:
- Around line 100-104: The stream read error handling in the while loop that
processes chunks is returning (buf, false) to indicate a non-truncated response,
but this causes lines 209-215 to treat it as a successful response and return
ok: true with partial data. Instead of returning (buf, false) on stream errors
in the Err(_) branch, return a value that indicates a transport error occurred
so that the response handling logic at lines 209-215 properly surfaces this as
an error rather than as a successful response with ok: true.
- Around line 244-265: The header key comparison and insertion in the fetch.rs
code is case-sensitive, but HTTP headers are case-insensitive, causing duplicate
logical headers when keys like "User-Agent" and "user-agent" are treated as
separate entries in the BTreeMap. Normalize all header keys to lowercase when
inserting them into the base map and when checking caller_keys contains values.
Specifically, convert the hardcoded header keys ("user-agent", "accept",
"accept-language") to lowercase, convert the page_format accept header key to
lowercase, and normalize all keys from payload.headers by calling to_lowercase()
on each key k before inserting into the base map to ensure the "caller wins"
logic works correctly regardless of the case used in the input headers.
In `@web/src/main.rs`:
- Around line 73-82: The YAML parsing error is being silently dropped when using
.ok() on the serde_yaml::from_str call, making misconfiguration hard to detect.
Replace the .ok() call with explicit error handling by matching on the Result to
capture the error when YAML parsing fails. When the serde_yaml parse fails (in
the current None branch), log the error using tracing::warn! with the path and
error details before returning None, similar to how the WebConfig::from_json
error is already being logged on line 76.
- Around line 85-92: The `register_config` and `fetch_config` function calls
currently use the `?` operator to propagate errors, causing the startup to fail
on transient configuration RPC errors. Instead, handle errors from both calls by
catching the result, logging a warning message with `tracing::warn!` that
includes the error details, and falling back to `WebConfig::default()` to allow
the worker to continue booting. Apply this pattern to both the `register_config`
call and the `fetch_config` call.
---
Nitpick comments:
In `@web/README.md`:
- Line 82: The documentation in the README currently uses unclear phrasing for
the max_bytes parameter default value. Locate the section describing
max_response_bytes and its default behavior in format mode, and replace the
current phrasing with the clearer format: "max_bytes | 5 MiB (256 KiB in format
mode)". This change makes the default values more explicit and aligns the
terminology and presentation with the documentation in skills/index.md for
consistency across the codebase.
In `@web/skills/index.md`:
- Line 109: Remove the word "very" from the phrase "very large pages" in the
documentation for the format parameter. The sentence currently reads "For very
large pages, lower max_bytes..." and should be changed to "For large pages,
lower max_bytes..." to eliminate redundant intensifier usage while maintaining
clarity and tightening the prose.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3c6f2ccf-e187-4e3a-93cc-5aa0e16198b6
⛔ Files ignored due to path filters (1)
web/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
web/Cargo.tomlweb/README.mdweb/build.rsweb/config.yaml.exampleweb/iii.worker.yamlweb/skills/index.mdweb/src/config.rsweb/src/configuration.rsweb/src/convert.rsweb/src/fetch.rsweb/src/functions/fetch.rsweb/src/functions/mod.rsweb/src/lib.rsweb/src/main.rsweb/src/manifest.rsweb/src/schemas.rsweb/src/ssrf.rsweb/tests/integration.rsweb/tests/ssrf_rebinding.rs
| max_transform_bytes: 1048576 | ||
| max_redirects: 5 | ||
| user_agent: 'iii-harness/0.1 (+web::fetch)' | ||
| allow_loopback: true |
There was a problem hiding this comment.
Default allow_loopback should be false for secure-by-default SSRF posture.
Enabling loopback in the example config makes localhost access the default behavior, which weakens the guardrail for operators who copy this file as-is.
Suggested change
- allow_loopback: true
+ allow_loopback: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| allow_loopback: true | |
| allow_loopback: false |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/config.yaml.example` at line 9, The allow_loopback setting in the example
configuration file is currently set to true, which creates an insecure default
that enables localhost access for SSRF attacks by default. Change the
allow_loopback value from true to false to establish a secure-by-default
posture, ensuring that operators who use this example configuration file without
modification will have the more restrictive and safer setting enabled.
Comment on lines
+85
to
+92
| configuration::register_config(&iii, seed.as_ref()) | ||
| .await | ||
| .map_err(anyhow::Error::msg) | ||
| .context("registering web configuration schema")?; | ||
| let cfg = configuration::fetch_config(&iii) | ||
| .await | ||
| .map_err(anyhow::Error::msg) | ||
| .context("loading web configuration")?; |
There was a problem hiding this comment.
Avoid hard-failing startup on configuration RPC errors.
Line 85 and Line 89 currently propagate errors with ?, so transient configuration-worker failures prevent the worker from registering and serving requests. Fall back to WebConfig::default() with tracing::warn! and continue booting.
Suggested fix
- configuration::register_config(&iii, seed.as_ref())
- .await
- .map_err(anyhow::Error::msg)
- .context("registering web configuration schema")?;
- let cfg = configuration::fetch_config(&iii)
- .await
- .map_err(anyhow::Error::msg)
- .context("loading web configuration")?;
+ if let Err(e) = configuration::register_config(&iii, seed.as_ref()).await {
+ tracing::warn!(error = %e, "failed to register web configuration schema; continuing");
+ }
+ let cfg = match configuration::fetch_config(&iii).await {
+ Ok(cfg) => cfg,
+ Err(e) => {
+ tracing::warn!(error = %e, "failed to load web configuration; using defaults");
+ WebConfig::default()
+ }
+ };Based on learnings: in this repository, worker binaries should warn and fall back to default config on config-load failures instead of exiting.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| configuration::register_config(&iii, seed.as_ref()) | |
| .await | |
| .map_err(anyhow::Error::msg) | |
| .context("registering web configuration schema")?; | |
| let cfg = configuration::fetch_config(&iii) | |
| .await | |
| .map_err(anyhow::Error::msg) | |
| .context("loading web configuration")?; | |
| if let Err(e) = configuration::register_config(&iii, seed.as_ref()).await { | |
| tracing::warn!(error = %e, "failed to register web configuration schema; continuing"); | |
| } | |
| let cfg = match configuration::fetch_config(&iii).await { | |
| Ok(cfg) => cfg, | |
| Err(e) => { | |
| tracing::warn!(error = %e, "failed to load web configuration; using defaults"); | |
| WebConfig::default() | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/main.rs` around lines 85 - 92, The `register_config` and
`fetch_config` function calls currently use the `?` operator to propagate
errors, causing the startup to fail on transient configuration RPC errors.
Instead, handle errors from both calls by catching the result, logging a warning
message with `tracing::warn!` that includes the error details, and falling back
to `WebConfig::default()` to allow the worker to continue booting. Apply this
pattern to both the `register_config` call and the `fetch_config` call.
Source: Learnings
- read_capped: surface a pre-cap stream error as a transport_error
instead of returning ok:true with a partial body
- execute_fetch: merge caller headers case-insensitively so a
differently-cased header (e.g. User-Agent) overrides the injected
default rather than being sent as a duplicate
- main: log YAML seed parse failures via tracing::warn! instead of
silently dropping them with .ok()
- skills/index.md: drop redundant intensifier ("very large pages")
Claude-Session: https://claude.ai/code/session_01QY5uCFgwr4wXR7ZcAKanaR
- release.yml: trigger the Release workflow on `web/v*` tags - create-tag.yml: add `web` to the Create Tag worker choices so a release tag can be cut via the standard dispatch - parse_publish_workers_input.py: allow `web` in the skills-publish allowlist (web ships skills/index.md), so `all` covers it web/iii.worker.yaml already declares deploy: binary / bin: iii-web, so the generic Rust binary build + registry publish path (same as iii-directory) handles the rest with no further changes. Claude-Session: https://claude.ai/code/session_01QY5uCFgwr4wXR7ZcAKanaR
andersonleal
added a commit
that referenced
this pull request
Jun 19, 2026
…` resolves it (#300) The TS→Rust migration (#295) named web's binary `iii-web` (Cargo.toml [[bin]] and iii.worker.yaml `bin`). `iii worker add web` downloads the release archive and looks for a binary named after the WORKER (`web`), since the registry payload carries only the worker name, never the cargo [[bin]] name. The archive shipped `iii-web`, so resolution failed with "Binary 'web' not found in archive". Rename the binary to `web`, matching the convention every other binary worker already follows (21/23; the tag parser even defaults `bin` to the worker name). Runtime identity is unchanged — main.rs registers name:"web" and manifest.rs uses CARGO_PKG_NAME. Also add a release-blocking guard in validate_worker.py: for deploy:binary workers, `bin` (defaulting to the worker name) must equal the worker name, or `iii worker add` can't unpack the archive. `acp` is allowlisted — its `iii-acp` binary is user-facing (editors launch it by name), so it needs a resolver-side fix instead of a rename. A new release tag (web → v1.1.2) is required for this to take effect; the already-published v1.1.1 archive stays broken.
This was referenced Jul 3, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Migrates the
webworker (web::fetch) from the orphanedharness/src/webTypeScript code to a standalone Rust crate atweb/, modeled oniii-directory. The TS worker was the last non-Rust holdout in the harness and its../runtime/*imports no longer existed, so it could not build.What changed
web/(binaryiii-web), one bus functionweb::fetch. Faithful behavioral port — same request fields and success/error/image envelopes, so existing callers and the harnesstrigger.rs::normalizeconsumer are unaffected.harness/src/webTypeScript leftovers (already absent from HEAD).Highlights
ssrf.rs):ipnet-based v4/v6 blocklist (RFC1918, loopback, link-local incl. AWS metadata, ULA, multicast, reserved), v4-mapped::ffff:handling viato_ipv4_mapped, literal-IP short-circuit, fail-closed DNS, validates every resolved address.fetch.rs): per-redirect-hopreqwestclient with.resolve(hostname, validated_ip)so the socket dials the vetted IP while SNI/cert/Host stay on the hostname (defeats DNS rebinding);redirect::Policy::none()re-validates each hop; relativeLocationjoin; cross-origin/downgrade credential stripping; Cloudflarecf-mitigated:challengeretry.htmd, HTML→text viatl(astral-tl), viewable-image envelope; CPU-bound transform runs ontokio::spawn_blockingbehind a pre-transform nesting-depth guard (htmd stack-overflow aborts are uncatchable).serde_json::Valueparsed internally (malformed → structuredinvalid_payload), always returns an envelope, whole-handlercatch_unwindpanic isolation.configuration-worker integration (register schema + seed, fetch authoritative value, hot-reload trigger), single snapshot per request.Testing
cargo build,cargo test, andcargo clippy -- -D warningsall clean.iii-sdk = "=0.19.2"(workspace standard); reqwest rustls-tls/json/stream/http2.Follow-ups (non-blocking, doc-only)
too_largeerror code; the implementation uses string codes and never emits it (matches intent) — reconcile spec text.::a.b.c.d, deprecated/non-routable) is intentionally not blocked (parity with the original TS); could add a clarifying comment.Design + plan:
docs/superpowers/specs/2026-06-19-web-worker-rust-migration-design.md,docs/superpowers/plans/2026-06-19-web-worker-rust-migration.md(local/gitignored).https://claude.ai/code/session_01QY5uCFgwr4wXR7ZcAKanaR
Summary by CodeRabbit
New Features
web::fetch, including configurable timeouts, response size limits, redirect following, and SSRF protection (with optional loopback allowance).Documentation
web::fetchskill documentation, including request/response fields, truncation behavior, and SSRF rules.