test(auth): regression-guard no unredacted secret in login/whoami/set-key/auth-set output (BE-3363)

[mattmillerai]

ELI-5

A while back someone reported that signing in to Comfy Cloud printed their secret API key on screen in plain text. Today that doesn't happen — every command that touches a secret already scrambles it to sk-S…cdef before printing. But nothing guarantees it stays that way: each command just happens to call the redacting version, and one careless refactor (or a new secret-bearing command) could quietly start leaking again.

This PR adds a test that stands guard: it runs each secret-printing command with a fake secret and fails the build if the full secret ever shows up in what the command prints. No product code changes — it just locks in the good behavior we already have.

What & why

Test-only regression guard (tests/comfy_cli/test_secret_redaction.py) for BE-3363. For every secret-bearing command, in both pretty and --json output modes, it feeds a long sentinel secret and asserts the full sentinel never appears in captured stdout or stderr (the redacted sk-S…cdef form is expected and fine):

• comfy auth set civitai --key <sentinel>
• comfy cloud set-key --key <sentinel>
• comfy cloud whoami (with a session seeded via store.save_cloud_session, tokens = sentinels)
• comfy cloud login success path (comfy_cli.cloud.command.run_login mocked to return sentinel tokens; runs in-process via Typer's CliRunner because a subprocess can't patch run_login)

Sentinels are >16 chars so they take the sk-S…cdef redaction branch, not the <=16 → *** one. The module is structured so a new secret-bearing command is a single _Case list entry (or a sibling of the login test when a network handshake must be mocked).

Two positive controls prove the guard observes redaction rather than vacuously passing on empty/errored output: test_auth_set_json_carries_only_the_redacted_key asserts the JSON envelope carries exactly store._redact(...), and the login test asserts a real session was persisted to disk.

Out of scope

No redaction behavior changes (redaction on current main already works — verified while writing this); the cloud web page's own server-side display of keys.

Testing

• pytest tests/comfy_cli/test_secret_redaction.py → 9 passed.
• ruff format --check + ruff check on the new file → clean.
• Manually confirmed --no-json forces OutputMode.PRETTY even in a non-tty subprocess (renderer.py:120-121), so the "pretty" cases genuinely exercise the banner-rendering path; eyeballed auth set/whoami pretty output (redacted key / expiry shown, never the raw token).

Notes for the reviewer

• Falsification (negative-claim check): this diff denies no capability — it is a pure additive test asserting existing behavior, so the falsification clause doesn't apply.
• Pre-existing, unrelated failures live in tests/comfy_cli/auth/test_where.py (2) and tests/comfy_cli/cloud/test_client.py (5); confirmed present on a clean origin/main checkout, so they are not introduced here.
• whoami/login use a subprocess vs. in-process CliRunner split respectively — the login handshake must be mocked, which a subprocess boundary can't reach.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 43 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3641c08c-aad2-44e2-8674-7f8414276af0

📥 Commits

Reviewing files that changed from the base of the PR and between 62c2401 and ad5ffe3.

📒 Files selected for processing (1)

• tests/comfy_cli/test_secret_redaction.py

📝 Walkthrough

Walkthrough

Adds regression coverage for unredacted secrets across auth, cloud, and login CLI paths in pretty and JSON modes, while verifying successful login persistence. A tiny secret-checking imp keeps watch.

Changes

Secret redaction tests

Layer / File(s)	Summary
Redaction test harness and command cases tests/comfy_cli/test_secret_redaction.py	Defines sentinel credentials, parametrized command cases, shared CLI environment setup, and subprocess execution helpers.
Command output redaction checks tests/comfy_cli/test_secret_redaction.py	Checks auth set, cloud set-key, and cloud whoami output across JSON and non-JSON modes, including JSON payload redaction.
Cloud login redaction and persistence tests/comfy_cli/test_secret_redaction.py	Mocks OAuth results, verifies access and refresh tokens are absent from login output, and confirms the access token is persisted in the secrets file.

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
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 unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch matt/be-3363-secret-redaction-guard

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch matt/be-3363-secret-redaction-guard

---

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

[coderabbitai]

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 `@tests/comfy_cli/test_secret_redaction.py`:
- Around line 145-149: Run Ruff formatting on the assertions in the test cases
around the combined-output secret checks and return-code failure messages,
including the additional block around lines 211–213. Preserve the existing
assertions and behavior while applying Ruff’s standard line wrapping and
formatting.

🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2752a09a-c50e-492f-8923-3a16bc982611

📥 Commits

Reviewing files that changed from the base of the PR and between a732f5c and 62c2401.

📒 Files selected for processing (1)

• tests/comfy_cli/test_secret_redaction.py

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

Found 1 finding(s).

Severity	Count
🟡 Medium	1

Panel: 6/8 reviewers contributed findings.

Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)