fix(chat): show deployment passwords to admins - #6177
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview
Password handling is tightened end-to-end: shared Reviewed by Cursor Bugbot for commit e011ed5. Configure here. |
Greptile SummaryThe PR allows workspace administrators to reveal saved chat deployment passwords while auditing each disclosure and preventing response caching.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported submission-state issue is invalid because the unconditional finally block clears the flag before confirmation or dismissal.
|
| Filename | Overview |
|---|---|
| apps/sim/app/api/chat/manage/[id]/password/route.ts | Adds an admin-authorized, audited, non-cacheable endpoint that decrypts and returns a saved deployment password. |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/chat/chat.tsx | Integrates password reveal and replacement confirmation while correctly resetting submission state through the existing finally block. |
| apps/sim/components/ui/generated-password-input.tsx | Adds on-demand password retrieval, plaintext disposal on hide, loading feedback, and clipboard support. |
| apps/sim/lib/api/contracts/chats.ts | Adds the reveal route contract and shared password validation limits for save and login paths. |
| apps/sim/lib/workflows/orchestration/chat-deploy.ts | Validates non-HTTP deployment passwords and prevents password-protected chats from being saved without a password. |
Reviews (4): Last reviewed commit: "fix(chat): correct password-change confi..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
ce3a491 to
36022ce
Compare
|
@cursor review |
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 8f97b0a. Configure here.
8f97b0a to
aaccfcf
Compare
Follow-ups from a security review of the password reveal endpoint. The
permission model itself was correct — the reveal is gated on workspace
admin via the canonical resolver, so derived org-admin access is honored.
These address secret handling and validation around it.
- Cap set-path passwords at the same 1024 chars the chat login accepts.
Neither the input nor the schema bounded length, so a longer password
saved fine and then failed the login POST on length before auth ran,
locking every visitor out permanently.
- Discard the revealed password when the field is hidden. It previously
stayed in state and in the input's DOM value with Copy still armed, so
the field read as hidden while still handing out the plaintext.
- Evict the decrypted password from the mutation cache on unmount, and
correct the TSDoc claiming it was never retained — it sat in the
MutationCache for the default five minutes after the modal closed.
- Validate the password inside performChatDeploy, the writer both callers
must use. The copilot deploy_chat tool bypasses the route contract and
could still store a whitespace-only or over-long password, or create a
password-protected chat with no password at all.
- Stop echoing raw decryption errors from the reveal endpoint.
- Only persist a new password when the chat ends up password-protected;
PATCH { authType: 'email', password } used to re-arm the secret that the
auth-type branch had just cleared.
Also replaces the hand-rolled copy state with useCopyToClipboard, which
fixes an unawaited clipboard write that surfaced as an unhandled rejection
and a "Copied" confirmation shown even when the write failed.
Co-Authored-By: Claude <noreply@anthropic.com>
… error Addresses both open Bugbot findings. - shouldConfirmPasswordChange keyed on "a chat exists" rather than "a password exists", so switching a public chat to password protection for the first time asked the admin to confirm changing a password that was never set. It now takes the existing-password signal the component already computes. - A failed reveal left "Failed to load the current password" on screen while the admin typed or generated a replacement, because the mutation only drops its error on the next attempt. Editing or regenerating now resets it. Co-Authored-By: Claude <noreply@anthropic.com>
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e011ed5. Configure here.
2 participants
Summary
Type of Change
Testing
Checklist