Skip to content

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

Open
letv1nnn wants to merge 3 commits into
NVIDIA:mainfrom
letv1nnn:warn-credential-env-vars
Open

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

Conversation

@letv1nnn

@letv1nnn letv1nnn commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

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

#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.

Example output

image

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)

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
@copy-pr-bot

copy-pr-bot Bot commented Aug 7, 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.

@letv1nnn
letv1nnn marked this pull request as draft August 7, 2026 16:09
@letv1nnn
letv1nnn force-pushed the warn-credential-env-vars branch from ca90468 to 3d4db33 Compare August 7, 2026 16:24
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
@letv1nnn
letv1nnn force-pushed the warn-credential-env-vars branch from 3d4db33 to 51926bc Compare August 7, 2026 16:32
@letv1nnn
letv1nnn marked this pull request as ready for review August 7, 2026 16:35
}

fn credential_env_matches(env: &HashMap<String, String>) -> Vec<(String, Vec<ProfileSuggestion>)> {
const SUFFIXES: [&str; 7_usize] = [

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

instead of suffixes could check for presence anywhere within the key.

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.

2 participants