Skip to content

F-019: docs(cli): document --allow-secrets risk in help text#22

Open
Sephyi wants to merge 1 commit intodevelopmentfrom
audit/f-019-allow-secrets-risk-docs
Open

F-019: docs(cli): document --allow-secrets risk in help text#22
Sephyi wants to merge 1 commit intodevelopmentfrom
audit/f-019-allow-secrets-risk-docs

Conversation

@Sephyi
Copy link
Copy Markdown
Owner

@Sephyi Sephyi commented Apr 22, 2026

Summary

docs(cli): document --allow-secrets risk in help text.

Audit context

Closes audit entry F-019 from #3.

Verification

  • cargo fmt --check
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo test --all-targets

Note: one pre-existing test porcelain_exits_within_timeout_with_no_staged_changes is a known macOS cold-start flake that reproduces on unmodified development — unrelated to this change.

Expand the one-line doc comment on the `--allow-secrets` flag to spell
out what bypassing the secret scanner actually does: the staged diff
(including any API keys, credentials, or tokens it may contain) is sent
to the configured LLM provider as part of the prompt. A prominent
warning glyph makes the risk visible in `--help` output, and the text
frames the flag as a deliberate opt-in for diffs the user has audited
(e.g., test fixtures with obviously fake tokens) rather than a generic
override switch.

Closes audit entry F-019 from #3.
Copilot AI review requested due to automatic review settings April 22, 2026 19:51
@Sephyi Sephyi added the audit Codebase audit cleanup (issue #3) label Apr 22, 2026
@Sephyi Sephyi self-assigned this Apr 22, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates CLI help text to better communicate the security implications of --allow-secrets, addressing audit finding F-019 by warning users about potential secret exfiltration to the configured LLM provider.

Changes:

  • Expanded --allow-secrets flag documentation in src/cli.rs to warn that secrets in the staged diff may be sent to the LLM.
  • Added a stronger “danger” warning block in the clap help text.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/cli.rs
/// Allow committing with detected secrets (local only)
/// Allow committing even when potential secrets are detected in staged changes.
///
/// ⚠ DANGER: This disables the secret-scan block, meaning any API keys,
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

This introduces a symbol in CLI help output. The project docs note an explicit preference to replace with a text label like WARNING: (see CHANGELOG.md / PRD.md) for better compatibility/tokenization. Consider switching to a plain-text prefix (e.g., WARNING: / DANGER:) to stay consistent and avoid terminals that render the glyph poorly.

Suggested change
/// DANGER: This disables the secret-scan block, meaning any API keys,
/// DANGER: This disables the secret-scan block, meaning any API keys,

Copilot uses AI. Check for mistakes.
Comment thread src/cli.rs
Comment on lines +28 to +34
/// Allow committing even when potential secrets are detected in staged changes.
///
/// ⚠ DANGER: This disables the secret-scan block, meaning any API keys,
/// credentials, or tokens present in the staged diff will be sent to the
/// configured LLM provider as part of the prompt. Only use this flag when
/// you have manually audited the diff and are certain no real credentials
/// are present (e.g., test fixtures with obviously fake tokens).
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

The help text implies --allow-secrets simply disables the secret-scan block. In practice (see src/app.rs), secrets are still scanned and the flag only allows proceeding after an interactive confirmation; in non-interactive modes (e.g., --yes/--porcelain or piped stdin) it still fails closed with SecretsDetected. Consider updating this help text to reflect the actual behavior so users aren't surprised when --allow-secrets doesn't bypass the block in non-interactive runs.

Suggested change
/// Allow committing even when potential secrets are detected in staged changes.
///
/// ⚠ DANGER: This disables the secret-scan block, meaning any API keys,
/// credentials, or tokens present in the staged diff will be sent to the
/// configured LLM provider as part of the prompt. Only use this flag when
/// you have manually audited the diff and are certain no real credentials
/// are present (e.g., test fixtures with obviously fake tokens).
/// Allow proceeding after potential secrets are detected in staged changes.
///
/// ⚠ DANGER: Secrets are still scanned for. This flag does not bypass
/// detection; it only allows an interactive confirmation to continue after
/// secrets are found. In non-interactive modes (for example --yes,
/// --porcelain, or piped stdin), commitbee still fails closed instead of
/// proceeding. If you do confirm, any API keys, credentials, or tokens
/// present in the staged diff will be sent to the configured LLM provider
/// as part of the prompt. Only use this flag when you have manually
/// audited the diff and are certain no real credentials are present
/// (e.g., test fixtures with obviously fake tokens).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

audit Codebase audit cleanup (issue #3)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants