Skip to content

test(dcode): cover secret rejection exit code - #8148

Closed
VinayBhagavath wants to merge 2 commits into
NVIDIA:mainfrom
VinayBhagavath:codex/fix-dcode-secret-exit-8137
Closed

test(dcode): cover secret rejection exit code#8148
VinayBhagavath wants to merge 2 commits into
NVIDIA:mainfrom
VinayBhagavath:codex/fix-dcode-secret-exit-8137

Conversation

@VinayBhagavath

@VinayBhagavath VinayBhagavath commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Add a Linux regression test for dcode secret rejection. The test covers a raw OPENAI_API_KEY in the managed .env file and verifies exit code 2, no launch, and no secret leakage. The current wrapper already implements this behavior; the test protects the contract reported by #8137.

Related Issue

Fixes #8137

Changes

  • Add the exact OPENAI_API_KEY=sk-live-... .env case to the dcode wrapper security-boundary test.
  • Assert exit code 2, no dcode launch, the variable name in the refusal, and absence of the raw value.

Type of Change

  • 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)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: Test-only regression coverage; no user-facing documentation changed.
  • 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: Pending maintainer review.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: No documentation paths changed; the review covered the test title and PR wording.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit:
  • Station profile/scenario:
  • Result:
  • Supporting evidence:

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable — npm run check:diff reached the repository checks; all applicable file checks and the secret scan passed, but the existing source-architecture budget failed because src/lib/core/shell-quote.ts fan-in is 26 against a 27 limit.
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — npx vitest run --project integration test/dcode-wrapper-identity.test.ts collected 26 Linux-only tests and skipped them on macOS. The related supervisor and empty-prompt tests passed 2 tests, with 18 Linux-only tests skipped.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run 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)

Signed-off-by: Vinay Bhagavath bhagavathvinay@gmail.com

Summary by CodeRabbit

  • Bug Fixes
    • Added coverage ensuring startup is blocked when a raw API key is present in the environment file.
    • Confirmed the application reports the affected variable, safely redacts the secret, and prevents launch.

@copy-pr-bot

copy-pr-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds a regression test for raw OPENAI_API_KEY values in .env. The test verifies exit code 2, prevents dcode launch, reports the variable name, and redacts the secret.

Changes

dcode secret validation

Layer / File(s) Summary
Secret rejection regression test
test/dcode-wrapper-identity.test.ts
Adds assertions for exit code 2, blocked dcode startup, variable-name reporting, and secret redaction.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: integration: dcode, area: sandbox

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The test covers #8137 by checking exit code 2, launch refusal, variable-name reporting, and secret redaction.
Out of Scope Changes check ✅ Passed The only change adds a focused regression test for the linked secret-rejection behavior.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the dcode regression test for secret rejection and its exit code.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/dcode-wrapper-identity.test.ts`:
- Line 198: Update the redaction assertion in the runBashWrapper refusal-path
test to verify that the secret does not appear in either captured output stream,
checking both run.stdout and run.stderr while preserving the existing
public-boundary behavior.
🪄 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: 08e7fc8b-b2a0-4c67-bed8-61a94fc5738a

📥 Commits

Reviewing files that changed from the base of the PR and between c63e7eb and 2270b83.

📒 Files selected for processing (1)
  • test/dcode-wrapper-identity.test.ts

Comment thread test/dcode-wrapper-identity.test.ts Outdated
@VinayBhagavath

Copy link
Copy Markdown
Contributor Author

Closing this test-only PR. The current source and v0.0.81 tag already return exit code 2 for this guard, so #8137 needs artifact-level reproduction before a production fix can be proposed.

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized terminology decisions differ; normalized E2E selections differ; severity counts match.
2 additional E2E selections from the second opinion

Advisory only. The primary lane did not select these E2E jobs or targets.

  • openshell-credential-generation-window: The completed second-opinion lane identified E2E coverage that the primary lane omitted.
  • gateway-guard-recovery: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate.

1 semantic terminology decision

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

  • established — raw OpenAI API key at test/dcode-wrapper-identity.test.ts:187: Retain the established term.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: None

2 optional E2E recommendations
  • ubuntu-repo-cloud-langchain-deepagents-code
  • security-posture

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[All Platforms][Security] dcode startup secret detection returns exit code 0 instead of 2 when refusing to start

1 participant