fix(dcode): allow plain OTLP endpoint URLs past the secret guard#6538
Merged
Conversation
The managed Deep Agents Code secret guard treated the OTLP endpoint variables (OTEL_EXPORTER_OTLP_ENDPOINT and its _TRACES_ variant) as credential-named vars and refused any value >=10 chars, so every real collector URL set by the documented `--observability` flow (e.g. http://host.openshell.internal:4318) was rejected and dcode refused to start with a false-positive secret error. Both the Bash wrapper and the managed Python runtime carried the same over-broad rule. Treat the _ENDPOINT variables as URLs, not credentials: accept a clean bare http(s) URL, but still refuse a value with embedded userinfo (scheme://user:pass@host) or a structured key-bearing blob, mirroring the existing OpenClaw OTEL config contract. Token-shaped material anywhere in the value is still caught by the value-shape scan that runs first. The _HEADERS variants stay under the credential-name refusal because they carry auth material. Fixes #6466 Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
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:
📝 WalkthroughWalkthroughThis change fixes false-positive secret detection for ChangesOTLP Endpoint Credential Scanner Fix
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant dcode-wrapper.sh
participant managed-dcode-runtime.py
User->>dcode-wrapper.sh: Launch dcode with OTEL_EXPORTER_OTLP_ENDPOINT
dcode-wrapper.sh->>dcode-wrapper.sh: is_otlp_endpoint_name(name)
dcode-wrapper.sh->>dcode-wrapper.sh: is_safe_otlp_endpoint_url(value)
dcode-wrapper.sh->>managed-dcode-runtime.py: Start process with env value
managed-dcode-runtime.py->>managed-dcode-runtime.py: _assert_safe_environment()
managed-dcode-runtime.py->>managed-dcode-runtime.py: _is_safe_otlp_endpoint_url(value)
alt plain managed URL
managed-dcode-runtime.py-->>User: Process starts
else credential-shaped value
dcode-wrapper.sh-->>User: Refuse start
end
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
E2E Advisor RecommendationRequired E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
![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: 1
🧹 Nitpick comments (1)
test/langchain-deepagents-code-image-credentials.test.ts (1)
89-89: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMissing
(#6466)suffix in test title.The issue reference is only in the inline comment, not the title itself.
✏️ Proposed fix
- it("allows plain OTLP endpoint URLs but rejects credential-bearing endpoint values", () => { + it("allows plain OTLP endpoint URLs but rejects credential-bearing endpoint values (`#6466`)", () => {As per coding guidelines, "
**/*.test.ts: Write behavior-oriented test titles, and put local issue references in a final(#1234)suffix."🤖 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 `@test/langchain-deepagents-code-image-credentials.test.ts` at line 89, The test title in the behavior-oriented spec should include the local issue reference as a final suffix. Update the `it(...)` description in `langchain-deepagents-code-image-credentials.test.ts` so it ends with `(`#6466`)`, keeping the existing behavior-focused wording in place and preserving the test intent.Source: Coding guidelines
🤖 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 `@agents/langchain-deepagents-code/dcode-wrapper.sh`:
- Around line 490-494: Allow empty OTLP endpoint values in the OTLP endpoint
handling paths inside dcode-wrapper.sh. In the is_otlp_endpoint_name branch,
make sure an empty $value bypasses is_bare_http_url_without_userinfo so
OTEL_EXPORTER_OTLP_ENDPOINT= is treated as unset instead of rejected. Apply the
same empty-value guard at both OTLP endpoint wiring sites, keeping the existing
refuse_secret_env behavior only for non-empty values.
---
Nitpick comments:
In `@test/langchain-deepagents-code-image-credentials.test.ts`:
- Line 89: The test title in the behavior-oriented spec should include the local
issue reference as a final suffix. Update the `it(...)` description in
`langchain-deepagents-code-image-credentials.test.ts` so it ends with `(`#6466`)`,
keeping the existing behavior-focused wording in place and preserving the test
intent.
🪄 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: Enterprise
Run ID: 4d0ff75d-ffb5-4b17-8ff5-99d127e21083
📒 Files selected for processing (4)
agents/langchain-deepagents-code/dcode-wrapper.shagents/langchain-deepagents-code/managed-dcode-runtime.pytest/langchain-deepagents-code-direct-module-patch.test.tstest/langchain-deepagents-code-image-credentials.test.ts
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the branch. TypeScript / code-coverage/cliThe overall coverage in the branch remains at 76%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Updated |
Contributor
PR Review Advisor (Nemotron Ultra) — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
Review findings by urgency: 0 required fixes, 4 items to resolve/justify, 2 in-scope improvements
|
Contributor
PR Review Advisor — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
Review findings by urgency: 0 required fixes, 1 item to resolve/justify, 0 in-scope improvements
|
Follow-up to review: an empty OTEL_EXPORTER_OTLP_ENDPOINT= is unset, not a credential. The managed Python runtime already skips the URL check for an empty value; make the Bash wrapper consistent at both OTLP endpoint sites and only scan a non-empty value. Adds an empty-value acceptance case to the wrapper test. Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
cv
requested changes
Jul 9, 2026
cv
left a comment
cv
left a comment
Collaborator
There was a problem hiding this comment.
Exact-head security review at 245da0d137 found a merge-blocking fail-open in the new OTLP endpoint exception.
The helpers at agents/langchain-deepagents-code/dcode-wrapper.sh:379-393 and agents/langchain-deepagents-code/managed-dcode-runtime.py:243-257 check only the scheme prefix, a small delimiter set, and literal @; they do not establish one canonical credential-free URL. Full-entry probes are accepted by the direct module, wrapper runtime, and wrapper dotenv paths for percent-encoded known-token query material (?x=sk%2D...), opaque query credentials (?apikey=opaquevalue12345), percent-encoded userinfo-like authorities, hostless http://, semicolon-joined URLs, and ESC controls. Python also accepts VT/FF values that Bash rejects. Literal userinfo and raw ?token= are rejected, but the encoded and parser-differential cases contradict the claimed trust boundary.
The managed runtime later strips these OTLP variables before launching the child, so this review did not confirm credential exfiltration. The classifier is still materially fail-open and its tests currently overstate the assurance.
Please replace the hand-rolled predicate with canonical parsing or a conservative documented-endpoint allowlist. At minimum require a non-empty valid host/port, reject userinfo/query/fragment, reject all C0 controls/DEL/non-ASCII/backslashes/oversized inputs, and apply the secret scan to a safely decoded or normalized representation with Bash/Python parity. Add the adversarial corpus for both endpoint names across runtime, dotenv, and direct-module paths; rejected wrapper cases should exit exactly 2, never run the stub, and never echo the value.
The prior OTLP endpoint predicate checked only a scheme prefix, a small delimiter set, and literal `@`, so it accepted several credential-bearing forms: percent-encoded tokens (`?x=sk%2D...`), opaque query credentials (`?apikey=...`), percent-encoded userinfo, hostless `http://`, semicolon-joined URLs, and control chars. Bash and Python also disagreed on VT/FF because the Bash side used `[[:space:]]` while Python spelled out a narrower set. Replace both the Bash `is_safe_otlp_endpoint_url` and the Python `_is_safe_otlp_endpoint_url` with a strict allowlist that accepts only a bounded `scheme://host[:port][/path]` URL built from an identical ASCII charset, rejecting userinfo, query, fragment, percent-encoding, C0 controls, DEL, non-ASCII, backslashes, and oversized inputs. The Bash side runs under `LC_ALL=C` so locale collation cannot fold non-ASCII bytes into `[A-Za-z0-9]`, keeping byte-for-byte parity with Python. The value-shape secret scan still runs first, so token material in any accepted field is still caught. Adds adversarial accept/reject coverage for both the wrapper and the managed runtime that would have caught each fail-open case. Fixes #6466 Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
Contributor
Author
|
Thanks for the careful review — you're right, the predicate was fail-open. Replaced the hand-rolled check in both
Added adversarial accept/reject coverage that exercises the real wrapper and the real managed runtime for each of the cases you listed (percent-encoded token, opaque query cred, encoded userinfo, hostless, semicolon-joined, fragment, non-ASCII host, oversized). Ready for another look. |
cv
requested changes
Jul 9, 2026
cv
left a comment
cv
left a comment
Collaborator
There was a problem hiding this comment.
Exact-head follow-up at b3739eacd3: the new conservative grammar and adversarial tests do fix the original encoded/query/fragment/userinfo/control fail-open corpus across the Python module and wrapper runtime paths. Thank you.
Three merge-blocking pieces remain:
- The prior request required a non-empty valid host/port. The current
[A-Za-z0-9.-]+(:[0-9]+)?grammar still accepts malformed authorities such as.,-,a..b, leading/trailing dots or hyphens, invalid IPv4 text, and ports0,65536, or arbitrarily large decimal strings. Please validate a real DNS/IPv4 host and a port in the supported range, with identical Bash/Python behavior. If path dot segments are intentionally allowed, document that boundary; otherwise reject them too. - Pin the full wrapper refusal contract in committed tests, not only local probes: exact status
2, stub/ran marker absent, variable name present, and rejected value absent. Cover both endpoint names for runtime and dotenv invalid values, plus dotenv empty values. The exact-head advisor independently flags these as PRA-1/PRA-2. - The “all C0 controls with Bash/Python parity” clause is not yet true for dotenv input: raw NUL is silently dropped by Bash and quoted trailing TAB/VT/FF/CR can be trimmed rather than rejected. Add byte-level dotenv coverage and fail closed before normalization, or narrow and justify the claimed boundary.
Local exact-head evidence: 185 submitted tests pass, and the original adversarial corpus now refuses with status 2 and no value echo. The required ubuntu-repo-cloud-langchain-deepagents-code target is still outstanding; it is reasonable to run that after these code/test items land.
…ract Address the exact-head review on the OTLP endpoint exception: - Host/port: replace the permissive `[A-Za-z0-9.-]+(:[0-9]+)?` grammar with an exact-host allowlist of the one collector a managed sandbox can reach (`host.openshell.internal`), plus an in-range 1..65535 port and a clean path. This refuses malformed hosts (`.`, `-`, `a..b`, `999.999.999.999`), subdomain/suffix confusion, and ports `0`/`65536`/oversized by construction, with trivial Bash/Python parity (verified byte-for-byte on a 31-case adversarial corpus). Path dot segments are documented as intentional: the path reaches only the exact managed host. - Dotenv controls: fail closed on any C0 control/DEL carried in the raw dotenv value before trim/unquote could silently strip a smuggled trailing TAB/VT/FF/ESC/CR. NUL cannot reach the check (Bash drops it), noted in the boundary comment. - Tests: pin the full refusal contract in committed tests — exact status 2, variable name present, rejected value absent (no echo), no run — across both endpoint names and both the runtime and dotenv paths, plus dotenv empty-value acceptance and byte-level control coverage. Fixes #6466 Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
…VIDIA/NemoClaw into fix/dcode-otlp-endpoint-secret-fp-6466
Contributor
Author
|
Thanks — addressed all three: 1. Valid host/port. Rather than hand-roll DNS/IPv4/port validation in two runtimes (a parity hazard), I restricted to the one collector a managed sandbox can actually reach: the observability egress preset opens exactly 2. Full refusal contract in committed tests. 3. Dotenv controls fail closed. The dotenv branch now rejects any C0 control/DEL carried in the raw value before trim/unquote could strip a smuggled trailing TAB/VT/FF/ESC/CR, with byte-level test coverage. NUL cannot reach the check — Bash drops it from the line at read time — which I've noted as an explicit boundary in the comment. Local: the touched suites pass and the adversarial corpus refuses with status 2 and no value echo. Agreed the |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/langchain-deepagents-code-image-credentials.test.ts (1)
110-111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReject-case comments overstate what's exercised.
http://999.999.999.999:4318andhttp://héllo:4318are already rejected by the exact-host allowlist check (host ≠host.openshell.internal) before any IPv4/non-ASCII-specific validation would run, per the PR's "exact host matching" design. The inline comments ("invalid IPv4", "non-ASCII host") imply these test dedicated format-validation branches, but they're functionally redundant with the "non-managed host" case at Line 101. Not a correctness bug — the security boundary is still verified — but if isolating those specific sub-checks matters, use a value with the correct host and a malformed remainder (e.g.,http://host.openshell.internal:999.999.999.999or non-ASCII path segment).As per path instructions, "Flag copied production algorithms, broad mocks that bypass the behavior under test, and conditionals that make a test pass without exercising its claim."
🤖 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 `@test/langchain-deepagents-code-image-credentials.test.ts` around lines 110 - 111, The reject-case assertions in the credential URL tests are overstating what they cover, because `http://999.999.999.999:4318` and `http://héllo:4318` are rejected earlier by the exact-host allowlist logic in the same test flow. Update the cases near the existing `host.openshell.internal` checks in `test/langchain-deepagents-code-image-credentials.test.ts` so they actually exercise the intended IPv4/non-ASCII validation branch, or remove the misleading inline labels if you want them to remain generic non-managed-host rejections.Source: Path instructions
🤖 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.
Nitpick comments:
In `@test/langchain-deepagents-code-image-credentials.test.ts`:
- Around line 110-111: The reject-case assertions in the credential URL tests
are overstating what they cover, because `http://999.999.999.999:4318` and
`http://héllo:4318` are rejected earlier by the exact-host allowlist logic in
the same test flow. Update the cases near the existing `host.openshell.internal`
checks in `test/langchain-deepagents-code-image-credentials.test.ts` so they
actually exercise the intended IPv4/non-ASCII validation branch, or remove the
misleading inline labels if you want them to remain generic non-managed-host
rejections.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1b911a7e-5e5b-4c69-be7c-4a83dd637a41
📒 Files selected for processing (4)
agents/langchain-deepagents-code/dcode-wrapper.shagents/langchain-deepagents-code/managed-dcode-runtime.pytest/langchain-deepagents-code-direct-module-patch.test.tstest/langchain-deepagents-code-image-credentials.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- test/langchain-deepagents-code-direct-module-patch.test.ts
- agents/langchain-deepagents-code/dcode-wrapper.sh
Contributor
E2E Target Results — ✅ All selected jobs passedRun: 29001622636
|
cv
approved these changes
Jul 9, 2026
cv
pushed a commit
that referenced
this pull request
Jul 9, 2026
<!-- markdownlint-disable MD041 --> ## Summary Adds the pre-tag v0.0.79 release notes entry to `docs/about/release-notes.mdx` so the release plan can be generated after docs merge. The entry summarizes the merged v0.0.79 release train across inference, diagnostics, runtime hardening, policies, onboarding recovery, and release validation. ## Changes - Added the v0.0.79 release notes section with linked follow-up documentation for OpenRouter onboarding, managed vLLM changes, completion and logging, Deep Agents runtime limits, policy updates, onboarding recovery, and release validation. - Source summary: - #6461 -> `docs/about/release-notes.mdx`: Documents OpenRouter onboarding support and links to inference/provider references. - #6271 and #6272 -> `docs/about/release-notes.mdx`: Documents shell completion and structured logging highlights. - #6465, #6539, #6570, and #6528 -> `docs/about/release-notes.mdx`: Documents status route-drift, orphaned sandbox, gateway cleanup, and DGX Spark express-install diagnostics. - #6523, #6551, #6484, #6488, #6324, and #6542 -> `docs/about/release-notes.mdx`: Documents managed vLLM, Qwen3.6 tool parser, compaction, and timeout/readiness improvements. - #6559, #6538, #6560, #6568, #6552, #6567, and #6587 -> `docs/about/release-notes.mdx`: Documents runtime, credential, proxy, PID namespace, TOML, and provider-state hardening. - #6541, #5415, #6246, #6496, and #6573 -> `docs/about/release-notes.mdx`: Documents GitHub policy, Gmail policy, MCP allowlist, WhatsApp, and messaging-variant updates. - #6253, #6572, #6444, #6536, and #5860 -> `docs/about/release-notes.mdx`: Documents onboarding resume and create-step recovery improvements. - #6508, #6527, #5506, #6588, #6446, #6447, #6582, #6296, #6367, #6397, and #6505 -> `docs/about/release-notes.mdx`: Documents docs, release-risk, and E2E validation updates. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: Release-note prose only. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: Tests not applicable, release-note prose only. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Docs validation note: `npm run docs:check-agent-variants && npm run docs:check-routes && git diff --check` passed. Full `npm run docs` is currently blocked before Fern validation because the pinned `fern-api@5.65.2` package is unavailable from npm (`ETARGET No matching version found`). --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.79 with a new summary of recent improvements, including onboarding and inference options, operator/CLI diagnostics, sandbox recovery hardening, runtime limits, network policy behavior, and release validation updates. * Added updated references and links for the latest release. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
…DIA#6538) <!-- markdownlint-disable MD041 --> ## Summary The managed Deep Agents Code secret guard rejected any non-empty `OTEL_EXPORTER_OTLP_ENDPOINT` / `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` value, so a plain collector URL set by the documented `--observability` flow made `dcode` refuse to start with a false-positive secret error. This PR treats the OTLP `_ENDPOINT` variables as URLs (accept a clean bare http(s) URL, still refuse credential-bearing forms) in both the Bash wrapper and the managed Python runtime. Closes NVIDIA#6466. ## Reproduction On a fresh dcode `--observability` sandbox: ```bash nemoclaw <sandbox> exec -- bash -c "export OTEL_EXPORTER_OTLP_ENDPOINT=http://host.openshell.internal:4318; dcode -n 'Reply with the single word traced.'" ``` **Environment** - Test machine: our Ubuntu 24.04 x86_64 test host (no GPU) - Fresh dcode (LangChain Deep Agents Code) `--observability` sandbox; the guard code on this path is identical to `main` - Fix built against `main` **Observed before fix** ``` dcode: refusing to start — runtime environment variable contains a secret-shaped value in OTEL_EXPORTER_OTLP_ENDPOINT. Remove it from the environment, or use 'nemoclaw credentials' to register provider keys. ``` (With the Bash refusal removed, the managed Python runtime raised the same class of error: `runtime environment variable OTEL_EXPORTER_OTLP_ENDPOINT contains a credential`, confirming two layers carried the same over-broad rule.) **Observed after fix** ``` Running task non-interactively... App: v0.1.34 | Agent: agent (default) | Model: ... Starting LangGraph server... ✓ Server ready ``` `dcode` now starts and proceeds normally; a credential-bearing endpoint (e.g. `http://token@host:4318`) is still refused. **Note on the secondary symptom (exit 0):** the report also states the process exits `0` on refusal. On our test host the refusal exits `2` (the wrapper `exit 2`s and `nemoclaw exec` propagates it; verified under both a pipe and a PTY), so I could not reproduce the exit-`0` behavior and this PR makes no exit-code change. Happy to look again with a `strace -f -e trace=exit_group` of the failing command if it still reproduces. ## Analysis `has_credential_name_context()` in `agents/langchain-deepagents-code/dcode-wrapper.sh` listed `OTEL_EXPORTER_OTLP_ENDPOINT` and `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` alongside the `_HEADERS` variants, and `assert_no_secret_runtime_env` / `assert_no_secret_env_file` refuse any value `>= 10` chars for a credential-named var. An OTLP endpoint value is always a URL (`>= 10` chars), so every real endpoint was rejected. The managed Python runtime `agents/langchain-deepagents-code/managed-dcode-runtime.py` carried the identical rule via `_CREDENTIAL_ENV_NAMES` in `_assert_safe_environment()`; the Bash layer refused first, so the reporter only saw the Bash message. The `_HEADERS` variants legitimately carry auth material and must stay refused; only the `_ENDPOINT` variants are plain URLs. ## Fix Both layers now treat the OTLP `_ENDPOINT` variables as URLs rather than credential names: - Removed the two `_ENDPOINT` names from the blanket name-context / `_CREDENTIAL_ENV_NAMES` refusal (kept `_HEADERS`). - Added a helper (`is_bare_http_url_without_userinfo` / `_is_bare_http_url_without_userinfo`) that accepts only a single clean http(s) URL and refuses a value with embedded userinfo (`scheme://user:pass@host`) or a structured key-bearing blob (JSON/quoted/whitespace/comma). This mirrors the existing OpenClaw OTEL config contract (`... must not include credentials`). - The value-shape scan (`is_secret_shaped_value` / `_contains_secret_shape`) still runs first, so token-shaped material embedded anywhere in the URL is still caught. Tests lock in that a plain collector URL is accepted (runtime + dotenv + direct-module) while userinfo URLs and structured key blobs are still refused; the previous test that asserted a plain URL was rejected is updated to a credential-bearing value. ## Changes - `agents/langchain-deepagents-code/dcode-wrapper.sh`: OTLP endpoint URL carve-out in the runtime-env and .env-file scans. - `agents/langchain-deepagents-code/managed-dcode-runtime.py`: same carve-out in `_assert_safe_environment`. - `test/langchain-deepagents-code-image-credentials.test.ts`: new accept/refuse coverage for OTLP endpoints. - `test/langchain-deepagents-code-direct-module-patch.test.ts`: accept plain endpoint URLs; keep credential-bearing refusal. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [x] `npx prek run` passes on the changed files - [x] `npm test` passes (touched files at minimum) - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the style guide (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ## AI Disclosure - [x] AI-assisted — tool: Claude Code Signed-off-by: Yanyun Liao <yanyunl@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved OpenTelemetry OTLP handling for `OTEL_EXPORTER_OTLP_ENDPOINT` / `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT`: managed `http://`/`https://` collector URLs are now accepted. * Rejected endpoint values with credential-like or unsafe URL forms (e.g., embedded user info, malformed host/port/path). * Empty endpoint values are treated as unset, consistently across runtime-injected variables and `.env` settings; unsafe control characters in `.env` are now fail-closed. * **Tests** * Expanded regression coverage for accepted managed/plain endpoints and rejected credential-bearing and malformed variants in both runtime and dotenv modes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Yanyun Liao <yanyunl@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
<!-- markdownlint-disable MD041 --> ## Summary Adds the pre-tag v0.0.79 release notes entry to `docs/about/release-notes.mdx` so the release plan can be generated after docs merge. The entry summarizes the merged v0.0.79 release train across inference, diagnostics, runtime hardening, policies, onboarding recovery, and release validation. ## Changes - Added the v0.0.79 release notes section with linked follow-up documentation for OpenRouter onboarding, managed vLLM changes, completion and logging, Deep Agents runtime limits, policy updates, onboarding recovery, and release validation. - Source summary: - NVIDIA#6461 -> `docs/about/release-notes.mdx`: Documents OpenRouter onboarding support and links to inference/provider references. - NVIDIA#6271 and NVIDIA#6272 -> `docs/about/release-notes.mdx`: Documents shell completion and structured logging highlights. - NVIDIA#6465, NVIDIA#6539, NVIDIA#6570, and NVIDIA#6528 -> `docs/about/release-notes.mdx`: Documents status route-drift, orphaned sandbox, gateway cleanup, and DGX Spark express-install diagnostics. - NVIDIA#6523, NVIDIA#6551, NVIDIA#6484, NVIDIA#6488, NVIDIA#6324, and NVIDIA#6542 -> `docs/about/release-notes.mdx`: Documents managed vLLM, Qwen3.6 tool parser, compaction, and timeout/readiness improvements. - NVIDIA#6559, NVIDIA#6538, NVIDIA#6560, NVIDIA#6568, NVIDIA#6552, NVIDIA#6567, and NVIDIA#6587 -> `docs/about/release-notes.mdx`: Documents runtime, credential, proxy, PID namespace, TOML, and provider-state hardening. - NVIDIA#6541, NVIDIA#5415, NVIDIA#6246, NVIDIA#6496, and NVIDIA#6573 -> `docs/about/release-notes.mdx`: Documents GitHub policy, Gmail policy, MCP allowlist, WhatsApp, and messaging-variant updates. - NVIDIA#6253, NVIDIA#6572, NVIDIA#6444, NVIDIA#6536, and NVIDIA#5860 -> `docs/about/release-notes.mdx`: Documents onboarding resume and create-step recovery improvements. - NVIDIA#6508, NVIDIA#6527, NVIDIA#5506, NVIDIA#6588, NVIDIA#6446, NVIDIA#6447, NVIDIA#6582, NVIDIA#6296, NVIDIA#6367, NVIDIA#6397, and NVIDIA#6505 -> `docs/about/release-notes.mdx`: Documents docs, release-risk, and E2E validation updates. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: Release-note prose only. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: Tests not applicable, release-note prose only. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Docs validation note: `npm run docs:check-agent-variants && npm run docs:check-routes && git diff --check` passed. Full `npm run docs` is currently blocked before Fern validation because the pinned `fern-api@5.65.2` package is unavailable from npm (`ETARGET No matching version found`). --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.79 with a new summary of recent improvements, including onboarding and inference options, operator/CLI diagnostics, sandbox recovery hardening, runtime limits, network policy behavior, and release validation updates. * Added updated references and links for the latest release. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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.
Summary
The managed Deep Agents Code secret guard rejected any non-empty
OTEL_EXPORTER_OTLP_ENDPOINT/OTEL_EXPORTER_OTLP_TRACES_ENDPOINTvalue, so a plain collector URL set by the documented--observabilityflow madedcoderefuse to start with a false-positive secret error. This PR treats the OTLP_ENDPOINTvariables as URLs (accept a clean bare http(s) URL, still refuse credential-bearing forms) in both the Bash wrapper and the managed Python runtime.Closes #6466.
Reproduction
On a fresh dcode
--observabilitysandbox:Environment
--observabilitysandbox; the guard code on this path is identical tomainmainObserved before fix
(With the Bash refusal removed, the managed Python runtime raised the same class of error:
runtime environment variable OTEL_EXPORTER_OTLP_ENDPOINT contains a credential, confirming two layers carried the same over-broad rule.)Observed after fix
dcodenow starts and proceeds normally; a credential-bearing endpoint (e.g.http://token@host:4318) is still refused.Note on the secondary symptom (exit 0): the report also states the process exits
0on refusal. On our test host the refusal exits2(the wrapperexit 2s andnemoclaw execpropagates it; verified under both a pipe and a PTY), so I could not reproduce the exit-0behavior and this PR makes no exit-code change. Happy to look again with astrace -f -e trace=exit_groupof the failing command if it still reproduces.Analysis
has_credential_name_context()inagents/langchain-deepagents-code/dcode-wrapper.shlistedOTEL_EXPORTER_OTLP_ENDPOINTandOTEL_EXPORTER_OTLP_TRACES_ENDPOINTalongside the_HEADERSvariants, andassert_no_secret_runtime_env/assert_no_secret_env_filerefuse any value>= 10chars for a credential-named var. An OTLP endpoint value is always a URL (>= 10chars), so every real endpoint was rejected. The managed Python runtimeagents/langchain-deepagents-code/managed-dcode-runtime.pycarried the identical rule via_CREDENTIAL_ENV_NAMESin_assert_safe_environment(); the Bash layer refused first, so the reporter only saw the Bash message. The_HEADERSvariants legitimately carry auth material and must stay refused; only the_ENDPOINTvariants are plain URLs.Fix
Both layers now treat the OTLP
_ENDPOINTvariables as URLs rather than credential names:_ENDPOINTnames from the blanket name-context /_CREDENTIAL_ENV_NAMESrefusal (kept_HEADERS).is_bare_http_url_without_userinfo/_is_bare_http_url_without_userinfo) that accepts only a single clean http(s) URL and refuses a value with embedded userinfo (scheme://user:pass@host) or a structured key-bearing blob (JSON/quoted/whitespace/comma). This mirrors the existing OpenClaw OTEL config contract (... must not include credentials).is_secret_shaped_value/_contains_secret_shape) still runs first, so token-shaped material embedded anywhere in the URL is still caught.Tests lock in that a plain collector URL is accepted (runtime + dotenv + direct-module) while userinfo URLs and structured key blobs are still refused; the previous test that asserted a plain URL was rejected is updated to a credential-bearing value.
Changes
agents/langchain-deepagents-code/dcode-wrapper.sh: OTLP endpoint URL carve-out in the runtime-env and .env-file scans.agents/langchain-deepagents-code/managed-dcode-runtime.py: same carve-out in_assert_safe_environment.test/langchain-deepagents-code-image-credentials.test.ts: new accept/refuse coverage for OTLP endpoints.test/langchain-deepagents-code-direct-module-patch.test.ts: accept plain endpoint URLs; keep credential-bearing refusal.Type of Change
Verification
npx prek runpasses on the changed filesnpm testpasses (touched files at minimum)make docsbuilds without warnings (doc changes only)AI Disclosure
Signed-off-by: Yanyun Liao yanyunl@nvidia.com
Summary by CodeRabbit
OTEL_EXPORTER_OTLP_ENDPOINT/OTEL_EXPORTER_OTLP_TRACES_ENDPOINT: managedhttp:///https://collector URLs are now accepted..envsettings; unsafe control characters in.envare now fail-closed.