Load secret values in account editor

[kentcdodds]

Summary

• Kept plaintext secret values behind the existing password reauth-protected reveal endpoint instead of returning them from GET /account/secrets.json.
• Added coverage that selected secret metadata still loads without decrypted plaintext and without calling secret resolution.

Testing

• npx vitest run packages/worker/src/app/handlers/account-secrets.node.test.ts
• npm run test:e2e:run -- --grep "connect secret shows editable name and scope and saves the edited name"
• npm run typecheck
• npm run lint (passes with existing warnings outside this change)

  

Summary by CodeRabbit

• Tests
  • Updated account secrets test suite to ensure selected-secret metadata responses exclude decrypted values, maintaining proper data handling for sensitive information requests.

[coderabbitai]

📝 Walkthrough

Walkthrough

Test mocks for resolveSecret were updated to return a structured object { found: false, value: null } instead of null. The selected-secret metadata GET endpoint test was adjusted to assert the response excludes the value field and resolveSecret is not invoked.

Changes

Account Secrets Test Updates

Layer / File(s)	Summary
Mock Structure packages/worker/src/app/handlers/account-secrets.node.test.ts (lines 35–41)	resolveSecret mock return value changed from null to { found: false, value: null } to reflect structured result expectations.
Test Assertions packages/worker/src/app/handlers/account-secrets.node.test.ts (lines 567–601)	Selected-secret GET response should exclude value field; test verifies resolveSecret is not called for this metadata-only request.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

• fix: prevent sensitive data leakage in handlers (security audit findings 4A + 4B) #316: Both PRs modify the account-secrets flow to prevent plaintext value exposure in GET selected-secret responses, deferring decryption to an explicit reveal operation.

Poem

🐰 A secret kept is safer still,
No plaintext leaks on GET request's thrill—
Mock and test align their dance,
Where metadata's shown, values prance
Only when explicitly asked by chance! 🔐

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title describes loading secret values in the account editor, which aligns with the PR's primary objective to resolve selected account secrets and display decrypted values in the editor.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 cursor/load-secret-values-0e06

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 0/1 reviews remaining, refill in 60 minutes.}

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

[github-actions]

🔎 Preview deployed: https://kody-pr-323.kentcdodds.workers.dev

Worker: kody-pr-323
D1: kody-pr-323-db
KV: kody-pr-323-oauth-kv

Mocks:

• ai: https://kody-pr-323-mock-ai.kentcdodds.workers.dev/__mocks (kody-pr-323-mock-ai)
• cloudflare: https://kody-pr-323-mock-cloudflare.kentcdodds.workers.dev/__mocks (kody-pr-323-mock-cloudflare)

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/worker/src/app/handlers/account-secrets.ts`:
- Around line 903-928: The helper resolveAccountSecretDetail currently resolves
and attaches plaintext by calling resolveSecret (using parsed,
getSecretContextForAccountSecret and returning an AccountSecretDetail with
value), which bypasses the reauth flow; change it to stop resolving or returning
plaintext here—either remove the resolveSecret call entirely and return the
selected secret without a value, or gate attaching resolved.value behind an
explicit reauth check/token parameter (e.g., require a freshReauthToken flag or
call path from createAccountSecretRevealHandler) so only the reauth-protected
endpoint populates value; keep all other metadata from selected but do not
expose plaintext in resolveAccountSecretDetail.

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

Review profile: CHILL

Plan: Pro Plus

Run ID: a2ff28f7-98a0-43d9-865a-0915ec748bfb

📥 Commits

Reviewing files that changed from the base of the PR and between 8c47382 and 5cc4e4e.

📒 Files selected for processing (4)

• e2e/connect-secret.spec.ts
• packages/worker/client/routes/account-secrets.tsx
• packages/worker/src/app/handlers/account-secrets.node.test.ts
• packages/worker/src/app/handlers/account-secrets.ts

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 5cc4e4e. Configure here.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/worker/src/app/handlers/account-secrets.node.test.ts (1)

38-38: 💤 Low value

Consider completing the resolveSecret default mock to match the full ResolvedSecret shape.

The mock currently omits scope, allowedHosts, allowedCapabilities, and allowedPackages. While no test currently reaches the default mock (the reveal tests use mockResolvedValueOnce; the GET metadata test asserts resolveSecret is not called), an incomplete fallback can produce confusing undefined accesses if a future test exercises an unexpected handler path.

♻️ Suggested completion

-	resolveSecret: vi.fn(async () => ({ found: false, value: null })),
+	resolveSecret: vi.fn(async () => ({
+		found: false,
+		value: null,
+		scope: null,
+		allowedHosts: [],
+		allowedCapabilities: [],
+		allowedPackages: [],
+	})),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/app/handlers/account-secrets.node.test.ts` at line 38,
The default mock for resolveSecret returns an incomplete ResolvedSecret shape
which can cause undefined property access later; update the vi.fn default in the
test so resolveSecret returns the full ResolvedSecret fields (include scope,
allowedHosts, allowedCapabilities, allowedPackages in addition to found and
value) with safe default values (e.g., null/empty array/empty string as
appropriate) so any fallback invocation of resolveSecret has the complete shape.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/worker/src/app/handlers/account-secrets.node.test.ts`:
- Line 38: The default mock for resolveSecret returns an incomplete
ResolvedSecret shape which can cause undefined property access later; update the
vi.fn default in the test so resolveSecret returns the full ResolvedSecret
fields (include scope, allowedHosts, allowedCapabilities, allowedPackages in
addition to found and value) with safe default values (e.g., null/empty
array/empty string as appropriate) so any fallback invocation of resolveSecret
has the complete shape.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c22c08e4-0b39-4971-9c81-029af0101cb1

📥 Commits

Reviewing files that changed from the base of the PR and between 5cc4e4e and 7e1f251.

📒 Files selected for processing (1)

• packages/worker/src/app/handlers/account-secrets.node.test.ts