fix(dcode): allow managed OTLP collector endpoint#6575
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/langchain-deepagents-code-managed-entrypoints.test.ts`:
- Around line 155-159: The table in the OTLP collector endpoint test is missing
the remaining allowed variants, so expand the `it.each` cases in
`test/langchain-deepagents-code-managed-entrypoints.test.ts` to cover
`OTEL_EXPORTER_OTLP_ENDPOINT` with `/v1/traces` and
`OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` with a trailing slash. Keep the assertion
focused on the existing allowlist behavior in the test block around the `allows
the local managed OTLP collector endpoint` case.
🪄 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: 6f45f99f-4028-4879-880c-6bc0c9f0728b
📒 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-managed-entrypoints.test.ts
| it.each([ | ||
| ["OTEL_EXPORTER_OTLP_ENDPOINT", "http://host.openshell.internal:4318"], | ||
| ["OTEL_EXPORTER_OTLP_ENDPOINT", "http://host.openshell.internal:4318/"], | ||
| ["OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "http://host.openshell.internal:4318/v1/traces"], | ||
| ])("allows the local managed OTLP collector endpoint in %s", (name, value) => { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Cover the remaining allowed endpoint variants.
The implementation also accepts OTEL_EXPORTER_OTLP_ENDPOINT with /v1/traces and normalizes a trailing slash for OTEL_EXPORTER_OTLP_TRACES_ENDPOINT; add those table rows so the test proves the full public allowlist behavior. As per path instructions, “Review tests for behavioral confidence rather than implementation lock-in.”
Proposed test case additions
it.each([
["OTEL_EXPORTER_OTLP_ENDPOINT", "http://host.openshell.internal:4318"],
["OTEL_EXPORTER_OTLP_ENDPOINT", "http://host.openshell.internal:4318/"],
+ ["OTEL_EXPORTER_OTLP_ENDPOINT", "http://host.openshell.internal:4318/v1/traces"],
+ ["OTEL_EXPORTER_OTLP_ENDPOINT", "http://host.openshell.internal:4318/v1/traces/"],
["OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "http://host.openshell.internal:4318/v1/traces"],
+ ["OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "http://host.openshell.internal:4318/v1/traces/"],
])("allows the local managed OTLP collector endpoint in %s", (name, value) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it.each([ | |
| ["OTEL_EXPORTER_OTLP_ENDPOINT", "http://host.openshell.internal:4318"], | |
| ["OTEL_EXPORTER_OTLP_ENDPOINT", "http://host.openshell.internal:4318/"], | |
| ["OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "http://host.openshell.internal:4318/v1/traces"], | |
| ])("allows the local managed OTLP collector endpoint in %s", (name, value) => { | |
| it.each([ | |
| ["OTEL_EXPORTER_OTLP_ENDPOINT", "http://host.openshell.internal:4318"], | |
| ["OTEL_EXPORTER_OTLP_ENDPOINT", "http://host.openshell.internal:4318/"], | |
| ["OTEL_EXPORTER_OTLP_ENDPOINT", "http://host.openshell.internal:4318/v1/traces"], | |
| ["OTEL_EXPORTER_OTLP_ENDPOINT", "http://host.openshell.internal:4318/v1/traces/"], | |
| ["OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "http://host.openshell.internal:4318/v1/traces"], | |
| ["OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "http://host.openshell.internal:4318/v1/traces/"], | |
| ])("allows the local managed OTLP collector endpoint in %s", (name, value) => { |
🤖 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-managed-entrypoints.test.ts` around lines 155
- 159, The table in the OTLP collector endpoint test is missing the remaining
allowed variants, so expand the `it.each` cases in
`test/langchain-deepagents-code-managed-entrypoints.test.ts` to cover
`OTEL_EXPORTER_OTLP_ENDPOINT` with `/v1/traces` and
`OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` with a trailing slash. Keep the assertion
focused on the existing allowlist behavior in the test block around the `allows
the local managed OTLP collector endpoint` case.
Source: Path instructions
|
Closing as superseded by #6538, which already merged the #6466 fix. Against current main, this PR only adds redundant narrower allowlists and tests; keeping a second security-sensitive allowlist would create drift without any remaining behavior to land. Thank you for the thorough verification and sign-off. |
3 participants
Summary
Allows the NemoClaw-managed Deep Agents Code runtime to accept the fixed, credential-free local OTLP collector endpoint without tripping the secret scanner. The change keeps arbitrary OTEL endpoints and OTEL headers blocked, and managed runtime setup still strips ambient OTEL configuration before child/server execution.
Related Issue
Fixes #6466
Changes
OTEL_EXPORTER_OTLP_ENDPOINT=http://host.openshell.internal:4318and the local/v1/tracesendpoint in the Bash DCode wrapper.Type of Change
Quality Gates
.envremains fail-closed, external OTEL endpoints and headers remain rejected, and targeted security/observability tests pass.Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailable — command/result: DGX Spark Linux exact commitd6a7c52064b2f0e4e1bcfef94b1f6b719dfa5cfe:npm run check:diffpassed.d6a7c52064b2f0e4e1bcfef94b1f6b719dfa5cfe:npx vitest run --project integration test/langchain-deepagents-code-managed-entrypoints.test.ts test/langchain-deepagents-code-direct-module-patch.test.tspassed, 87 tests;npx vitest run --project integration test/langchain-deepagents-code-secret-pattern-parity.test.ts test/langchain-deepagents-code-observability.test.tspassed, 10 tests. Actual OpenShell sandbox smoke against local vLLMQwen/Qwen3.6-35B-A3Bpassed:dcode statusacceptedOTEL_EXPORTER_OTLP_ENDPOINT=http://host.openshell.internal:4318, rejectedOTEL_EXPORTER_OTLP_ENDPOINT=https://collector.example.test:4318, anddcode -n 'Reply with exactly OK.'returnedOK.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: HwangJohn angelic805@gmail.com
Summary by CodeRabbit