Skip to content

feat(cli): warn when --env values look like credentials - #7

Draft
letv1nnn wants to merge 4 commits into
mainfrom
warn-credential-env-vars
Draft

feat(cli): warn when --env values look like credentials#7
letv1nnn wants to merge 4 commits into
mainfrom
warn-credential-env-vars

Conversation

@letv1nnn

@letv1nnn letv1nnn commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Summary

openshell sandbox create now emits a non-blocking warning when an --env key looks like a credential, steering users toward providers so secrets aren't exposed to the sandboxed agent. The warning suggests a specific provider create command when the key matches a built-in provider profile, and otherwise links to the providers docs.

Related Issue

NVIDIA#2548

Changes

  • Add credential_env_matches in crates/openshell-cli/src/commands/common.rs: flags --env keys by name only — known provider env vars from builtin_profiles(), plus *_TOKEN, *_SECRET, *_PASSWORD, *_CREDENTIAL, *_ACCESS_KEY, *_SECRET_KEY, *_API_KEY suffixes (case-insensitive). Never inspects or prints values.
  • Add warn_credential_env_vars: prints a non-blocking stderr warning per flagged key. When built-in profiles claim the
    key it lists a specific openshell provider create command per match (a key can map to several, e.g. GITHUB_TOKEN
    copilot and github); otherwise it links to the providers docs.
  • Add the --no-credential-warnings flag to sandbox create and wire the warning in before the CreateSandbox RPC
    (create only, not exec).
  • Re-export warn_credential_env_vars through run to match the existing parse_env_pairs call pattern.
  • Unit tests in common.rs covering profile match, suffix-only match, case-insensitivity, non-credential skip,
    no-value-leak, and deterministic ordering.
  • Document the warning and flag in docs/sandboxes/manage-sandboxes.mdx.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Publisher/user docs updated (if applicable)
  • Architecture docs updated (if applicable)

Summary by CodeRabbit

  • New Features

    • Added non-blocking warnings when sandbox environment variable names appear to contain provider credentials.
    • Added suggestions for creating configured providers when matching credentials are detected.
    • Added --no-credential-warnings to suppress these notifications.
  • Documentation

    • Documented credential warnings, suppression, and provider recommendations.
    • Clarified that credential values are identified by variable names only and are never inspected or displayed.

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The CLI detects credential-like --env keys during sandbox create, displays provider guidance without exposing values, and supports warning suppression with --no-credential-warnings. Documentation describes the detection and provider workflow.

Changes

Credential environment warnings

Layer / File(s) Summary
Credential detection and warning output
crates/openshell-cli/src/commands/common.rs
Credential suffixes and built-in provider mappings identify environment keys. Warnings are sorted and exclude credential values. Tests cover matching, filtering, case-insensitivity, ordering, and value non-disclosure.
Sandbox create integration and documentation
crates/openshell-cli/src/main.rs, crates/openshell-cli/src/run.rs, docs/sandboxes/manage-sandboxes.mdx
sandbox create accepts --no-credential-warnings, passes the option to validation, re-exports the warning function, and documents the behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SandboxCreate
  participant warn_credential_env_vars
  participant ProviderProfiles
  SandboxCreate->>warn_credential_env_vars: parsed --env pairs
  warn_credential_env_vars->>ProviderProfiles: match credential-like keys
  ProviderProfiles-->>warn_credential_env_vars: provider suggestions
  warn_credential_env_vars-->>SandboxCreate: warnings or suppressed output
Loading

Suggested reviewers: varshaprasad96, drew

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title is concise and related, but it incorrectly says the CLI checks --env values; the implementation checks credential-like variable names without inspecting values. Change the title to state that the CLI warns when --env variable names look like credentials, without inspecting their values.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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 warn-credential-env-vars

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

@letv1nnn letv1nnn changed the title Warn credential env vars feat(cli): warn when --env values look like credentials Aug 7, 2026
@letv1nnn
letv1nnn marked this pull request as draft August 7, 2026 16:18
@letv1nnn
letv1nnn force-pushed the warn-credential-env-vars branch 2 times, most recently from 3d4db33 to 51926bc Compare August 7, 2026 16:32
@letv1nnn

letv1nnn commented Aug 7, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
@letv1nnn
letv1nnn force-pushed the warn-credential-env-vars branch from 51926bc to e594d04 Compare August 7, 2026 21:35
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
@letv1nnn
letv1nnn force-pushed the warn-credential-env-vars branch from e876779 to 9977c6d Compare August 10, 2026 23:29
@letv1nnn

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

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 `@crates/openshell-cli/src/main.rs`:
- Around line 1442-1444: Update the help text for the no_credential_warnings CLI
option to say it suppresses warnings when --env keys look like credentials,
replacing “values” with “keys” while leaving the flag behavior unchanged.
🪄 Autofix

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 795c59d2-dd89-4dbe-a469-a55454f43c85

📥 Commits

Reviewing files that changed from the base of the PR and between f48b05e and 9977c6d.

📒 Files selected for processing (4)
  • crates/openshell-cli/src/commands/common.rs
  • crates/openshell-cli/src/main.rs
  • crates/openshell-cli/src/run.rs
  • docs/sandboxes/manage-sandboxes.mdx

Comment on lines +1442 to +1444
/// Suppress warnings when --env values look like credentials.
#[arg(long = "no-credential-warnings")]
no_credential_warnings: bool,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Correct the flag description.

The detector checks --env keys. It does not inspect values. Change “values” to “keys” so --help matches the behavior.

Proposed fix
-        /// Suppress warnings when --env values look like credentials.
+        /// Suppress warnings when --env keys look like credentials.
📝 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.

Suggested change
/// Suppress warnings when --env values look like credentials.
#[arg(long = "no-credential-warnings")]
no_credential_warnings: bool,
/// Suppress warnings when --env keys look like credentials.
#[arg(long = "no-credential-warnings")]
no_credential_warnings: bool,
🤖 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 `@crates/openshell-cli/src/main.rs` around lines 1442 - 1444, Update the help
text for the no_credential_warnings CLI option to say it suppresses warnings
when --env keys look like credentials, replacing “values” with “keys” while
leaving the flag behavior unchanged.

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.

1 participant