docs: clarify Discord preset validation

[ericksoa]

Summary

• add Discord-specific policy-add guidance that curl is not a valid success signal because it is outside the preset binary allowlist
• document Node HTTPS probes for Discord REST/CDN validation behind proxy and call out DNS-only checks as inconclusive
• cover the direct warning helper plus interactive and named policy-add discord flows

Reopen Context

Issue #3477 was closed by #3663, then reopened from a v0.0.44 QA repro that still used curl and dns.resolve as validation signals. That same repro showed Node HTTPS to https://discord.com/api/v10/gateway returning 200, which is the signal this PR now documents and surfaces in policy-add.

Related Issue

Fixes #3477

Verification

• npm run build:cli
• npm test -- test/policies.test.ts
• npm test -- test/validate-blueprint.test.ts
• make docs

Signed-off-by: Aaron Erickson aerickson@nvidia.com

Summary by CodeRabbit

Documentation

• Added troubleshooting guidance for Discord preset validation behind proxies: explains why curl probes can mislead, clarifies the built-in preset’s behavior, warns about DNS-only checks, and provides Node HTTPS one-liners to verify Discord API/CDN egress and next investigative steps.

Tests

• Extended coverage to assert Discord preset validation warnings and to verify interactive and non-interactive policy-add flows print the new guidance and proceed with apply as expected.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c0e0eac6-a12d-47ed-ad7b-8892cfb47404

📥 Commits

Reviewing files that changed from the base of the PR and between 03a75e6 and 8443f40.

📒 Files selected for processing (1)

• test/policies.test.ts

---

📝 Walkthrough

Walkthrough

Adds Discord-specific proxy-aware validation guidance to messaging preset warnings, updates tests to cover the new warning and policy-add flows, and inserts troubleshooting docs with Node HTTPS probes for verifying Discord API/CDN egress behind proxies.

Changes

Discord Preset Proxy Validation

Layer / File(s)	Summary
Discord validation warning implementation src/lib/policy/index.ts	getMessagingPresetWarning builds warning text from a lines array and conditionally appends a Discord-only note advising against curl probes and recommending Node HTTPS and DNS checks when presetName === "discord".
Test coverage for Discord warnings and policy-add flow test/policies.test.ts	Mock preset list updated to include discord; unit test verifies getMessagingPresetWarning("discord") contains Node probe guidance, gateway URL, and dns.resolve("gateway.discord.gg") snippet; two end-to-end policy-add tests confirm guidance prints during interactive selection and --yes flows and that applyPreset is invoked with prompt suppression in the non-interactive case.
Troubleshooting documentation for proxy scenarios docs/reference/troubleshooting.mdx	New subsection explains why curl and DNS-only checks are unreliable behind the proxy, provides Node HTTPS one-liners to verify Discord API and CDN egress, and directs investigators to native gateway/proxy path logs when REST probes succeed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• NVIDIA/NemoClaw#3663: Related work on Discord preset validation and tests demonstrating curl denial vs Node allowance through Discord proxy/whitelist paths.

Suggested labels

NemoClaw CLI, Integration: Discord, enhancement: policy

Poem

🐰 Behind the proxy the messages hide,
I hop with Node and a one-line guide.
No curl for truth, DNS can lie,
Probe with HTTPS and read the sky—
Gateway logs will show where connections die.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: adding Discord-specific validation guidance to documentation and policy-add flows.
Linked Issues check	✅ Passed	All coding objectives from issue #3477 are met: documentation clarifies curl limitations, documents Node HTTPS validation, updates policy-add guidance, and includes DNS-resolve examples.
Out of Scope Changes check	✅ Passed	All changes directly support the linked issue objectives: documentation updates, policy warning implementation, and test coverage align with the stated scope.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/discord-preset-validation-3477

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

🌿 Preview your docs: https://nvidia-preview-pr-3934.docs.buildwithfern.com/nemoclaw

[github-actions]

E2E Advisor Recommendation

Required E2E: None
Optional E2E: network-policy-e2e, messaging-providers-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• None. No merge-blocking E2E is recommended because the runtime change is limited to CLI/help warning text for the Discord messaging preset and documentation; it does not modify preset YAML, policy enforcement, sandbox lifecycle, credentials, onboarding, or messaging runtime behavior. Added unit tests cover the warning content and policy-add output paths.

Optional E2E

• network-policy-e2e (medium): Optional confidence for the touched policy CLI area: exercises policy-add, dry-run, live hot-reload, restricted policy behavior, and SSRF validation. The PR only changes warning text, so this is not merge-blocking.
• messaging-providers-e2e (high): Optional adjacent coverage for the Discord guidance: validates the messaging provider/proxy chain and includes Discord REST/gateway behavior plus Node-vs-curl binary-whitelist proof. Broad and expensive for a warning-text change, so not required.

New E2E recommendations

• None.

[github-actions]

PR Review Advisor

Recommendation: blocked
Confidence: medium
Analyzed HEAD: 21bcf46a28c3bce29252ebe0809eb2d6abf2dcf6
Findings: 2 blocker(s), 2 warning(s), 2 suggestion(s)

This is an automated advisory review. A human maintainer must make the final merge decision.

Limitations: Review used read-only repository inspection and deterministic trusted GitHub context; tests, scripts, package-manager commands, and E2E jobs were not executed by this advisor.; CI was still pending for head SHA 21bcf46, so final required-job status is unknown.; An E2E Advisor comment exists, but the current-head E2E recommendation check is still in progress; required E2E status may change when it finishes.; Linked issue and PR text were treated as untrusted evidence and used only for acceptance mapping.; Open PR overlaps touch the same files, so maintainers should watch for merge conflict or behavior drift before final merge.

Workflow run

Full advisor summary

PR Review Advisor

Base: origin/main
Head: HEAD
Analyzed SHA: 21bcf46a28c3bce29252ebe0809eb2d6abf2dcf6
Recommendation: blocked
Confidence: medium

The patch is narrow and appears security-benign, but merge is blocked by pending CI and GitHub mergeStateStatus=BLOCKED for head SHA 21bcf46.

Gate status

• CI: pending — Trusted context reports 13 status context(s) appear pending for head SHA 21bcf46; GraphQL rollup shows in-progress/queued jobs including cli-parity, preview, E2E recommendation, wsl-e2e, macos-e2e, PR review advisor, CodeQL, unit-vitest-linux, checks, ShellCheck SARIF, and sandbox image builds.
• Mergeability: fail — GitHub reports mergeStateStatus=BLOCKED and REST mergeable_state=blocked for head SHA 21bcf46.
• Review threads: pass — Trusted context reports 1 review thread(s), all resolved; GraphQL reviewThreads node isResolved=true for the CodeQL regex discussion.
• Risky code tested: pass — Trusted path heuristics reported no risky code areas; the source change adds Discord-specific warning text and tests without changing policy rules, binary allowlists, network enforcement, credentials, workflows, Dockerfiles, installers, or sandbox lifecycle code.

🔴 Blockers

• Required CI is still pending for the current head SHA: The PR cannot be treated as merge-ready while multiple required or relevant status contexts are queued or in progress for head SHA 21bcf46.
  • Recommendation: Wait for all required CI, CodeQL, unit, docs/preview, E2E recommendation, PR review advisor, and sandbox image jobs to complete successfully for the current head SHA before considering merge.
  • Evidence: Trusted gateStatus.ci says 13 status context(s) appear pending; GraphQL rollup lists in-progress/queued cli-parity, preview, E2E recommendation, wsl-e2e, macos-e2e, PR review advisor, CodeQL jobs, unit-vitest-linux, checks, ShellCheck SARIF, build-sandbox-images, and build-sandbox-images-arm64.
• GitHub mergeability gate is blocked: GitHub reports the PR merge state as blocked. This is a hard gate independent of the narrowness of the patch.
  • Recommendation: Do not consider the PR merge-ready until GitHub mergeStateStatus and repository branch-protection requirements are satisfied for head SHA 21bcf46.
  • Evidence: Trusted gateStatus.mergeability: mergeStateStatus=BLOCKED; REST mergeable_state=blocked.

🟡 Warnings

• Current-head E2E recommendation is not complete: An E2E Advisor comment exists and recommends no required E2E, with optional network-policy-e2e and messaging-providers-e2e, but the current head rollup shows the E2E recommendation check still in progress. The current-head E2E status is therefore ambiguous.
  • Recommendation: Use the completed E2E Advisor result for head SHA 21bcf46. If it still recommends no required E2E, no merge-blocking E2E jobs are needed; otherwise verify any required jobs passed for this SHA.
  • Evidence: E2E Advisor comment says Required E2E: None and Optional E2E: network-policy-e2e, messaging-providers-e2e; GraphQL statusCheckRollup for current head includes check run 'E2E recommendation' status IN_PROGRESS.
• Linked issue's explicit proxy-403 disposition is only partially documented (docs/reference/troubleshooting.mdx): The docs and CLI guidance now explain why curl and DNS-only checks can mislead and recommend Node HTTPS probes, but several linked issue clauses asked for explicit handling of external proxy HTTP 403/curl error 56 and marking the test as blocked by network policy rather than a sandbox policy bug. That exact operational disposition is not present in the diff.
  • Recommendation: Consider adding one sentence to the troubleshooting section that explicitly says environments where an external proxy returns 4xx/HTTP 403 on CONNECT to discord.com or cdn.discordapp.com should mark the Discord preset test as blocked by network policy, not as a sandbox policy bug.
  • Evidence: Issue [NemoClaw][Linux][Discord] Discord preset connectivity test fails behind proxy: outbound CONNECT to discord.com / cdn.discordapp.com returns HTTP 403 (curl error 56) #3477 asks for: 'In environments where an external proxy returns 4xx on CONNECT for these domains (e.g., HTTP 403), step 4–5 will fail with curl error 56, and the test should be marked ‘blocked by network policy’, not ‘sandbox policy bug’.' The added docs mention curl/DNS limitations and Node HTTPS probes, but do not mention HTTP 403, curl error 56, or the 'blocked by network policy' marking.

🔵 Suggestions

• Policy module monolith grew slightly (src/lib/policy/index.ts:117): The policy module is already large and this patch adds more preset-specific user-facing guidance inline. This is not blocking, but the module continues to accumulate policy UI behavior.
  • Recommendation: If preset-specific guidance continues to grow, consider extracting messaging preset guidance into a small table/helper colocated with preset metadata.
  • Evidence: Trusted monolith delta reports src/lib/policy/index.ts grew from 1101 to 1114 lines, delta 13, severity warning.
• Watch for active-work overlap before final merge: The patched files still exist and have no rename hints, but multiple open PRs touch the same files. This does not supersede the PR, but maintainers should watch for conflict or behavior drift as CI and review complete.
  • Recommendation: Before merge, re-check that overlapping PRs have not changed policy-add behavior, Discord guidance, or troubleshooting structure in a contradictory way.
  • Evidence: Trusted openPrOverlaps include PRs fix(sandbox): replace brew policy preset with first-class brew subcommand #3878 and chore: upgrade agent runtime dependencies #3925 touching src/lib/policy/index.ts and test/policies.test.ts, plus docs(troubleshooting): add Hermes section #3658, Docs/comprehensive documentation cleanup and v0.0.44 synchronization #3766, and fix(onboard): reject host.docker.internal inference URLs #3804 touching docs/reference/troubleshooting.mdx.

Acceptance coverage

• partial — When a NemoClaw sandbox is configured with the Discord policy preset, outbound HTTPS traffic to discord.com and cdn.discordapp.com from inside the sandbox is blocked by the environment’s HTTPS proxy (10.200.0.1:3128) with HTTP 403 on CONNECT, so the Discord preset connectivity tests cannot pass as written.: docs/reference/troubleshooting.mdx adds 'Discord preset validation behind a proxy' and explains curl/DNS-only signals may be wrong behind the OpenShell proxy, but it does not explicitly document proxy 10.200.0.1:3128 or HTTP 403 CONNECT denial.
• partial — Instead of seeing local policy‑layer behavior (curl blocked by binary whitelist, Node allowed), all traffic is rejected upstream by the proxy.: src/lib/policy/index.ts and docs/reference/troubleshooting.mdx distinguish curl from Node HTTPS probes, but do not explicitly state the proxy-rejects-all-traffic case.
• partial — This prevents validating the Discord preset’s intended egress and binary‑whitelist behavior in this environment.: The new docs state curl and DNS-only checks can be the wrong signal behind a proxy; they do not explicitly say the preset's intended egress and binary-whitelist behavior cannot be validated in that environment.
• partial — This is an environment‑dependent failure of the documented test case (proxy policy vs. preset expectations), not a NemoClaw gateway crash, but it blocks the QA scenario and may warrant either (a) relaxing proxy rules in the test lab or (b) updating the test plan to account for proxy‑restricted deployments.: The PR implements the documentation/test-plan path by adding docs guidance and CLI warning text; it does not change lab proxy policy or explicitly state the QA scenario disposition.
• met — Step 4: curl -s --max-time 10 https://discord.com inside the sandbox should be blocked locally by NemoClaw/OpenClaw policy because curl is not in the binary whitelist for Discord.: src/lib/policy/index.ts getMessagingPresetWarning now says 'do not use curl as the success signal' because 'curl is not in the preset binary allowlist'; docs/reference/troubleshooting.mdx says the Discord preset allows Node binaries and does not allow curl.
• partial — Step 5: https://cdn.discordapp.com should be reachable via Node for GET‑only access (status 200 or 301) when using the allowed Node binaries; curl behavior is not the primary signal here, but the policy assumes CDN access is possible when the environment allows it.: docs/reference/troubleshooting.mdx adds a Node HTTPS probe for https://cdn.discordapp.com/ and states any HTTP status means the Node process reached the endpoint; it does not specifically retain the 200/301 expected-status wording.
• met — curl blocked by binary whitelist when hitting Discord,: Both src/lib/policy/index.ts and docs/reference/troubleshooting.mdx explicitly say curl is not in the Discord preset binary allowlist and curl probes can fail even when policy is working.
• met — Node allowed where specified,: Docs add Node HTTPS probes for Discord REST and CDN; tests assert the Discord warning contains 'Node HTTPS' and https://discord.com/api/v10/gateway.
• partial — and external network conditions assumed permissive enough that Discord/CDN allow basic HTTPS.: Docs now explain proxy-aware validation and endpoint reachability via Node, but do not explicitly state the original external-network permissiveness assumption in the same terms.
• partial — Both curl commands go through the proxy:: Docs explain curl failing/hanging/printing no output is not proof and can be the wrong signal, but do not explicitly document curl going through the proxy or show CONNECT traces.
• missing — The proxy is actively denying CONNECT to discord.com:443 and cdn.discordapp.com:443 with HTTP 403.: No changed file explicitly mentions HTTP 403 CONNECT denial for discord.com:443 or cdn.discordapp.com:443.
• partial — The test does not exhibit a local “blocked by binary whitelist (000)” behavior; instead, both domains are blocked at the egress proxy, and the sandbox cannot talk to Discord at all, regardless of binary whitelist.: The warning distinguishes curl from Node HTTPS validation, but the docs do not explicitly describe the 'blocked at egress proxy regardless of binary whitelist' scenario.
• partial — The Discord preset test’s expectations for steps 4–5 cannot be validated in this environment because the external proxy’s policy supersedes NemoClaw/OpenClaw’s sandbox policy behavior.: The added section says curl and DNS-only checks can be wrong behind a proxy and recommends Node HTTPS validation, but does not explicitly say the test is unvalidatable when proxy policy supersedes sandbox policy.
• partial — You cannot observe “curl blocked by sandbox binary whitelist” vs “Node allowed” distinctions; the proxy 403s both equally.: The warning distinguishes curl from Node HTTPS validation, but does not explicitly document the proxy-403s-both-equally case.
• partial — Any higher‑level behavior that assumes Discord/CDN reachability (e.g., ws‑proxy‑fix or WebSocket gateway checks) will also fail for environmental reasons, not due to NemoClaw policy bugs.: Docs say if the Node REST probe works but Discord remains unhealthy, investigate gateway logs and WebSocket upgrade failures instead of widening the preset; they do not mention ws-proxy-fix or explicitly label such failures environmental.
• missing — This blocks QA from asserting that the Discord preset behaves as documented, unless the environment is changed or the test is marked as “not applicable behind a proxy that denies Discord”.: The PR adds troubleshooting guidance but does not add the explicit QA disposition language 'not applicable behind a proxy that denies Discord'.
• unknown — Environment / Infra change (preferred for full validation):: No infrastructure/proxy policy files are changed; changed files are docs/reference/troubleshooting.mdx, src/lib/policy/index.ts, and test/policies.test.ts.
• unknown — Update the HTTP(S) proxy policy for the discord-pre lab environment to allow: textCONNECT discord.com:443 CONNECT cdn.discordapp.com:443: No lab proxy policy change is present in the diff.
• met — Test‑plan / Documentation update:: docs/reference/troubleshooting.mdx adds a Discord preset validation section; src/lib/policy/index.ts adds Discord-specific policy-add warning guidance.
• partial — “This test assumes outbound HTTPS to discord.com and cdn.discordapp.com is permitted by the host’s egress policy. In environments where an external proxy returns 4xx on CONNECT for these domains (e.g., HTTP 403), step 4–5 will fail with curl error 56, and the test should be marked ‘blocked by network policy’, not ‘sandbox policy bug’.”: The docs address the core curl/DNS vs Node HTTPS validation distinction, but do not add the exact HTTP 403/curl error 56/'blocked by network policy' marking guidance.

Security review

• pass — 1. Secrets and Credentials: No hardcoded secrets, API keys, tokens, passwords, PEMs, credential JSON, or .env files are added. The Node HTTPS examples do not include credentials.
• pass — 2. Input Validation and Data Sanitization: The code change adds static warning text selected by an existing preset-name lookup. It does not introduce new parsing of untrusted input, command construction, deserialization, path handling, URL validation, SSRF-sensitive network behavior, or shell execution.
• pass — 3. Authentication and Authorization: No endpoints, authentication, authorization, token validation, scopes, or resource ownership checks are modified.
• pass — 4. Dependencies and Third-Party Libraries: No dependencies, package manifests, lockfiles, registries, installers, or third-party trust boundaries are changed.
• pass — 5. Error Handling and Logging: No application error-handling or logging paths are changed. Documentation examples print generic error messages and do not log secrets.
• pass — 6. Cryptography and Data Protection: Not applicable — no cryptographic operations, algorithms, key handling, or data-protection mechanisms are changed.
• pass — 7. Configuration and Security Headers: No HTTP server headers, CORS/CSP, Dockerfiles, container permissions, port exposure, runtime defaults, or actual policy preset egress/binary allowlists are changed.
• warning — 8. Security Testing: Unit tests cover the new warning helper and policy-add display paths, including interactive and named preset flows. However, CI is still pending and the current-head E2E recommendation is in progress/ambiguous, so final security regression coverage has not completed.
• pass — 9. Holistic Security Posture: The PR appears to reduce misleading validation by steering users away from curl/DNS-only signals and toward allowlisted Node HTTPS probes. It does not add sandbox escape, SSRF bypass, policy bypass, credential leakage, blueprint tampering, installer trust, or workflow trusted-code-boundary changes.

Test / E2E status

• Test depth: e2e_required — Trusted classifier marked runtime/sandbox/infrastructure paths as needing real execution coverage for docs/reference/troubleshooting.mdx and src/lib/policy/index.ts. The diff adds targeted unit coverage for warning text and mocked policy-add flows, but current-head CI/E2E recommendation is not complete.
• E2E Advisor: ambiguous
• Missing for analyzed SHA: E2E recommendation

✅ What looks good

• The patch is narrowly scoped to Discord preset validation guidance, one warning helper, and targeted tests.
• The Discord preset itself is not widened; no new egress hosts, binary allowlist entries, credentials, network policy rules, provider wiring, Dockerfiles, installers, or workflows are added.
• Tests cover the direct getMessagingPresetWarning helper plus interactive and named policy-add Discord flows, including prompt suppression for --yes.
• The latest test assertion avoids the earlier URL regex concern by using string containment for the gateway URL, and the GitHub review thread is resolved in trusted context.
• Docs provide concrete Node HTTPS probes for Discord REST and CDN validation and discourage misleading curl/DNS-only conclusions.
• Codebase drift check found all patched files still exist with no rename hints; overlaps exist but no duplicate linked issue was identified.

Review completeness

• Review used read-only repository inspection and deterministic trusted GitHub context; tests, scripts, package-manager commands, and E2E jobs were not executed by this advisor.
• CI was still pending for head SHA 21bcf46, so final required-job status is unknown.
• An E2E Advisor comment exists, but the current-head E2E recommendation check is still in progress; required E2E status may change when it finishes.
• Linked issue and PR text were treated as untrusted evidence and used only for acceptance mapping.
• Open PR overlaps touch the same files, so maintainers should watch for merge conflict or behavior drift before final merge.
• Human maintainer review required: yes