fix(sandbox): restore Telegram media downloads in proxy-only sandbox#1755
Merged
cv merged 20 commits intoNVIDIA:mainfrom Apr 16, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBuild-time Dockerfile changes patch bundled OpenClaw JS to switch SSRF-guarded fetch calls to a trusted-env/proxy mode, add conditional Changes
Sequence Diagram(s)sequenceDiagram
participant Build as Build (Dockerfile)
participant Bundle as OpenClawBundle
participant Runtime as App Runtime
participant Proxy as TrustedEnvProxy
participant External as ExternalHost/Media
rect rgba(200,200,255,0.5)
Build->>Bundle: Patch bundled JS\nreplace strict guarded fetch with trusted-env/proxy mode
end
rect rgba(200,255,200,0.5)
Runtime->>Proxy: Request external resource (telegram/discord/slack/media)
Proxy->>External: Forward request using proxy host/port
External-->>Proxy: Return response
Proxy-->>Runtime: Deliver proxied response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Around line 53-57: After the sed rewrite in the Dockerfile's RUN block that
targets openclaw/dist, add post-patch verification that fails the build if the
transformation didn't happen: grep to assert the old import/identifier
"withStrictGuardedFetchMode" (and the old callsite pattern using
fetchWithSsrFGuard(withStrictGuardedFetchMode({) is no longer present) and grep
to assert the new import name "withTrustedEnvProxyGuardedFetchMode" and the new
callsite using fetchWithSsrFGuard(withTrustedEnvProxyGuardedFetchMode({) does
exist; if either check fails, exit non-zero so the image build closes rather
than silently succeeding. Ensure checks run immediately after the sed step that
edits withStrictGuardedFetchMode/fetchWithSsrFGuard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Dockerfile (1)
72-72: Static analysis flag:NEMOCLAW_PROVIDER_KEYnaming is misleading.Trivy flags this as potential secret exposure. While this is a false positive (the value is used as a provider identifier/name at Line 155, not an actual credential—the API key is hardcoded as
'unused'at Line 157), the namingPROVIDER_KEYsuggests it's a secret when it's really a provider name.Consider renaming to
NEMOCLAW_PROVIDER_NAMEorNEMOCLAW_PROVIDER_IDin a future cleanup to avoid confusion and static analysis noise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` at line 72, The ARG NEMOCLAW_PROVIDER_KEY in the Dockerfile is named like a secret and triggers static analysis; rename the argument to a non-secret-sounding identifier (e.g., NEMOCLAW_PROVIDER_NAME or NEMOCLAW_PROVIDER_ID) and update all usages that reference NEMOCLAW_PROVIDER_KEY (including where the provider value is consumed) to the new name to avoid Trivy false positives while preserving functionality.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Dockerfile`:
- Line 72: The ARG NEMOCLAW_PROVIDER_KEY in the Dockerfile is named like a
secret and triggers static analysis; rename the argument to a
non-secret-sounding identifier (e.g., NEMOCLAW_PROVIDER_NAME or
NEMOCLAW_PROVIDER_ID) and update all usages that reference NEMOCLAW_PROVIDER_KEY
(including where the provider value is consumed) to the new name to avoid Trivy
false positives while preserving functionality.
wscurran
added
bug
Contributor
|
✨
Possibly related open issues: |
wscurran
added
the
Integration: Telegram
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Dockerfile (2)
53-57:⚠️ Potential issue | 🟠 MajorFail this dist rewrite closed.
This
RUNstep still succeeds even when a futureopenclaw/distbundle no longer matches these exact minified strings, so proxy-only media downloads can silently regress on base-image updates. Add a post-rewrite assertion that at least one file was patched, the trusted-proxy callsite exists afterward, and the strict callsite is gone.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 53 - 57, The RUN step that rewrites minified imports must fail the build if nothing was changed and must verify the replacement succeeded: after running the sed, add commands to (1) assert at least one file was modified (e.g., check grep -R -q for withTrustedEnvProxyGuardedFetchMode or count of changed files), (2) assert the new callsite fetchWithSsrFGuard(withTrustedEnvProxyGuardedFetchMode({) exists, and (3) assert the old callsite fetchWithSsrFGuard(withStrictGuardedFetchMode({) no longer exists; if any check fails, exit non‑zero so the Docker build fails. Use the existing symbols withStrictGuardedFetchMode, withTrustedEnvProxyGuardedFetchMode, and fetchWithSsrFGuard to locate/verify changes.
147-147:⚠️ Potential issue | 🟠 MajorHonor the proxy build args here.
Line 147 hardcodes
10.200.0.1:3128, so the documentedNEMOCLAW_PROXY_HOST/NEMOCLAW_PROXY_PORToverride never reaches Telegram/Discord/Slack media calls on non-default networks.Suggested fix
RUN python3 -c "\ import base64, json, os, secrets; \ from urllib.parse import urlparse; \ +proxy_url = f\"http://{os.environ['NEMOCLAW_PROXY_HOST']}:{os.environ['NEMOCLAW_PROXY_PORT']}\"; \ model = os.environ['NEMOCLAW_MODEL']; \ chat_ui_url = os.environ['CHAT_UI_URL']; \ provider_key = os.environ['NEMOCLAW_PROVIDER_KEY']; \ @@ -_ch_cfg = {ch: {'accounts': {'default': {_token_keys[ch]: f'openshell:resolve:env:{_env_keys[ch]}', 'enabled': True, **({'proxy': 'http://10.200.0.1:3128'} if ch in ('telegram', 'discord', 'slack') else {}), **({'groupPolicy': 'open'} if ch == 'telegram' else {}), **({'dmPolicy': 'allowlist', 'allowFrom': _allowed_ids[ch]} if ch in _allowed_ids and _allowed_ids[ch] else {})}}} for ch in msg_channels if ch in _token_keys}; \ +_ch_cfg = {ch: {'accounts': {'default': {_token_keys[ch]: f'openshell:resolve:env:{_env_keys[ch]}', 'enabled': True, **({'proxy': proxy_url} if ch in ('telegram', 'discord', 'slack') else {}), **({'groupPolicy': 'open'} if ch == 'telegram' else {}), **({'dmPolicy': 'allowlist', 'allowFrom': _allowed_ids[ch]} if ch in _allowed_ids and _allowed_ids[ch] else {})}}} for ch in msg_channels if ch in _token_keys}; \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` at line 147, The channel config comprehension that builds _ch_cfg hardcodes the proxy URL ('http://10.200.0.1:3128') for message channels, preventing NEMOCLAW_PROXY_HOST/NEMOCLAW_PROXY_PORT build args from taking effect; update the proxy construction inside the dict comprehension (the branch for ch in ('telegram', 'discord', 'slack')) to build the proxy string from the NEMOCLAW_PROXY_HOST and NEMOCLAW_PROXY_PORT environment/build-arg values (or fall back to the current hardcoded URL if those vars are unset), keeping the change local to the _ch_cfg expression that uses msg_channels and _token_keys so the rest of the logic (groupPolicy, dmPolicy, allowFrom) is unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Line 235: Replace the current ln -sf invocation that links
/sandbox/.openclaw-data/media to /sandbox/.openclaw-data/workspace/media so it
always replaces the target path instead of nesting inside an existing directory;
update the Dockerfile line containing the `ln -sf /sandbox/.openclaw-data/media
/sandbox/.openclaw-data/workspace/media` command to either remove the existing
/sandbox/.openclaw-data/workspace/media before creating the symlink or use a
force-and-treat-destination-as-file ln option (e.g., using -f with -T) so the
target path is replaced rather than creating a nested media directory.
---
Duplicate comments:
In `@Dockerfile`:
- Around line 53-57: The RUN step that rewrites minified imports must fail the
build if nothing was changed and must verify the replacement succeeded: after
running the sed, add commands to (1) assert at least one file was modified
(e.g., check grep -R -q for withTrustedEnvProxyGuardedFetchMode or count of
changed files), (2) assert the new callsite
fetchWithSsrFGuard(withTrustedEnvProxyGuardedFetchMode({) exists, and (3) assert
the old callsite fetchWithSsrFGuard(withStrictGuardedFetchMode({) no longer
exists; if any check fails, exit non‑zero so the Docker build fails. Use the
existing symbols withStrictGuardedFetchMode,
withTrustedEnvProxyGuardedFetchMode, and fetchWithSsrFGuard to locate/verify
changes.
- Line 147: The channel config comprehension that builds _ch_cfg hardcodes the
proxy URL ('http://10.200.0.1:3128') for message channels, preventing
NEMOCLAW_PROXY_HOST/NEMOCLAW_PROXY_PORT build args from taking effect; update
the proxy construction inside the dict comprehension (the branch for ch in
('telegram', 'discord', 'slack')) to build the proxy string from the
NEMOCLAW_PROXY_HOST and NEMOCLAW_PROXY_PORT environment/build-arg values (or
fall back to the current hardcoded URL if those vars are unset), keeping the
change local to the _ch_cfg expression that uses msg_channels and _token_keys so
the rest of the logic (groupPolicy, dmPolicy, allowFrom) is unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ed54896-70af-498d-a158-b2504e4814be
📒 Files selected for processing (1)
Dockerfile
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
Dockerfile (2)
53-60:⚠️ Potential issue | 🟠 MajorFail closed on the import rewrite, not just the callsite rewrite.
Line 59 only proves the callsite text changed. If the exact import replacement on Line 57 stops matching a future OpenClaw bundle, the build still passes but the rewritten file will reference
withTrustedEnvProxyGuardedFetchModewithout importing it.Suggested fix
RUN set -eu; \ matches="$(grep -RIl --include='*.js' 'fetchWithSsrFGuard(withStrictGuardedFetchMode({' /usr/local/lib/node_modules/openclaw/dist/)"; \ test -n "$matches"; \ printf '%s\n' "$matches" | xargs sed -i \ -e 's|import { n as withStrictGuardedFetchMode, t as fetchWithSsrFGuard }|import { n as withStrictGuardedFetchMode, r as withTrustedEnvProxyGuardedFetchMode, t as fetchWithSsrFGuard }|g' \ -e 's|fetchWithSsrFGuard(withStrictGuardedFetchMode({|fetchWithSsrFGuard(withTrustedEnvProxyGuardedFetchMode({|g'; \ - grep -Rq --include='*.js' 'fetchWithSsrFGuard(withTrustedEnvProxyGuardedFetchMode({' /usr/local/lib/node_modules/openclaw/dist/; \ - ! grep -Rq --include='*.js' 'fetchWithSsrFGuard(withStrictGuardedFetchMode({' /usr/local/lib/node_modules/openclaw/dist/ + printf '%s\n' "$matches" | while IFS= read -r file; do \ + grep -q 'r as withTrustedEnvProxyGuardedFetchMode' "$file"; \ + grep -q 'fetchWithSsrFGuard(withTrustedEnvProxyGuardedFetchMode({' "$file"; \ + ! grep -q 'fetchWithSsrFGuard(withStrictGuardedFetchMode({' "$file"; \ + done#!/bin/bash set -eu tmp="$(mktemp -d)" trap 'rm -rf "$tmp"' EXIT cat >"$tmp/bundle.js" <<'EOF' import { x as withStrictGuardedFetchMode, t as fetchWithSsrFGuard } from "./guard.js"; export const run = () => fetchWithSsrFGuard(withStrictGuardedFetchMode({})); EOF matches="$(grep -RIl --include='*.js' 'fetchWithSsrFGuard(withStrictGuardedFetchMode({' "$tmp")" printf '%s\n' "$matches" | xargs sed -i \ -e 's|import { n as withStrictGuardedFetchMode, t as fetchWithSsrFGuard }|import { n as withStrictGuardedFetchMode, r as withTrustedEnvProxyGuardedFetchMode, t as fetchWithSsrFGuard }|g' \ -e 's|fetchWithSsrFGuard(withStrictGuardedFetchMode({|fetchWithSsrFGuard(withTrustedEnvProxyGuardedFetchMode({|g' grep -Rq --include='*.js' 'fetchWithSsrFGuard(withTrustedEnvProxyGuardedFetchMode({' "$tmp" ! grep -Rq --include='*.js' 'fetchWithSsrFGuard(withStrictGuardedFetchMode({' "$tmp" if grep -Rq --include='*.js' 'r as withTrustedEnvProxyGuardedFetchMode' "$tmp"; then echo "Unexpected: import rewrite landed" exit 1 fi cat "$tmp/bundle.js" # Expected: the current verification passes even though the new import fragment is still missing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 53 - 60, The import rewrite check is missing so the build can pass if only the callsite changed; update the Dockerfile block that edits openclaw dist files (the sed replacements referencing withStrictGuardedFetchMode, withTrustedEnvProxyGuardedFetchMode and fetchWithSsrFGuard) to also assert the import replacement occurred by grepping for the new import fragment (e.g. the token sequence "r as withTrustedEnvProxyGuardedFetchMode" or the full changed import) and fail the build if that import string is not present; in short, after the sed invocation and the existing callsite grep, add a check that verifies the import line containing withTrustedEnvProxyGuardedFetchMode was inserted and exit non‑zero if it was not.
239-239:⚠️ Potential issue | 🟠 MajorReplace
workspace/mediainstead of linking inside it.Line 239 still uses
ln -sfn, which does not replace a real directory target. If/sandbox/.openclaw-data/workspace/mediaalready exists, this creates.../workspace/media/mediaand leaves the old layout in place.Suggested fix
- && ln -sfn /sandbox/.openclaw-data/media /sandbox/.openclaw-data/workspace/media + && mkdir -p /sandbox/.openclaw-data/workspace \ + && if [ -e /sandbox/.openclaw-data/workspace/media ] && [ ! -L /sandbox/.openclaw-data/workspace/media ]; then \ + cp -a /sandbox/.openclaw-data/workspace/media/. /sandbox/.openclaw-data/media/ 2>/dev/null || true; \ + rm -rf /sandbox/.openclaw-data/workspace/media; \ + fi \ + && ln -sfnT /sandbox/.openclaw-data/media /sandbox/.openclaw-data/workspace/media#!/bin/bash set -eu tmp="$(mktemp -d)" trap 'rm -rf "$tmp"' EXIT mkdir -p "$tmp/media" "$tmp/workspace/media" touch "$tmp/workspace/media/existing" ln -sfn "$tmp/media" "$tmp/workspace/media" find "$tmp" -maxdepth 3 -mindepth 1 -printf '%P %y -> %l\n' | sort # Expected if Line 239 is safe: workspace/media becomes a symlink. # Actual with GNU ln: a nested workspace/media/media symlink appears.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` at line 239, The ln invocation "ln -sfn /sandbox/.openclaw-data/media /sandbox/.openclaw-data/workspace/media" can create a nested media directory if the target already exists; change the Dockerfile so the existing /sandbox/.openclaw-data/workspace/media is removed or replaced before creating the symlink (e.g., rm -rf /sandbox/.openclaw-data/workspace/media && ln -sfn /sandbox/.openclaw-data/media /sandbox/.openclaw-data/workspace/media) or, if you rely on GNU coreutils, use the -T flag (ln -sfnT ...) to force-replace the target atomically; update the line referencing that ln command accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Dockerfile`:
- Around line 53-60: The import rewrite check is missing so the build can pass
if only the callsite changed; update the Dockerfile block that edits openclaw
dist files (the sed replacements referencing withStrictGuardedFetchMode,
withTrustedEnvProxyGuardedFetchMode and fetchWithSsrFGuard) to also assert the
import replacement occurred by grepping for the new import fragment (e.g. the
token sequence "r as withTrustedEnvProxyGuardedFetchMode" or the full changed
import) and fail the build if that import string is not present; in short, after
the sed invocation and the existing callsite grep, add a check that verifies the
import line containing withTrustedEnvProxyGuardedFetchMode was inserted and exit
non‑zero if it was not.
- Line 239: The ln invocation "ln -sfn /sandbox/.openclaw-data/media
/sandbox/.openclaw-data/workspace/media" can create a nested media directory if
the target already exists; change the Dockerfile so the existing
/sandbox/.openclaw-data/workspace/media is removed or replaced before creating
the symlink (e.g., rm -rf /sandbox/.openclaw-data/workspace/media && ln -sfn
/sandbox/.openclaw-data/media /sandbox/.openclaw-data/workspace/media) or, if
you rely on GNU coreutils, use the -T flag (ln -sfnT ...) to force-replace the
target atomically; update the line referencing that ln command accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7b3667fa-7f87-4776-a9f2-0a578f8cab7e
📒 Files selected for processing (1)
Dockerfile
wscurran
added
enhancement: integration
…ly sandbox Three issues prevent inbound media (photos, voice, documents) from working when messaging channels are enabled in a NemoClaw sandbox: 1. OpenClaw's fetchRemoteMedia uses SSRF strict mode (DNS pinning + direct connect) which bypasses the OpenShell L7 proxy. The sandbox netns blocks direct egress, so file downloads fail with TypeError: fetch failed. This is the same root cause as NVIDIA#1252 but affects a different code path (local-roots-*.js and compact-*.js bundles) that was not covered by the existing web_search/web_fetch patch. Fix: patch all OpenClaw dist bundles at image build time to use withTrustedEnvProxyGuardedFetchMode instead of withStrictGuardedFetchMode for fetchRemoteMedia. 2. OpenClaw's getFile and other grammy-based Telegram API calls also bypass the proxy because the channel account config lacks a proxy field. Without it, grammy uses its own dispatcher instead of EnvHttpProxyAgent. Fix: inject proxy: http://10.200.0.1:3128 into all messaging channel account configs generated at image build time. 3. Downloaded media is saved to /sandbox/.openclaw/media/inbound/ but that directory does not exist. .openclaw/ is Landlock read-only and media was not included in the .openclaw-data symlink set. This causes EACCES: permission denied, mkdir .openclaw/media. Fix: add .openclaw-data/media to the mkdir, chown, and symlink loop alongside logs, credentials, and sandbox. Tested end-to-end: Telegram photo with caption is downloaded, saved, resized, and passed to the inference model successfully. Related: NVIDIA#1252, NVIDIA#1301 Signed-off-by: Tommy Lin <tommylin@signalpro.com.tw>
OpenClaw saves inbound media to stateDir/media/inbound/ but the agent read tool looks in stateDir/workspace/media/inbound/ (upstream bug: openclaw/openclaw#64434). Bridge the gap with a symlink so downloaded Telegram/Discord photos are readable by the agent until upstream fixes the path inconsistency. Signed-off-by: Tommy Lin <tommylin@signalpro.com.tw>
- Add post-patch verification to fail-close the sed rewrite: assert new pattern exists and old pattern is gone, so the build fails if openclaw bundle structure changes - Use NEMOCLAW_PROXY_HOST/NEMOCLAW_PROXY_PORT build args instead of hardcoding proxy URL in channel config - Change ln -sf to ln -sfn to prevent symlink nesting when workspace/media already exists as a directory Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add post-patch assertion for import fragment to catch minified alias drift that would leave the callsite referencing an unimported symbol - Replace ln -sfn with rm -rf + ln -s to handle workspace/media as a real directory, not just a symlink Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Only patch files that import withStrictGuardedFetchMode via the module import pattern. Files like mattermost.js and shared-*.js define the function inline and would crash with ReferenceError if the callsite is rewritten without a matching import. Drop the global negative grep assertion since inline-definition files legitimately retain withStrictGuardedFetchMode callsites. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tommylin-signalpro
force-pushed
the
fix/telegram-media-download-in-sandbox
branch
from
5e31aec to
32af26e
Compare
The previous approach matched a specific minified import alias pattern (`n as withStrictGuardedFetchMode`) that worked on openclaw@2026.3.11 but broke on 2026.4.2 where the aliases became `i`/`n` instead of `n`/`t`. The CI build failed at the fail-close verification step. Switch to rewriting the fetch-guard module export so any consumer that imports the strict-mode alias receives the trusted-env-proxy function. The export pattern `withStrictGuardedFetchMode as <letter>` is stable across versions while alias letters vary between minified bundles. Files that define withStrictGuardedFetchMode locally without exporting (e.g. mattermost.js) keep their original strict behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…y hostname OpenClaw 2026.4.2 introduced assertExplicitProxyAllowed() in fetch-guard, which validates the explicit proxy URL by passing the proxy hostname through resolvePinnedHostnameWithPolicy() with the *target's* SsrfPolicy. When the target uses hostnameAllowlist (e.g. Telegram media: ["api.telegram.org"]), the proxy hostname (10.200.0.1) is rejected because it is not in the target's allowlist. This breaks every Telegram/Discord/Slack media download in NemoClaw's proxy-only sandbox, with no config or env var to disable it. Inject an early `return;` in the function so the proxy hostname is never validated against the target allowlist. The OpenShell L7 proxy still enforces per-endpoint network policy, so SSRF protection at the trust boundary is unchanged. Both this patch and the existing fetch-guard export rewrite share the same RUN block with set -eu plus pre/post-grep verification, so a future OPENCLAW_VERSION bump fails-close at CI when openclaw bundle structure changes — the next maintainer sees the failure with full context in the Dockerfile comment. Replace `! grep` (skipped by set -e under !-prefix) with explicit `if grep; then exit 1; fi` so Patch 1's negative invariant actually fails the build when violated (SC2251). End-to-end verified: Telegram photo download now reaches /file/bot.../path successfully and the file is saved to /sandbox/.openclaw/media/inbound/. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the unconditional `return;` injection in assertExplicitProxyAllowed with `if (process.env.NEMOCLAW_SANDBOX === "1") return;` and set `ENV NEMOCLAW_SANDBOX=1` on the production sandbox image. Outside NemoClaw's sandbox runtime the env var is unset, so the upstream SSRF check on the explicit proxy hostname stays in effect. This narrows the bypass from "globally neuter the SSRF check" to "sandbox-specific compatibility workaround" and keeps the upstream behavior intact for anyone running OpenClaw bare-metal or in another wrapper that uses the same bundle without NEMOCLAW_SANDBOX set. Update Patch 2 fail-close marker grep to match the new injected snippet so a future OPENCLAW_VERSION bump still aborts the build if the function signature changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tommylin-signalpro
changed the title
Align implementation scope with PR title and verified test scope. The previous code injected `proxy: <url>` into telegram, discord, and slack account configs, but only telegram was end-to-end verified. Narrow the channel config injection to telegram only so the patch matches what we claim it fixes. Discord and Slack remain unaffected by this change: main currently injects no proxy config for them either, so this is not a regression, just no extension of the fix to those channels. Future PRs can extend proxy config to discord/slack with their own end-to-end verification. The fetch-guard module patches (Patch 1 export rewrite, Patch 2 env- gated bypass) still apply to every openclaw fetch path inside the sandbox by construction. That is intentional given NemoClaw's proxy- only architecture, not an unscoped side effect. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The previous gate used `NEMOCLAW_SANDBOX=1` set via Dockerfile `ENV`, but
OpenShell controls the sandbox pod env at runtime and image-level ENV vars
do not propagate to the running sandbox process. The gate condition
therefore evaluated false and the SSRF check still ran, reproducing the
original "Blocked hostname (not in allowlist): 10.200.0.1" failure on
freshly onboarded sandboxes.
OpenShell injects `OPENSHELL_SANDBOX=1` into every sandbox pod automatically
(observed in the sandbox shell env). Use that as the gate condition; it is
the only env marker reliably present in the sandbox runtime, and its
semantics ("we are inside an OpenShell sandbox") match exactly the scope
the bypass needs.
Drop the unused `ENV NEMOCLAW_SANDBOX=1` from the Dockerfile; it never
propagated to the sandbox runtime and would only mislead future readers.
End-to-end re-verified on a freshly onboarded sandbox: Telegram photo
downloaded successfully and Claude Haiku correctly described the image
content (motorcycle dashboard, Japanese curry menu) — both round-trips
exercised the patched fetch-guard path.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…wnload-in-sandbox
cv
approved these changes
Apr 16, 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
4 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.
Summary
Restore Telegram media downloads inside NemoClaw's proxy-only sandbox. The agent receives photo messages but
MediaFetchError: Blocked hostname (not in allowlist): 10.200.0.1aborts the download before the file reaches the model. This PR makes the existingfetchRemoteMediacallsite route through the L7 proxy and stops OpenClaw's new SSRF check from rejecting the proxy hostname.Scope: Telegram only. Discord/Slack are not part of this PR — see Implementation scope below for what does and doesn't extend to them.
Related Issue
fetchRemoteMedia/file/bot*/**policy fix, already merged)Root Cause
NemoClaw forces all sandbox egress through the OpenShell L7 proxy (default
10.200.0.1:3128). OpenClaw's media download path requires two things to work in this environment, and OpenClaw 2026.4.2 broke the second one.Layer 1 — wrong fetch mode (long-standing)
openclaw/dist/fetch-ClF-ZgDC.jscallsunconditionally. Strict mode does DNS pinning + direct connect, which fails in the sandbox netns where only the proxy is reachable. The agent needs
withTrustedEnvProxyGuardedFetchMode(usesEnvHttpProxyAgent) for any callsite running in a proxy-only environment.Layer 2 — SSRF check rejects the proxy itself (regression in 2026.4.2)
OpenClaw 2026.4.2 added
assertExplicitProxyAllowed()infetch-guard-*.js:When fetching Telegram media, OpenClaw passes
{ hostnameAllowlist: ["api.telegram.org"] }as the SSRF policy (the target's allowlist). The new check then validates the proxy hostname (10.200.0.1) against that same allowlist and rejects it because the proxy is notapi.telegram.org. Even withallowPrivateProxy: trueflippingallowPrivateNetwork,matchesHostnameAllowlist("10.200.0.1", ["api.telegram.org"])still returns false.This is an upstream OpenClaw design issue — a proxy is infrastructure, not a fetch target, so the target's hostname allowlist should not apply to it. NemoClaw's proxy-only architecture hits this 100% of the time.
This PR previously worked end-to-end against
openclaw@2026.3.11(which had noassertExplicitProxyAllowed). After main bumped to2026.4.2(#1522), only the layer 1 fix survived; the new SSRF check started rejecting every media download.Implementation scope
This PR contains four changes; their reach is deliberately not the same. Reviewers should know what each one actually affects.
proxy: <url>intoaccounts.defaultfetch-guard-*.jsexport so the strict-mode alias maps towithTrustedEnvProxyGuardedFetchModeassertExplicitProxyAllowed(active only whenOPENSHELL_SANDBOX=1, injected by OpenShell at pod startup)/sandbox/.openclaw-data/mediadirectory +workspace/mediasymlinkWhy Patches 1 and 2 are necessarily fetch-guard-wide: they operate on the openclaw module's exports, not on individual messaging channels. Inside NemoClaw's proxy-only sandbox, every fetch must go through the proxy and every fetch hits the same SSRF check. Narrowing these patches to "Telegram-only" would require a different patching strategy that NemoClaw maintains long-term. The wider reach is consistent with the sandbox's architectural constraint (proxy-only egress), so it is presented as intended behavior, not collateral.
Discord and Slack are explicitly out of scope. Their account configs are not touched by this PR. Any latent improvement they get from the fetch-guard patches inside the sandbox is incidental and not claimed here. If Discord/Slack media downloads also need restoring, they belong in a separate PR with their own end-to-end verification.
Fail-close design
Both sed patches use
set -euand explicit pre/post-grep verification:test -n "$matches"aborts if no candidate file matches.if grep ...; then exit 1; fithat nowithStrictGuardedFetchMode as <letter>export remains (Patch 1, written this way becauseset -edoes not propagate failure through!-prefixed commands).If a future OpenClaw bundle reorganizes the function or exports, the build fails at the next
OPENCLAW_VERSIONbump instead of silently producing broken media downloads. The next maintainer reviewing the bump PR sees the failure with full context in this Dockerfile comment.Removal criteria
withStrictGuardedFetchMode, or when all proxy-relevant callsites unconditionally passuseEnvProxy.assertExplicitProxyAllowedto skip the target's hostname allowlist when validating the proxy, or exposes config to disable the check. TheOPENSHELL_SANDBOXenv var is set by OpenShell, not by this PR — no extra cleanup needed.Testing
End-to-end on WSL2 + Docker Desktop, against
openclaw@2026.4.2(CI's pinned version):Build verification
docker buildon a minimal Dockerfile that pullsopenclaw@2026.4.2and runs the sameRUNblock) — patches apply cleanly, post-patch verification passes.docker buildwith the productionDockerfile(usingBASE_IMAGE=ghcr.io/nvidia/nemoclaw/sandbox-base:latest) succeeds. All 19 stages pass, noset -euaborts.End-to-end Telegram round-trip
Setup:
nemoclaw onboardwith Telegram channel enabled and a real bot token. Inference provider: Anthropicclaude-haiku-4-5(chosen because vision support is reliable, so a successful model description confirms the file actually reached the model intact, not just that download succeeded).getUpdates, calledPOST /bot[CREDENTIAL]/getFile(ALLOWED), thenGET /file/bot[CREDENTIAL]/photos/file_X.jpg(ALLOWED — the request that previously never reached the proxy).openshell sandbox downloadand visually confirmed the bytes match the source photo (motorcycle dashboard, then a Japanese curry menu — the bytes are not corrupted in transit).download → save → read tool → model vision pipelineworks.MediaFetchError, noBlocked hostname, noFailed to download mediain any log layer.NEMOCLAW_SANDBOX=1) — that var did not propagate into the OpenShell sandbox runtime, so the gate stayed false and the bug recurred. Switched the gate toOPENSHELL_SANDBOX(injected by OpenShell at pod startup) and the e2e flow became reliable.Type of Change
Verification
npx prek run --all-filespassesnpm testpasses (pre-existing CLI lifecycle-recovery test failures reproduce onorigin/main— unrelated)make docsbuilds without warnings (doc changes only)AI Disclosure
Signed-off-by: Tommy Lin tommylin@signalpro.com.tw