Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Warning Rate limit exceeded@N2D4 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 16 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds a Data Vault feature: DB model + migration, backend set/get routes using KMS envelope encryption, client/server crypto helpers, interface and server-app integration, dashboard UI for stores, E2E tests, LocalStack dev infra, config/defaults, KnownErrors, and related tooling changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Dashboard as Dashboard UI
participant ServerApp as StackServerApp
participant Interface as ServerInterface
participant API as Backend API (/set)
participant DB as DB (DataVaultEntry)
participant KMS as AWS KMS (LocalStack)
participant CryptoClient as Crypto (client)
participant CryptoServer as Crypto (server)
User->>Dashboard: setValue(secret, storeId, key, value)
Dashboard->>ServerApp: setValue(...)
ServerApp->>Interface: setDataVaultStoreValue(secret, storeId, key, value)
Interface->>CryptoClient: hashKey(secret, key)
Interface->>CryptoClient: encryptValue(secret, key, value)
Interface->>API: POST /data-vault/stores/{id}/set { hashed_key, encrypted_value }
API->>KMS: Ensure KEK, GenerateDataKey
API->>CryptoServer: encryptWithKms -> { edkBase64, ciphertextBase64 }
API->>DB: upsert DataVaultEntry (tenancyId, storeId, hashedKey, encrypted)
API-->>Interface: 200
Interface-->>ServerApp: done
sequenceDiagram
autonumber
actor User
participant Dashboard as Dashboard UI
participant ServerApp as StackServerApp
participant Interface as ServerInterface
participant API as Backend API (/get)
participant DB as DB (DataVaultEntry)
participant KMS as AWS KMS (LocalStack)
participant CryptoClient as Crypto (client)
participant CryptoServer as Crypto (server)
User->>Dashboard: getValue(secret, storeId, key)
Dashboard->>ServerApp: getValue(...)
ServerApp->>Interface: getDataVaultStoreValue(secret, storeId, key)
Interface->>CryptoClient: hashKey(secret, key)
Interface->>API: POST /data-vault/stores/{id}/get { hashed_key }
API->>DB: find DataVaultEntry by (tenancyId, storeId, hashedKey)
alt entry found
API->>KMS: Decrypt(edk) -> DEK
API->>CryptoServer: decryptWithKms -> ciphertextBase64 -> decrypted_value
API-->>Interface: 200 { encrypted_value: decrypted_value }
Interface->>CryptoClient: decryptValue(secret, key, encrypted_value)
Interface-->>ServerApp: plaintext
ServerApp-->>Dashboard: value
else not found
API-->>Interface: 400 KnownErrors.DataVaultStoreHashedKeyDoesNotExist
Interface-->>ServerApp: null
ServerApp-->>Dashboard: null
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements "Data Vaults" - a secure key-value storage system with client-side encryption and server-side envelope encryption. The system allows storing encrypted data in isolated stores where Stack Auth can't access the actual values without the client-provided secret.
Key changes:
- Added cryptographic utilities for encryption, decryption, and hashing
- Implemented data vault stores with client-side and server-side encryption layers
- Created API endpoints for storing and retrieving encrypted values
- Added dashboard UI for managing data vault stores
Reviewed Changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/stack-shared/src/utils/crypto.tsx | Added encryption/decryption and hashing utilities using Web Crypto API |
| packages/stack-shared/src/utils/bytes.tsx | Enhanced base64 encoding performance and added comprehensive tests |
| packages/stack-shared/src/helpers/vault/client-side.ts | Client-side encryption helpers for key hashing and value encryption |
| packages/stack-shared/src/helpers/vault/server-side.ts | Server-side KMS integration for envelope encryption |
| packages/template/src/lib/stack-app/data-vault/index.ts | Data vault store type definitions |
| packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts | Server app implementation with data vault store methods |
| apps/backend/src/app/api/latest/data-vault/stores/[id]/set/route.tsx | API endpoint for storing encrypted values |
| apps/backend/src/app/api/latest/data-vault/stores/[id]/get/route.tsx | API endpoint for retrieving encrypted values |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/page-client.tsx | Dashboard UI for managing data vault stores |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts:1
- The test on line 71 attempts to use a different secret but expects the value to not equal the original. However, this test behavior is incorrect - using a wrong secret should either throw an error or return null, not return a different decrypted value. This suggests the encryption/decryption logic may not be properly validating secrets.
import { KnownErrors, StackServerInterface } from "@stackframe/stack-shared";
apps/e2e/tests/js/data-vault.test.ts:1
- This test is using 'different-secret' as the key parameter instead of as the secret parameter. The correct call should be
await decryptValue('different-secret', 'key', encrypted)to test decryption with a wrong secret.
import { StackServerApp } from '@stackframe/js';
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR introduces a comprehensive Data Vault feature to Stack Auth, providing secure encrypted key-value storage with client-side encryption and zero-knowledge architecture. The implementation spans multiple layers of the application:
Core Infrastructure: A new DataVaultEntry database table stores encrypted data with proper multi-tenancy isolation using composite keys (tenancyId, storeId, hashedKey). The table uses JSONB for flexible encrypted payload storage and includes optimized indexing for query performance.
Encryption Architecture: The system implements envelope encryption using AWS KMS for server-side key management combined with client-side PBKDF2 key derivation (100,000 iterations) and AES-GCM encryption. Keys are hashed before server storage to prevent enumeration attacks, and the server cannot decrypt stored values without client-provided secrets.
API Layer: New REST endpoints (/data-vault/stores/{id}/get and /data-vault/stores/{id}/set) handle encrypted storage operations with proper authentication and validation. The server interface includes getDataVaultStoreValue and setDataVaultStoreValue methods that maintain the client-side encryption pattern.
Template Integration: Data vault stores are exposed through the familiar AsyncStoreProperty pattern, providing both async methods (getValue, setValue) and React hooks (useValue) for different usage scenarios. This follows the same architectural patterns as other Stack resources like users and teams.
Dashboard UI: New pages allow users to create, manage, and delete vault stores with inline editing capabilities, confirmation dialogs, and usage documentation. The UI provides store isolation visualization and clipboard functionality for easy integration.
Development Support: LocalStack integration provides AWS service emulation for local development, with proper Docker Compose configuration and environment variable setup for KMS operations.
Configuration: The branch config schema now supports data vault stores with display names, following the established pattern for user-defined resources. Error handling includes specific error types for missing stores and keys.
Confidence score: 3/5
- This PR introduces complex cryptographic functionality with several security-sensitive implementation details that require careful review
- Score reflects concerns about API inconsistencies, test coverage gaps, and potential security issues in the encryption implementation
- Pay close attention to cryptographic implementation in client-side.ts, API route handlers, and test files with authentication contradictions
26 files reviewed, 12 comments
![recurseml[bot]](https://avatars.githubusercontent.com/in/1054195?s=60&v=4){kind=small}
| const hashedKey = hashKey(key); | ||
| const encryptedValue = "encrypted-secret-value-123"; | ||
|
|
||
| // Store a value |
There was a problem hiding this comment.
The URL path is concatenated as a string, which violates the code pattern rule that states 'Don't concatenate URLs as strings - avoid patterns like /users/${userId}'. This pattern occurs throughout the test file and could lead to injection vulnerabilities if the storeId or other variables contain special characters.
🔍 This comment matches your code_patterns.mdc rule.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| }); | ||
|
|
||
| // Retrieve the value | ||
| const getResponse = await niceBackendFetch(`/api/latest/data-vault/stores/${storeId}/get`, { |
There was a problem hiding this comment.
URL concatenation using template strings instead of proper URL construction methods. This violates the code pattern rule that specifically prohibits patterns like /users/${userId}.
🔍 This comment matches your code_patterns.mdc rule.
| const getResponse = await niceBackendFetch(`/api/latest/data-vault/stores/${storeId}/get`, { | |
| const getResponse = await niceBackendFetch(new URL(`data-vault/stores/${storeId}/get`, "/api/latest/").toString(), { |
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| const updatedValue = "updated-value"; | ||
|
|
||
| // Store initial value | ||
| const firstSetResponse = await niceBackendFetch(`/api/latest/data-vault/stores/${storeId}/set`, { |
There was a problem hiding this comment.
URL is constructed by string concatenation using template literals. The code_patterns.mdc rule specifically prohibits this pattern and recommends using proper URL construction utilities instead of patterns like /users/${userId}.
🔍 This comment matches your code_patterns.mdc rule.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| const firstId = firstSetResponse.body.id; | ||
|
|
||
| // Update the value | ||
| const updateResponse = await niceBackendFetch(`/api/latest/data-vault/stores/${storeId}/set`, { |
There was a problem hiding this comment.
URL path is concatenated as a string with variables using template literals. This violates the code pattern rule against concatenating URLs as strings with patterns like /users/${userId}. Proper URL construction utilities should be used instead.
🔍 This comment matches your code_patterns.mdc rule.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| expect(updateResponse.body.id).toBe(firstId); | ||
|
|
||
| // Verify the updated value | ||
| const getResponse = await niceBackendFetch(`/api/latest/data-vault/stores/${storeId}/get`, { |
There was a problem hiding this comment.
URL string concatenation using template literals instead of proper URL construction methods. The code_patterns.mdc rule specifically prohibits concatenating URLs as strings with patterns like /users/${userId}.
🔍 This comment matches your code_patterns.mdc rule.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| await Auth.Otp.signIn(); | ||
|
|
||
| // Test empty store ID (by using spaces which get trimmed) | ||
| const emptyStoreResponse = await niceBackendFetch(`/api/latest/data-vault/stores/${encodeURIComponent(" ")}/set`, { |
There was a problem hiding this comment.
While this line does use encodeURIComponent() which is better than raw concatenation, it still violates the code pattern rule against concatenating URLs as strings. The proper approach would be to use a URL construction utility rather than template literals.
🔍 This comment matches your code_patterns.mdc rule.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| `); | ||
|
|
||
| // Test empty hashed key | ||
| const emptyHashResponse = await niceBackendFetch(`/api/latest/data-vault/stores/valid-store/set`, { |
There was a problem hiding this comment.
URL is hardcoded as a string. While this particular instance doesn't include variable interpolation, it still violates the code pattern rule that recommends using proper URL construction utilities rather than string literals for URLs.
🔍 This comment matches your code_patterns.mdc rule.
| const emptyHashResponse = await niceBackendFetch(`/api/latest/data-vault/stores/valid-store/set`, { | |
| const emptyHashResponse = await niceBackendFetch(new URL("/api/latest/data-vault/stores/valid-store/set", window.location.origin).toString(), { |
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| `); | ||
|
|
||
| // Test empty encrypted value | ||
| const emptyValueResponse = await niceBackendFetch(`/api/latest/data-vault/stores/test-store-1/set`, { |
There was a problem hiding this comment.
URL is hardcoded as a string literal. The code_patterns.mdc rule recommends using proper URL construction utilities instead of string literals or concatenation for URLs.
🔍 This comment matches your code_patterns.mdc rule.
| const emptyValueResponse = await niceBackendFetch(`/api/latest/data-vault/stores/test-store-1/set`, { | |
| const emptyValueResponse = await niceBackendFetch(new URL("/api/latest/data-vault/stores/test-store-1/set", process.env.BACKEND_URL).toString(), { |
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| `); | ||
|
|
||
| // Test empty hashed key for get | ||
| const getEmptyHashResponse = await niceBackendFetch(`/api/latest/data-vault/stores/valid-store/get`, { |
There was a problem hiding this comment.
URL is hardcoded as a string literal. The code_patterns.mdc rule specifies not to concatenate URLs as strings and to use proper URL construction utilities instead.
🔍 This comment matches your code_patterns.mdc rule.
| const getEmptyHashResponse = await niceBackendFetch(`/api/latest/data-vault/stores/valid-store/get`, { | |
| const getEmptyHashResponse = await niceBackendFetch(new URL("/api/latest/data-vault/stores/valid-store/get", window.location.origin).toString(), { |
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| const value2 = "value-for-store-2"; | ||
|
|
||
| // Set value in store 1 | ||
| await niceBackendFetch(`/api/latest/data-vault/stores/${store1Id}/set`, { |
There was a problem hiding this comment.
URL path is concatenated as a string using template literals. This violates the code pattern rule that specifically prohibits patterns like /users/${userId} and recommends using proper URL construction utilities instead.
🔍 This comment matches your code_patterns.mdc rule.
| await niceBackendFetch(`/api/latest/data-vault/stores/${store1Id}/set`, { | |
| await niceBackendFetch(new URL(`/api/latest/data-vault/stores/${store1Id}/set`, window.location.origin).toString(), { |
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| }); | ||
|
|
||
| // Set value in store 2 with same key | ||
| await niceBackendFetch(`/api/latest/data-vault/stores/${store2Id}/set`, { |
There was a problem hiding this comment.
URL is constructed by concatenating strings and variables using template literals. The code_patterns.mdc rule specifically prohibits this pattern in favor of proper URL construction utilities.
🔍 This comment matches your code_patterns.mdc rule.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| }); | ||
|
|
||
| // Get from store 1 | ||
| const getStore1Response = await niceBackendFetch(`/api/latest/data-vault/stores/${store1Id}/get`, { |
There was a problem hiding this comment.
URL string concatenation using template literals with variables. This violates the code_patterns.mdc rule that prohibits concatenating URLs as strings with patterns like /users/${userId}.
🔍 This comment matches your code_patterns.mdc rule.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| expect(getStore1Response.body.encrypted_value).toBe(value1); | ||
|
|
||
| // Get from store 2 | ||
| const getStore2Response = await niceBackendFetch(`/api/latest/data-vault/stores/${store2Id}/get`, { |
There was a problem hiding this comment.
URL is constructed by concatenating strings with variables using template literals. The code_patterns.mdc rule specifically prohibits this pattern and recommends using proper URL construction utilities.
🔍 This comment matches your code_patterns.mdc rule.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| const value2 = "value-2"; | ||
|
|
||
| // Set first key-value pair | ||
| await niceBackendFetch(`/api/latest/data-vault/stores/${storeId}/set`, { |
There was a problem hiding this comment.
URL is constructed by concatenating strings and variables using template literals. This violates the code pattern rule against concatenating URLs as strings with patterns like /users/${userId}.
🔍 This comment matches your code_patterns.mdc rule.
| await niceBackendFetch(`/api/latest/data-vault/stores/${storeId}/set`, { | |
| await niceBackendFetch(new URL(`/api/latest/data-vault/stores/${storeId}/set`, 'https://example.com').pathname, { |
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| }); | ||
|
|
||
| // Set second key-value pair | ||
| await niceBackendFetch(`/api/latest/data-vault/stores/${storeId}/set`, { |
There was a problem hiding this comment.
URL is constructed using string concatenation with template literals. The code_patterns.mdc rule specifically prohibits concatenating URLs as strings with patterns like /users/${userId} and recommends using proper URL construction utilities.
🔍 This comment matches your code_patterns.mdc rule.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| }); | ||
|
|
||
| // Get first value | ||
| const getResponse1 = await niceBackendFetch(`/api/latest/data-vault/stores/${storeId}/get`, { |
There was a problem hiding this comment.
URL is constructed by string concatenation using template literals. This violates the code pattern rule against concatenating URLs as strings with patterns like /users/${userId}.
🔍 This comment matches your code_patterns.mdc rule.
| const getResponse1 = await niceBackendFetch(`/api/latest/data-vault/stores/${storeId}/get`, { | |
| const getResponse1 = await niceBackendFetch(new URL(`/api/latest/data-vault/stores/${storeId}/get`, 'https://localhost').toString(), { |
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| expect(getResponse1.body.encrypted_value).toBe(value1); | ||
|
|
||
| // Get second value | ||
| const getResponse2 = await niceBackendFetch(`/api/latest/data-vault/stores/${storeId}/get`, { |
There was a problem hiding this comment.
URL is constructed using string concatenation with template literals. The code_patterns.mdc rule specifically prohibits concatenating URLs as strings with patterns like /users/${userId} and recommends using proper URL construction utilities.
🔍 This comment matches your code_patterns.mdc rule.
| const getResponse2 = await niceBackendFetch(`/api/latest/data-vault/stores/${storeId}/get`, { | |
| const getResponse2 = await niceBackendFetch(new URL(`/api/latest/data-vault/stores/${storeId}/get`, origin).toString(), { |
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| `); | ||
|
|
||
| // Try to get without auth | ||
| const getResponse = await niceBackendFetch(`/api/latest/data-vault/stores/${storeId}/get`, { |
There was a problem hiding this comment.
URL is constructed using string concatenation with template literals. The code_patterns.mdc rule specifically prohibits concatenating URLs as strings with patterns like /users/${userId} and recommends using proper URL construction utilities.
🔍 This comment matches your code_patterns.mdc rule.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| // Retrieve and parse the JSON | ||
| const retrievedValue = await vault.getValue(key, { secret: testSecret }); | ||
| expect(retrievedValue).toBe(value); | ||
| expect(JSON.parse(retrievedValue!)).toEqual(data); |
There was a problem hiding this comment.
The code uses a TypeScript non-null assertion (!) which violates the code patterns rule. According to the rule, non-null assertions should be replaced with the ?? throwErr("message", { extraData }) pattern. This assertion assumes retrievedValue is not null without proper error handling, which could lead to runtime errors if the value is actually null.
🔍 This comment matches your code_patterns.mdc rule.
| expect(JSON.parse(retrievedValue!)).toEqual(data); | |
| expect(JSON.parse(retrievedValue ?? throwErr("retrievedValue is null"))).toEqual(data); |
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
Review by RecurseML🔍 Review performed on 6a3459e..333ba3a ✅ Files analyzed, no issues (3)• ⏭️ Files skipped (low suspicion) (23)• |
Documentation Changes Requiredstack-app.mdx
Note on DataVaultStoreThe PR introduces a new Please ensure these changes are reflected in the relevant documentation file. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx (1)
263-277: Bug: Webhook breadcrumb builds a Teams URL instead of Webhooks URL.
When viewing a specific webhook, the breadcrumb link points to/teams/${id}; should be/webhooks/${id}.Apply this fix:
- href = `/teams/${match[1]}`; + href = `/webhooks/${match[1]}`;packages/stack-shared/src/utils/bytes.tsx (1)
118-120: Mirror Buffer fallback in decode for large payloads.
atobon very large strings is slow and memory-heavy; prefer Buffer when available.-export function decodeBase64(input: string): Uint8Array { - return new Uint8Array(atob(input).split("").map((char) => char.charCodeAt(0))); -} +export function decodeBase64(input: string): Uint8Array { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + if (typeof Buffer !== "undefined") { + return new Uint8Array(Buffer.from(input, "base64")); + } + const bin = atob(input); + const out = new Uint8Array(bin.length); + for (let i = 0; i < bin.length; i++) out[i] = bin.charCodeAt(i); + return out; +}
♻️ Duplicate comments (13)
packages/stack-shared/src/utils/bytes.tsx (1)
154-164: Make the “large case” round-trip-only and fix variable shadowing.
- Skip equality check against a massive expected string when expected is undefined.
- Avoid shadowing
iin.some()callback.- for (const [i, { input, expected }] of testCases.entries()) { + for (const [i, { input, expected }] of testCases.entries()) { // expect(...) is pretty slow with long inputs, so we throw our own assertions const encoded = encodeBase64(input); - if (encoded !== expected) { + if (expected !== undefined && encoded !== expected) { throw new StackAssertionError(`encodeBase64 test case ${i} failed`); } const decoded = decodeBase64(encoded); - if (decoded.some((b, i) => b !== input[i])) { + if (decoded.some((b, idx) => b !== input[idx])) { throw new StackAssertionError(`decodeBase64 test case ${i} failed`); } }apps/e2e/tests/js/data-vault.test.ts (1)
73-76: Replace non-null assertion with the project’s throwErr patternAvoid
!per code patterns; use?? throwErr(...)instead.- expect(JSON.parse(retrievedValue!)).toEqual(data); + expect(JSON.parse(retrievedValue ?? throwErr("retrievedValue is null"))).toEqual(data);apps/e2e/tests/backend/endpoints/api/v1/data-vault.test.ts (3)
134-166: Test name contradicts expected status (404 vs 400)Snapshot expects 400. Rename for accuracy.
-it("returns 404 when trying to get non-existent value", async ({ expect }: { expect: any }) => { +it("returns 400 when trying to get non-existent value", async ({ expect }: { expect: any }) => {
394-435: Auth test title and setup are inconsistentYou’re using
accessType: "server"(i.e., authenticated) while the test says “requires authentication” and expects success. Either:
- Rename to “works without user sign-in (server access)”, or
- Actually test rejection by using
accessType: "client"and asserting a known error (likelySCHEMA_ERROR400).Example change for rejection path:
- const setResponse = await niceBackendFetch(`/api/latest/data-vault/stores/${storeId}/set`, { + const setResponse = await niceBackendFetch(`/api/latest/data-vault/stores/${storeId}/set`, { method: "POST", - accessType: "server", + accessType: "client", body: { hashed_key: hashedKey, encrypted_value: "test-value" }, }); - expect(setResponse).toMatchInlineSnapshot(` - NiceResponse { - "status": 200, - "body": { "success": true }, - "headers": Headers { <some fields may have been hidden> }, - } - `); + expect(setResponse.status).toBe(400); + expect(setResponse.headers.get("x-stack-known-error")).toBe("SCHEMA_ERROR");Do the same for the GET call.
49-50: URL concatenation patternMultiple places build URLs via template strings. Prefer
new URL(path, base)to comply with repo patterns and avoid edge cases.Also applies to: 90-90, 102-103, 117-117, 141-141, 172-174, 198-198, 231-231, 249-249, 293-293, 303-303, 313-313, 325-325, 350-350, 360-360, 370-370, 382-382, 402-402, 420-420
packages/stack-shared/src/utils/crypto.tsx (3)
49-51: Use globalVar.crypto for RNG to match the rest of the module
This was flagged earlier and still applies here. Align RNG source with generateRandomValues().- const iv = crypto.getRandomValues(new Uint8Array(12)); - const salt = crypto.getRandomValues(new Uint8Array(16)); + const iv = globalVar.crypto.getRandomValues(new Uint8Array(12)); + const salt = globalVar.crypto.getRandomValues(new Uint8Array(16));
62-69: Don’t include raw cipher bytes/text in error data
Leaking ciphertext (and attempted decoding) in errors is unnecessary and risks exposure in logs. Include minimal metadata instead.- if (version[0] !== 0x01 || version[1] !== 0x00) throw new StackAssertionError("Invalid ciphertext version in decrypt(...); expected 0x0100", { purpose, cipher, cipherText: new TextDecoder().decode(cipher) }); + if (version[0] !== 0x01 || version[1] !== 0x00) { + throw new StackAssertionError("Invalid ciphertext version in decrypt(...); expected 0x0100", { + purpose, + cipherLength: cipher.length, + }); + }
27-46: Also switch subtle crypto calls to globalVar.crypto for Node/browser parity
Direct global crypto.subtle can be undefined in some Node runtimes.- const originalSecretKey = await crypto.subtle.importKey("raw", typeof secret === "string" ? new TextEncoder().encode(secret) : secret, "HKDF", false, ["deriveKey"]); - return await crypto.subtle.deriveKey( + const originalSecretKey = await globalVar.crypto.subtle.importKey("raw", typeof secret === "string" ? new TextEncoder().encode(secret) : secret, "HKDF", false, ["deriveKey"]); + return await globalVar.crypto.subtle.deriveKey(packages/stack-shared/src/helpers/vault/client-side.ts (1)
67-73: Test bug: not actually changing the secret
You’re changing the key param, not the secret param. This test doesn’t validate the intended scenario.- import.meta.vitest?.it("should not decrypt a value with a different secret", async ({ expect }) => { + import.meta.vitest?.it("should not decrypt a value with a different secret", async ({ expect }) => { const secret = "test-secret"; const value = "test-value"; const encrypted = await encryptValue(secret, "key", value); - const decrypted = await decryptValue(secret, "different-secret", encrypted); + const decrypted = await decryptValue("different-secret", "key", encrypted); expect(decrypted).not.toEqual(value); });apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page-client.tsx (1)
43-47: Use toast instead of alert for consistency with the UI
Align with the rest of the dashboard UX.- if (deleteConfirmation !== storeId) { - alert("Please type the store ID to confirm deletion"); - return; - } + if (deleteConfirmation !== storeId) { + toast({ title: "Please type the store ID to confirm deletion", variant: "destructive" }); + return; + }packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (1)
1089-1101: Invalidate cache after setValue to avoid stale reads
Without a refresh, callers may read stale values from the cache.setValue: async (key, value, options) => { validateOptions(options); await this._interface.setDataVaultStoreValue(options.secret, id, key, value); + await this._serverDataVaultStoreValueCache.refresh([id, key, options.secret]); },apps/backend/src/app/api/latest/data-vault/stores/[id]/set/route.tsx (1)
26-29: Unify response bodyType with the GET endpointPick one of "success" or "json" for both GET/SET to keep API ergonomics consistent.
packages/stack-shared/src/helpers/vault/server-side.ts (1)
24-41: Remove non-null assertions and handle alias creation racesAvoid KeyMetadata!/KeyId! and handle AlreadyExistsException for CreateAlias.
async function getOrCreateKekId(): Promise<string> { const id = "alias/stack-data-vault-server-side-kek"; const kms = getKmsClient(); try { const describeResult = await kms.send(new DescribeKeyCommand({ KeyId: id })); - if (describeResult.KeyMetadata?.KeyId) return describeResult.KeyMetadata.KeyId; + if (describeResult.KeyMetadata?.KeyId) return describeResult.KeyMetadata.KeyId; } catch (e) { - if (e instanceof Error && e.name !== "NotFoundException") { + if (e instanceof Error && e.name !== "NotFoundException") { throw e; } } const { KeyMetadata } = await kms.send(new CreateKeyCommand({ KeyUsage: "ENCRYPT_DECRYPT", Description: "DataVault KEK" })); - await kms.send(new CreateAliasCommand({ AliasName: id, TargetKeyId: KeyMetadata!.KeyId! })); + const keyId = KeyMetadata?.KeyId; + if (!keyId) throw new Error("Failed to create KMS key"); + try { + await kms.send(new CreateAliasCommand({ AliasName: id, TargetKeyId: keyId })); + } catch (e) { + // tolerate concurrent creators + if (!(e instanceof Error && e.name === "AlreadyExistsException")) throw e; + } return id; }
🧹 Nitpick comments (37)
.vscode/settings.json (1)
16-16: cSpell entries OK; consider lowercase variants if used in code.
If the code uses lowercase “ciphertext/localstack/unhashed/healthcheck”, add lowercase forms to reduce false positives.Also applies to: 41-41, 49-49, 100-100
apps/backend/.env (1)
61-67: Tidy key order and spacing to satisfy dotenv-linter.
Alphabetize keys and remove the extra blank line.Apply:
-# AWS configuration -STACK_AWS_REGION= -STACK_AWS_KMS_ENDPOINT= -STACK_AWS_ACCESS_KEY_ID= -STACK_AWS_SECRET_ACCESS_KEY= - +# AWS configuration +STACK_AWS_ACCESS_KEY_ID= +STACK_AWS_KMS_ENDPOINT= +STACK_AWS_REGION= +STACK_AWS_SECRET_ACCESS_KEY=apps/backend/.env.development (1)
60-64: Alphabetize AWS keys to keep linters happy.
No behavior change; improves consistency.Apply:
-# AWS region defaults to LocalStack -STACK_AWS_REGION=us-east-1 -STACK_AWS_KMS_ENDPOINT=http://localhost:8124 -STACK_AWS_ACCESS_KEY_ID=test -STACK_AWS_SECRET_ACCESS_KEY=test +# AWS region defaults to LocalStack +STACK_AWS_ACCESS_KEY_ID=test +STACK_AWS_KMS_ENDPOINT=http://localhost:8124 +STACK_AWS_REGION=us-east-1 +STACK_AWS_SECRET_ACCESS_KEY=testapps/dev-launchpad/public/index.html (2)
113-115: Clarify S3 mock purpose to avoid confusion with LocalStack.Since both S3 mock and LocalStack are listed, clarifying that 8121 is for storage helps avoid misrouting during development.
- 8121: S3 mock + 8121: S3 mock (storage)
120-124: Add port-range mapping detail for LocalStack.Minor copy improvement to make the mapping explicit and consistent with docker-compose.
- 8150-8199: Reserved for LocalStack (external services) + 8150-8199: Reserved for LocalStack external services (maps to 4510-4559)docker/dependencies/docker.compose.yaml (2)
156-168: LocalStack addition looks good. Consider a lightweight readiness check.Optional: a healthcheck can make tests less flaky by ensuring KMS is up before running e2e.
Would you like me to propose a compose-native healthcheck that probes http://localhost:4566/_localstack/health without assuming curl/wget availability?
178-179: Fix port in Freestyle mock comment.The exposed port is 8122, but the comment still references 8119.
- - "8122:8080" # POST http://localhost:8119/execute/v1/script + - "8122:8080" # POST http://localhost:8122/execute/v1/scriptpackages/stack-shared/src/utils/bytes.tsx (1)
168-170: Optional: reduce timeout back to 10s after slimming the test.With the mega-test removed, 30s is unnecessary.
-}, { - timeout: 30000, -}); +}, { timeout: 10000 });apps/backend/prisma/schema.prisma (1)
775-787: Schema shape and indexes look good; comment on encrypted payload is helpful.
- Composite unique on (tenancyId, storeId, hashedKey) fits the access pattern.
- Index on (tenancyId, storeId) supports per-store scans.
Consider adding a brief note on
hashedKeyformat (e.g., base64url/hex and length) to aid future migrations and debugging.packages/stack-shared/src/config/schema.ts (2)
169-176: Constrain store IDs to our standard ID schema.Use
userSpecifiedIdSchema("storeId")to validate store IDs consistently with other config keys.- dataVault: yupObject({ - stores: yupRecord( - yupString(), - yupObject({ - displayName: yupString(), - }), - ), - }), + dataVault: yupObject({ + stores: yupRecord( + userSpecifiedIdSchema("storeId"), + yupObject({ + displayName: yupString(), + }), + ), + }),
486-494: Defaults look reasonable. Optionally use storeId as default display name.Using the key as a default display name can be more helpful than a generic label.
- dataVault: { - stores: (key: string) => ({ - displayName: "Unnamed Vault", - }), - }, + dataVault: { + stores: (key: string) => ({ + displayName: key, + }), + },packages/stack-shared/src/known-errors.tsx (2)
716-718: Consider 404 instead of 400 for missing resourcesNon-existent store/key reads are resource-absence conditions. Using 404 aligns better with REST semantics and existing patterns (e.g., VerificationCodeNotFound uses 404).
Apply if desired:
- 400, + 404,Also applies to: 730-733
726-738: Avoid returning hashed_key in error details to reduce leakageExposing
hashed_keyin client-visible error details can reveal deterministic identifiers. Prefer omitting it and keeping onlystore_id.- (storeId: string, hashedKey: string) => [ + (storeId: string, _hashedKey: string) => [ - 400, + 404, - `Data vault store with ID ${storeId} does not contain a key with hash ${hashedKey}.`, + `Data vault store with ID ${storeId} does not contain the requested key.`, { store_id: storeId, - hashed_key: hashedKey, }, ] as const, - (json: any) => [json.store_id, json.hashed_key] as const, + (json: any) => [json.store_id, "redacted"] as const,apps/backend/prisma/migrations/20250830000849_data_vault/migration.sql (1)
1-19: Add defaults and an update trigger for timestamps; consider UUID defaultCurrent NOT NULL constraints require callers to always set
idandupdatedAt. Safer to default them and auto-maintainupdatedAt.Propose a follow-up migration:
-- defaults ALTER TABLE "DataVaultEntry" ALTER COLUMN "id" SET DEFAULT gen_random_uuid(), ALTER COLUMN "updatedAt" SET DEFAULT CURRENT_TIMESTAMP; -- trigger to update updatedAt CREATE OR REPLACE FUNCTION set_timestamp() RETURNS TRIGGER AS $$ BEGIN NEW."updatedAt" = NOW(); RETURN NEW; END; $$ LANGUAGE plpgsql; DROP TRIGGER IF EXISTS set_timestamp_on_datavaultentry ON "DataVaultEntry"; CREATE TRIGGER set_timestamp_on_datavaultentry BEFORE UPDATE ON "DataVaultEntry" FOR EACH ROW EXECUTE FUNCTION set_timestamp(); -- optional: guard against empty storeId ALTER TABLE "DataVaultEntry" ADD CONSTRAINT datavault_storeid_not_empty CHECK (length("storeId") > 0);apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/page.tsx (2)
12-16: Remove unused params and asyncThis page doesn’t use
params; make it a sync component and simplify the signature.-export default async function Page({ params }: { params: Promise<Params> }) { - return ( - <PageClient /> - ); -} +export default function Page() { + return <PageClient />; +}
8-10: Drop unusedParamstypeNo longer needed once the function signature is simplified.
-type Params = { - projectId: string, -};packages/template/src/lib/stack-app/data-vault/index.ts (1)
3-8: Deduplicate the options shape for better reuseExtract
{ secret: string }into a named type to keep signatures consistent and future-proof (e.g., adding KDF params).+type DataVaultSecretOptions = { secret: string }; + export type DataVaultStore = & { id: string, - setValue: (key: string, value: string, options: { secret: string }) => Promise<void>, + setValue: (key: string, value: string, options: DataVaultSecretOptions) => Promise<void>, } - & AsyncStoreProperty<"value", [key: string, options: { secret: string }], string | null, false>; + & AsyncStoreProperty<"value", [key: string, options: DataVaultSecretOptions], string | null, false>;apps/e2e/tests/js/data-vault.test.ts (2)
86-96: Right-size the large-payload test to keep CI fast1MB is fine, but 256–512KB usually suffices to catch chunking/streaming issues while cutting runtime.
- // Create a large string (1MB) - const largeValue = 'x'.repeat(1024 * 1024); + // Create a large string (~512KB) + const largeValue = 'x'.repeat(512 * 1024);
226-242: Snapshot where stable; keep dynamic data as-isWhere outputs are stable strings (e.g., special value cases), prefer inline snapshots. Dynamic values (timestamps, Date.now) should remain direct assertions.
Example:
- expect(retrieved).toBe(value); + expect(retrieved).toMatchInlineSnapshot(value)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page.tsx (1)
13-18: Fix Next.js params typing; drop unnecessary await
paramsis not a Promise in App Router pages. The current type compiles but is misleading. Also,asyncisn’t needed.Apply:
-export default async function Page({ params }: { params: Promise<Params> }) { - const { storeId } = await params; - return ( - <PageClient storeId={storeId} /> - ); -} +export default function Page({ params }: { params: Params }) { + const { storeId } = params; + return <PageClient storeId={storeId} />; +}apps/e2e/tests/backend/endpoints/api/v1/data-vault.test.ts (3)
35-38: Use the real hashing function for fidelityStubbed
hashKey()may mask integration issues. Prefer the shared helper.Apply:
-import { it } from "../../../../helpers"; +import { it } from "../../../../helpers"; +import { hashKey as realHashKey } from "@stackframe/stack-shared/dist/helpers/vault/client-side"; ... -function hashKey(key: string): string { - // we don't actually need to hash the key here, so let's keep it simple - return `hashed(${key})`; -} +async function hashKey(key: string): Promise<string> { + // Use a stable test secret; callers already run in server context. + return await realHashKey("test-secret-please-change", key); +}Update call sites to
await hashKey(...).
26-33: Unused helper: add coverage for disabled data vault projects
createDataVaultDisabledProject()is unused. Add a test that hits GET/SET for a project withoutdataVaultconfigured and expectDATA_VAULT_STORE_DOES_NOT_EXIST(not 500).I can draft this test if you want.
59-77: Prefer inline snapshots for consistencySeveral tests assert via
toBe/toMatchObjectwhile others use inline snapshots. Consider snapshotting the fullNiceResponsehere as well for uniformity.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/page-client.tsx (2)
24-38: Replace alert() with toasts for consistent UXUse the existing toast system instead of blocking alerts.
- if (!newStoreId.trim()) { - alert("Store ID is required"); + if (!newStoreId.trim()) { + toast({ title: "Store ID is required" }); return; } - if (!newStoreId.match(/^[a-z0-9-]+$/)) { - alert("Store ID can only contain lowercase letters, numbers, and hyphens"); + if (!newStoreId.match(/^[a-z0-9-]+$/)) { + toast({ title: "Invalid ID", description: "Use lowercase letters, numbers, and hyphens only." }); return; } - if (newStoreId in stores) { - alert("A store with this ID already exists"); + if (newStoreId in stores) { + toast({ title: "Duplicate store ID", description: "A store with this ID already exists." }); return; }
94-113: Avoid unnecessary type assertionsThe
as stringcasts onstoreIdare redundant if keys are typed correctly. Tighten typing onstoreEntriesand drop the casts.- const storeEntries = typedEntries(stores); + const storeEntries = typedEntries<typeof stores>(stores); ... - key={storeId as string} + key={storeId} ... - onClick={() => handleStoreClick(storeId as string)} + onClick={() => handleStoreClick(storeId)} ... - <h3 className="font-semibold text-sm truncate">{storeId as string}</h3> + <h3 className="font-semibold text-sm truncate">{storeId}</h3>packages/stack-shared/src/interface/server-interface.ts (1)
896-900: Wrap decrypt errors with context (storeId, hashedKey)
Decryption failures will currently bubble with little context. Wrap to speed triage.- return await decryptValue(secret, key, encryptedValue); + try { + return await decryptValue(secret, key, encryptedValue); + } catch (e) { + throw new StackAssertionError("Failed to decrypt data vault value", { cause: e, storeId, hashedKey }); + }packages/stack-shared/src/utils/crypto.tsx (3)
53-57: Use globalVar.crypto.subtle.encrypt
Consistency and SSR safety.- const cipher = await crypto.subtle.encrypt({ + const cipher = await globalVar.crypto.subtle.encrypt({
70-75: Use globalVar.crypto.subtle.decrypt
Consistency and SSR safety.- const plaintext = await crypto.subtle.decrypt({ + const plaintext = await globalVar.crypto.subtle.decrypt({
87-89: Use globalVar.crypto.getRandomValues in tests too
Keep tests aligned with runtime code paths.- const exampleKey1 = crypto.getRandomValues(new Uint8Array(32)); - const exampleKey2 = crypto.getRandomValues(new Uint8Array(32)); + const exampleKey1 = globalVar.crypto.getRandomValues(new Uint8Array(32)); + const exampleKey2 = globalVar.crypto.getRandomValues(new Uint8Array(32));apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page-client.tsx (1)
51-53: Remove unused variable
remainingStores is computed but not used.- const { [storeId]: _, ...remainingStores } = config.dataVault.stores; -packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (1)
1089-1091: Prefer StackAssertionError for option validation
Keep error types consistent across the codebase.- const validateOptions = (options: { secret: string }) => { - if (typeof options.secret !== "string") throw new Error("secret must be a string, got " + typeof options.secret); - }; + const validateOptions = (options: { secret: string }) => { + if (typeof options.secret !== "string") { + throw new StackAssertionError(`secret must be a string, got ${typeof options.secret}`); + } + };apps/backend/src/app/api/latest/data-vault/stores/[id]/set/route.tsx (3)
4-4: Avoid importing from package internals (dist)Import from a public export to prevent breakage across build setups.
-import { encryptWithKms } from "@stackframe/stack-shared/dist/helpers/vault/server-side"; +import { encryptWithKms } from "@stackframe/stack-shared/src/helpers/vault/server-side";If possible, re-export from the package entrypoint and import from "@stackframe/stack-shared".
31-34: Harden store config guardIf dataVault or stores is undefined, this will throw before the error is raised.
- if (!(storeId in tenancy.config.dataVault.stores)) { + if (!tenancy.config?.dataVault?.stores || !(storeId in tenancy.config.dataVault.stores)) { throw new KnownErrors.DataVaultStoreDoesNotExist(storeId); }
7-12: Path semantics: prefer resource-oriented routes over “set”Consider POST/PUT to a resource like /data-vault/stores/:id/entries with hashed_key in the body (or /entries/:hashed_key) instead of /set.
packages/stack-shared/src/helpers/vault/server-side.ts (3)
13-22: Reuse KMS client; optionally allow default AWS credentialsCreating a client per call is wasteful. Also allow omitting endpoint/explicit creds to use the default provider chain in non-LocalStack environments.
-let KMS_SINGLETON: KMSClient | undefined; function getKmsClient() { - return new KMSClient({ - region: getEnvVariable("STACK_AWS_REGION"), - endpoint: getEnvVariable("STACK_AWS_KMS_ENDPOINT"), - credentials: { - accessKeyId: getEnvVariable("STACK_AWS_ACCESS_KEY_ID"), - secretAccessKey: getEnvVariable("STACK_AWS_SECRET_ACCESS_KEY") - } - }); + // module-level memo (file scope) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const g = globalThis as any; + if (!g.__stack_kms_client) { + const region = getEnvVariable("STACK_AWS_REGION"); + const endpoint = process.env.STACK_AWS_KMS_ENDPOINT; // optional + const accessKeyId = process.env.STACK_AWS_ACCESS_KEY_ID; // optional + const secretAccessKey = process.env.STACK_AWS_SECRET_ACCESS_KEY; // optional + g.__stack_kms_client = new KMSClient({ + region, + ...(endpoint ? { endpoint } : {}), + ...(accessKeyId && secretAccessKey ? { credentials: { accessKeyId, secretAccessKey } } : {}), + }); + } + return g.__stack_kms_client as KMSClient; }
54-56: Name consistency: prefer edkBase64 over edk_b64-async function unwrapDEK(edk_b64: string) { - const edkBytes = decodeBase64(edk_b64); +async function unwrapDEK(edkBase64: string) { + const edkBytes = decodeBase64(edkBase64);
9-11: Node base64 helpersencodeBase64/decodeBase64 rely on atob/btoa. In Node runtimes these may be absent. Prefer Buffer-based helpers.
Outside this file (packages/stack-shared/src/utils/bytes.tsx):
-export function encodeBase64(input: Uint8Array): string { - return btoa([...input].map((b) => String.fromCharCode(b)).join("")); -} - -export function decodeBase64(input: string): Uint8Array { - return new Uint8Array(atob(input).split("").map((char) => char.charCodeAt(0))); -} +export function encodeBase64(input: Uint8Array): string { + return Buffer.from(input).toString("base64"); +} +export function decodeBase64(input: string): Uint8Array { + return new Uint8Array(Buffer.from(input, "base64")); +}Also applies to: 65-77, 79-91
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
.vscode/settings.json(3 hunks)apps/backend/.env(1 hunks)apps/backend/.env.development(1 hunks)apps/backend/prisma/migrations/20250830000849_data_vault/migration.sql(1 hunks)apps/backend/prisma/schema.prisma(1 hunks)apps/backend/src/app/api/latest/data-vault/stores/[id]/get/route.tsx(1 hunks)apps/backend/src/app/api/latest/data-vault/stores/[id]/set/route.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page-client.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/page-client.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/page.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx(1 hunks)apps/dev-launchpad/public/index.html(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/data-vault.test.ts(1 hunks)apps/e2e/tests/js/data-vault.test.ts(1 hunks)docker/dependencies/docker.compose.yaml(2 hunks)packages/stack-shared/package.json(1 hunks)packages/stack-shared/src/config/schema.ts(2 hunks)packages/stack-shared/src/helpers/vault/client-side.ts(1 hunks)packages/stack-shared/src/helpers/vault/server-side.ts(1 hunks)packages/stack-shared/src/interface/server-interface.ts(2 hunks)packages/stack-shared/src/known-errors.tsx(2 hunks)packages/stack-shared/src/utils/bytes.tsx(2 hunks)packages/stack-shared/src/utils/crypto.tsx(2 hunks)packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts(4 hunks)packages/template/src/lib/stack-app/apps/interfaces/server-app.ts(2 hunks)packages/template/src/lib/stack-app/data-vault/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/e2e/**
📄 CodeRabbit inference engine (CLAUDE.md)
Always add new E2E tests when you change the API or SDK interface
Files:
apps/e2e/tests/backend/endpoints/api/v1/data-vault.test.tsapps/e2e/tests/js/data-vault.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
In tests, prefer .toMatchInlineSnapshot where possible
Files:
apps/e2e/tests/backend/endpoints/api/v1/data-vault.test.tsapps/e2e/tests/js/data-vault.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer ES6 Map over Record where feasible
Files:
apps/e2e/tests/backend/endpoints/api/v1/data-vault.test.tsapps/backend/src/app/api/latest/data-vault/stores/[id]/get/route.tsxpackages/template/src/lib/stack-app/data-vault/index.tsapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/page.tsxapps/e2e/tests/js/data-vault.test.tsapps/backend/src/app/api/latest/data-vault/stores/[id]/set/route.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page-client.tsxpackages/stack-shared/src/utils/bytes.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/page-client.tsxpackages/stack-shared/src/interface/server-interface.tspackages/template/src/lib/stack-app/apps/interfaces/server-app.tspackages/stack-shared/src/helpers/vault/client-side.tspackages/stack-shared/src/known-errors.tsxpackages/stack-shared/src/helpers/vault/server-side.tsapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page.tsxpackages/stack-shared/src/utils/crypto.tsxpackages/template/src/lib/stack-app/apps/implementations/server-app-impl.tspackages/stack-shared/src/config/schema.ts
apps/backend/src/app/api/latest/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place backend API route handlers under /apps/backend/src/app/api/latest and follow RESTful, resource-based paths (auth, users, teams, oauth-providers)
Files:
apps/backend/src/app/api/latest/data-vault/stores/[id]/get/route.tsxapps/backend/src/app/api/latest/data-vault/stores/[id]/set/route.tsx
🧠 Learnings (1)
📚 Learning: 2025-08-24T18:36:37.712Z
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T18:36:37.712Z
Learning: Applies to apps/backend/src/app/api/latest/**/*.ts : Use the custom route handler system in the backend for consistent API responses
Applied to files:
apps/backend/src/app/api/latest/data-vault/stores/[id]/get/route.tsx
🧬 Code graph analysis (17)
apps/e2e/tests/backend/endpoints/api/v1/data-vault.test.ts (2)
packages/stack-shared/src/helpers/vault/client-side.ts (1)
hashKey(21-27)apps/e2e/tests/backend/backend-helpers.ts (1)
niceBackendFetch(107-166)
apps/backend/src/app/api/latest/data-vault/stores/[id]/get/route.tsx (7)
apps/backend/src/app/api/latest/data-vault/stores/[id]/set/route.tsx (1)
POST(7-66)apps/backend/src/route-handlers/smart-route-handler.tsx (1)
createSmartRouteHandler(209-294)packages/stack-shared/src/schema-fields.ts (5)
yupObject(247-251)serverOrHigherAuthTypeSchema(482-482)adaptSchema(330-330)yupString(187-190)yupNumber(191-194)packages/stack-shared/src/known-errors.tsx (2)
KnownErrors(1551-1553)KnownErrors(1555-1675)apps/backend/src/prisma-client.tsx (1)
getPrismaClientForTenancy(51-53)packages/stack-shared/src/utils/errors.tsx (1)
StackAssertionError(69-85)packages/stack-shared/src/helpers/vault/server-side.ts (1)
decryptWithKms(79-91)
packages/template/src/lib/stack-app/data-vault/index.ts (1)
packages/template/src/lib/stack-app/common.ts (1)
AsyncStoreProperty(8-10)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/page.tsx (3)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page.tsx (2)
metadata(4-6)Page(13-18)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page-client.tsx (1)
PageClient(16-245)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/page-client.tsx (1)
PageClient(11-167)
apps/e2e/tests/js/data-vault.test.ts (3)
packages/template/src/lib/stack-app/apps/interfaces/server-app.ts (2)
StackServerApp(17-67)StackServerApp(76-76)apps/e2e/tests/js/js-helpers.ts (1)
createApp(40-77)apps/e2e/tests/helpers.ts (1)
it(10-10)
apps/backend/src/app/api/latest/data-vault/stores/[id]/set/route.tsx (6)
apps/backend/src/app/api/latest/data-vault/stores/[id]/get/route.tsx (1)
POST(8-74)apps/backend/src/route-handlers/smart-route-handler.tsx (1)
createSmartRouteHandler(209-294)packages/stack-shared/src/schema-fields.ts (5)
yupObject(247-251)serverOrHigherAuthTypeSchema(482-482)adaptSchema(330-330)yupString(187-190)yupNumber(191-194)packages/stack-shared/src/known-errors.tsx (2)
KnownErrors(1551-1553)KnownErrors(1555-1675)apps/backend/src/prisma-client.tsx (1)
getPrismaClientForTenancy(51-53)packages/stack-shared/src/helpers/vault/server-side.ts (1)
encryptWithKms(65-77)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page-client.tsx (6)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/page-client.tsx (1)
PageClient(11-167)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/use-admin-app.tsx (1)
useAdminApp(27-34)apps/dashboard/src/components/router.tsx (1)
useRouter(15-33)packages/stack-shared/src/utils/strings.tsx (1)
deindent(235-238)packages/stack-ui/src/components/ui/input.tsx (1)
Input(10-41)apps/dashboard/src/components/code-block.tsx (1)
CodeBlock(17-60)
packages/stack-shared/src/utils/bytes.tsx (1)
packages/stack-shared/src/utils/errors.tsx (1)
StackAssertionError(69-85)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/page-client.tsx (4)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page-client.tsx (1)
PageClient(16-245)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/use-admin-app.tsx (1)
useAdminApp(27-34)apps/dashboard/src/components/router.tsx (1)
useRouter(15-33)packages/stack-shared/src/utils/objects.tsx (1)
typedEntries(263-265)
packages/stack-shared/src/interface/server-interface.ts (3)
packages/stack-shared/src/helpers/vault/client-side.ts (3)
hashKey(21-27)decryptValue(46-55)encryptValue(32-41)packages/stack-shared/src/known-errors.tsx (2)
KnownErrors(1551-1553)KnownErrors(1555-1675)packages/stack-shared/src/utils/errors.tsx (1)
StackAssertionError(69-85)
packages/template/src/lib/stack-app/apps/interfaces/server-app.ts (2)
packages/template/src/lib/stack-app/common.ts (1)
AsyncStoreProperty(8-10)packages/template/src/lib/stack-app/data-vault/index.ts (1)
DataVaultStore(3-8)
packages/stack-shared/src/helpers/vault/client-side.ts (2)
packages/stack-shared/src/utils/crypto.tsx (4)
iteratedHash(108-129)hash(104-106)encrypt(48-60)decrypt(62-76)packages/stack-shared/src/utils/bytes.tsx (2)
encodeBase64(114-116)decodeBase64(118-120)
packages/stack-shared/src/helpers/vault/server-side.ts (3)
packages/stack-shared/src/utils/env.tsx (1)
getEnvVariable(16-58)packages/stack-shared/src/utils/bytes.tsx (2)
decodeBase64(118-120)encodeBase64(114-116)packages/stack-shared/src/utils/crypto.tsx (2)
encrypt(48-60)decrypt(62-76)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page.tsx (1)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page-client.tsx (1)
PageClient(16-245)
packages/stack-shared/src/utils/crypto.tsx (2)
packages/stack-shared/src/utils/bytes.tsx (1)
encodeBase64(114-116)packages/stack-shared/src/utils/errors.tsx (1)
StackAssertionError(69-85)
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (2)
packages/template/src/lib/stack-app/apps/implementations/common.ts (2)
createCache(22-27)useAsyncCache(145-190)packages/template/src/lib/stack-app/data-vault/index.ts (1)
DataVaultStore(3-8)
packages/stack-shared/src/config/schema.ts (1)
packages/stack-shared/src/schema-fields.ts (3)
yupObject(247-251)yupRecord(283-322)yupString(187-190)
🪛 dotenv-linter (3.3.0)
apps/backend/.env.development
[warning] 62-62: [UnorderedKey] The STACK_AWS_KMS_ENDPOINT key should go before the STACK_AWS_REGION key
(UnorderedKey)
[warning] 63-63: [UnorderedKey] The STACK_AWS_ACCESS_KEY_ID key should go before the STACK_AWS_KMS_ENDPOINT key
(UnorderedKey)
apps/backend/.env
[warning] 63-63: [UnorderedKey] The STACK_AWS_KMS_ENDPOINT key should go before the STACK_AWS_REGION key
(UnorderedKey)
[warning] 64-64: [UnorderedKey] The STACK_AWS_ACCESS_KEY_ID key should go before the STACK_AWS_KMS_ENDPOINT key
(UnorderedKey)
[warning] 67-67: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
🪛 Biome (2.1.2)
packages/stack-shared/src/helpers/vault/client-side.ts
[error] 58-58: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test
- GitHub Check: lint_and_build (latest)
- GitHub Check: setup-tests
- GitHub Check: docker
- GitHub Check: docker
- GitHub Check: all-good
- GitHub Check: Security Check
🔇 Additional comments (10)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx (1)
43-44: LGTM – import formatting only.
No semantic changes. Safe.packages/stack-shared/package.json (1)
56-56: No client-facing imports of @aws-sdk/client-kms detected
All references to the KMS client are confined to the server-side vault helper (packages/stack-shared/src/helpers/vault/server-side.ts); no browser or shared entry points import or re-export it.docker/dependencies/docker.compose.yaml (1)
203-204: LGTM: Persisting LocalStack state.Volume addition for LocalStack persistence is appropriate.
packages/stack-shared/src/known-errors.tsx (1)
713-724: New Data Vault known errors look consistent and are correctly exportedConstructors, details payloads, and
constructorArgsFromJsonmatch the internal pattern. Export wiring is correct.Also applies to: 726-738, 1673-1674
packages/template/src/lib/stack-app/apps/interfaces/server-app.ts (2)
59-59: Confirm non-nullability of getDataVaultStore() is intentional
AsyncStoreProperty<"dataVaultStore", ..., DataVaultStore, false>yieldsgetDataVaultStore()/useDataVaultStore()that never returnnull. If an unknown store ID is passed, the error will surface only on subsequent operations. If you intend to signal non-existent stores at fetch time (likegetTeam()), change theValuetoDataVaultStore | null. Otherwise, document the throwing behavior on use.
5-5: Public API surface looks consistentImporting
DataVaultStorehere aligns with its usage and with the impliedgetDataVaultStore/useDataVaultStoremethods. No issues found.packages/stack-shared/src/helpers/vault/client-side.ts (1)
4-7: Confirm purpose strings before release
These purpose strings become part of the KDF/format and are hard to change later. Please confirm they’re final (no typos like repeated “encryption”).packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (2)
26-27: LGTM: added imports for InlineOffer/ServerItem
No issues.
134-136: LGTM: cache wiring for data vault values
Cache key includes storeId, key, and secret; sensible for isolation.apps/backend/src/app/api/latest/data-vault/stores/[id]/set/route.tsx (1)
41-59: Idempotent upsert is the right call hereComposite key + upsert gives deterministic last-write-wins semantics. Looks good.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (1)
1086-1107: Refresh/invalidate cache entries after setValue to prevent stale reads.Without invalidation, subsequent get/use may return the old value.
Apply this diff:
setValue: async (key, value, options) => { validateOptions(options); await this._interface.setDataVaultStoreValue(options.secret, id, key, value); + // Refresh all cached entries for this (storeId,key) across any secrets. + await this._serverDataVaultStoreValueCache.refreshWhere(([sid, k]) => sid === id && k === key); },
🧹 Nitpick comments (2)
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (2)
132-134: Avoid caching decrypted secrets indefinitely; add TTL or opt-out for vault values.This cache holds plaintext (string|null) for [storeId,key,secret] with no TTL. Consider adding a short TTL or a way to disable caching for vault reads to reduce in-memory exposure.
1086-1090: Strengthen secret validation.Type-check alone allows empty/weak secrets; enforce minimal quality or at least non-empty.
Apply this diff:
- const validateOptions = (options: { secret: string }) => { - if (typeof options.secret !== "string") throw new Error("secret must be a string, got " + typeof options.secret); - }; + const validateOptions = (options: { secret: string }) => { + if (typeof options.secret !== "string") throw new Error("secret must be a string, got " + typeof options.secret); + if (options.secret.trim().length === 0) throw new Error("secret must be non-empty"); + // Optional: enforce minimal length/entropy policy + // if (options.secret.length < 8) throw new Error("secret must be at least 8 characters"); + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer ES6 Map over Record where feasible
Files:
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts
🧬 Code graph analysis (1)
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (2)
packages/template/src/lib/stack-app/apps/implementations/common.ts (2)
createCache(22-27)useAsyncCache(145-190)packages/template/src/lib/stack-app/data-vault/index.ts (1)
DataVaultStore(3-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: all-good
- GitHub Check: restart-dev-and-test
- GitHub Check: docker
- GitHub Check: setup-tests
- GitHub Check: lint_and_build (latest)
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: build (22.x)
- GitHub Check: Security Check
🔇 Additional comments (4)
packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (4)
26-27: Imports wired correctly for Data Vault types.InlineOffer/ServerItem re-export and DataVaultStore import look consistent with usage below.
37-37: Platform-gated useAsyncCache import is appropriate.Matches existing pattern used throughout the file.
1109-1117: API surface for getDataVaultStore/useDataVaultStore looks good.Factory + useMemo provide stable instances for React use; server method is straightforward.
132-134: Avoid using the raw secret as a cache key — use a non-reversible alias (HMAC/SHA‑256) or an opaque ID.The cache key includes the secret (server-app-impl.ts:132–134):
private readonly _serverDataVaultStoreValueCache = createCache<[string, string, string], string | null>(async ([storeId, key, secret]) => { return await this._interface.getDataVaultStoreValue(secret, storeId, key); });
- Replace the secret in the dependencies with a stable, non-reversible identifier (e.g., HMAC‑SHA256(secret) using a server-only key or a stored opaque alias) so the raw secret never appears in cache keys, logs, or metrics.
- Verify AsyncCache and any logging/metrics paths do not serialize/log cache keys; if they do, switch to the alias approach and scrub historical logs/metrics as needed.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
apps/backend/src/app/api/latest/data-vault/stores/[id]/set/route.tsx (2)
27-29: Unify response bodyType across GET/SET.GET uses "json" while SET uses "success". Pick one style for consistency (e.g., both "json" with a minimal body, or both "success").
21-24: Rename “encrypted_value” to “value” and add sane size limits.The current name is misleading (server re-encrypts and GET returns the client-encrypted value). Also, inputs are unbounded. Propose rename + constraints; mirror the change in the GET route and tests.
- body: yupObject({ - hashed_key: yupString().defined().nonEmpty(), - encrypted_value: yupString().defined(), - }).defined(), + body: yupObject({ + hashed_key: yupString().defined().nonEmpty().max(128), + value: yupString().defined().nonEmpty().max(1_048_576), // 1 MiB; tune as needed + }).defined(),- async handler({ auth: { tenancy }, params: { id: storeId }, body: { hashed_key: hashedKey, encrypted_value: encryptedValue } }) { + async handler({ auth: { tenancy }, params: { id: storeId }, body: { hashed_key: hashedKey, value } }) {- const encrypted = await encryptWithKms(encryptedValue); + const encrypted = await encryptWithKms(value);Also applies to: 30-31, 41-41
packages/stack-shared/src/utils/crypto.tsx (2)
28-47: Use globalVar.crypto for subtle APIs (consistency/SSR safety).Other helpers in this file use globalVar.crypto.
-async function getDerivedSymmetricKey(purpose: string, secret: string | Uint8Array, salt: Uint8Array) { - const originalSecretKey = await crypto.subtle.importKey("raw", typeof secret === "string" ? new TextEncoder().encode(secret) : secret, "HKDF", false, ["deriveKey"]); - return await crypto.subtle.deriveKey( +async function getDerivedSymmetricKey(purpose: string, secret: string | Uint8Array, salt: Uint8Array) { + const originalSecretKey = await globalVar.crypto.subtle.importKey("raw", typeof secret === "string" ? new TextEncoder().encode(secret) : secret, "HKDF", false, ["deriveKey"]); + return await globalVar.crypto.subtle.deriveKey(
49-53: Use globalVar.crypto for RNG.Align random generation with the rest of the module.
- const iv = crypto.getRandomValues(new Uint8Array(12)); - const salt = crypto.getRandomValues(new Uint8Array(16)); + const iv = globalVar.crypto.getRandomValues(new Uint8Array(12)); + const salt = globalVar.crypto.getRandomValues(new Uint8Array(16));- const exampleKey1 = crypto.getRandomValues(new Uint8Array(32)); - const exampleKey2 = crypto.getRandomValues(new Uint8Array(32)); + const exampleKey1 = globalVar.crypto.getRandomValues(new Uint8Array(32)); + const exampleKey2 = globalVar.crypto.getRandomValues(new Uint8Array(32));Also applies to: 94-96
🧹 Nitpick comments (6)
apps/backend/src/app/api/latest/data-vault/stores/[id]/set/route.tsx (2)
4-4: Avoid importing from package “/dist” paths.Prefer the package’s public export to prevent path breakage under different bundlers/builds.
If available:
- import { encryptWithKms } from "@stackframe/stack-shared/dist/helpers/vault/server-side"; + import { encryptWithKms } from "@stackframe/stack-shared/helpers/vault/server-side";Otherwise, confirm the repo’s convention and align all imports consistently.
21-24: Optional: validate hashed_key format.If hashed_key is hex/base64 of a fixed length, enforce a regex to reduce garbage input.
- hashed_key: yupString().defined().nonEmpty().max(128), + hashed_key: yupString().defined().nonEmpty().max(128) + // example hex-64: adjust to your real format + .matches(/^[a-f0-9]{64}$/i, "invalid hashed_key"),packages/stack-shared/src/utils/crypto.tsx (4)
54-61: Avoid large-array spreads; assemble output with set().Spreading large typed arrays copies through JS arrays and can be slow/memory-heavy.
- const version = [0x01, 0x00]; - return new Uint8Array([...version, ...salt, ...iv, ...new Uint8Array(cipher)]); + const version = new Uint8Array([0x01, 0x00]); + const cipherBytes = new Uint8Array(cipher); + const out = new Uint8Array(2 + 16 + 12 + cipherBytes.length); + out.set(version, 0); + out.set(salt, 2); + out.set(iv, 18); + out.set(cipherBytes, 30); + return out;
63-83: Add quick length check and consistent error path.Short inputs should return a clean Result.error without invoking subtle.decrypt.
export async function decrypt({ purpose, secret, cipher }: { purpose: string, secret: string | Uint8Array, cipher: Uint8Array }) { - const version = cipher.slice(0, 2); + if (cipher.length < 2 + 16 + 12 + 1) { + return Result.error(new Error("Invalid ciphertext: too short")); + } + const version = cipher.slice(0, 2); if (version[0] !== 0x01 || version[1] !== 0x00) throw new StackAssertionError("Invalid ciphertext version in decrypt(...); expected 0x0100", { purpose }); const salt = cipher.slice(2, 18); const iv = cipher.slice(18, 30); const cipherBytes = cipher.slice(30); - const derivedSecretKey = await getDerivedSymmetricKey(purpose, secret, salt); + const derivedSecretKey = await getDerivedSymmetricKey(purpose, secret, salt); try { - const plaintext = await crypto.subtle.decrypt({ + const plaintext = await globalVar.crypto.subtle.decrypt({ name: "AES-GCM", iv, }, derivedSecretKey, cipherBytes); return Result.ok(new Uint8Array(plaintext)); } catch (e) {
115-136: Guard iterations parameter.Ensure iterations >= 1; otherwise PBKDF2 may error or be insecure.
export async function iteratedHash(options: HashOptions & { iterations: number }) { + if (!Number.isInteger(options.iterations) || options.iterations < 1) { + throw new StackAssertionError("iteratedHash: iterations must be >= 1"); + } const stringOrUint8ArrayToUint8Array = (value: string | Uint8Array) => typeof value === "string" ? new TextEncoder().encode(value) : value;
63-66: Optional: expose version constants.Avoid magic numbers by exporting header/salt/iv lengths.
const HEADER_LEN = 2, SALT_LEN = 16, IV_LEN = 12;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/backend/src/app/api/latest/data-vault/stores/[id]/get/route.tsx(1 hunks)apps/backend/src/app/api/latest/data-vault/stores/[id]/set/route.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/page-client.tsx(1 hunks)packages/stack-shared/src/helpers/vault/client-side.ts(1 hunks)packages/stack-shared/src/helpers/vault/server-side.ts(1 hunks)packages/stack-shared/src/utils/crypto.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/stack-shared/src/helpers/vault/server-side.ts
- packages/stack-shared/src/helpers/vault/client-side.ts
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/page-client.tsx
- apps/backend/src/app/api/latest/data-vault/stores/[id]/get/route.tsx
🧰 Additional context used
📓 Path-based instructions (2)
apps/backend/src/app/api/latest/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place backend API route handlers under /apps/backend/src/app/api/latest and follow RESTful, resource-based paths (auth, users, teams, oauth-providers)
Files:
apps/backend/src/app/api/latest/data-vault/stores/[id]/set/route.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer ES6 Map over Record where feasible
Files:
apps/backend/src/app/api/latest/data-vault/stores/[id]/set/route.tsxpackages/stack-shared/src/utils/crypto.tsx
🧬 Code graph analysis (2)
apps/backend/src/app/api/latest/data-vault/stores/[id]/set/route.tsx (6)
apps/backend/src/app/api/latest/data-vault/stores/[id]/get/route.tsx (1)
POST(8-77)apps/backend/src/route-handlers/smart-route-handler.tsx (1)
createSmartRouteHandler(209-294)packages/stack-shared/src/schema-fields.ts (5)
yupObject(247-251)serverOrHigherAuthTypeSchema(482-482)adaptSchema(330-330)yupString(187-190)yupNumber(191-194)packages/stack-shared/src/known-errors.tsx (2)
KnownErrors(1551-1553)KnownErrors(1555-1675)apps/backend/src/prisma-client.tsx (1)
getPrismaClientForTenancy(51-53)packages/stack-shared/src/helpers/vault/server-side.ts (1)
encryptWithKms(66-78)
packages/stack-shared/src/utils/crypto.tsx (3)
packages/stack-shared/src/utils/bytes.tsx (1)
encodeBase64(114-116)packages/stack-shared/src/utils/errors.tsx (1)
StackAssertionError(69-85)packages/stack-shared/src/utils/results.tsx (2)
Result(4-12)Result(26-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: restart-dev-and-test
- GitHub Check: setup-tests
- GitHub Check: all-good
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: lint_and_build (latest)
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: Security Check
🔇 Additional comments (2)
apps/backend/src/app/api/latest/data-vault/stores/[id]/set/route.tsx (2)
32-34: Good guard for unknown store IDs.Validates store existence against tenancy config and surfaces a KnownError.
44-61: No action required: composite unique constraint present
The Prisma schema defines@@unique([tenancyId, storeId, hashedKey])inapps/backend/prisma/schema.prisma, satisfying the upsert key requirement.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts (2)
10-10: Avoid fixed sleeps; make the delay configurable to reduce flakiness and CI runtime.Use a single env-driven constant so we can tune this without touching tests. Apply similarly to other waits in this file.
- await wait(2000); // the event log is async, so let's give it some time to be written to the DB + await wait(EVENT_LOG_FLUSH_MS); // async event log; allow env overrideAdd once near the top (after imports):
// Adjustable in CI: E2E_EVENT_LOG_FLUSH_MS=3000 const EVENT_LOG_FLUSH_MS = Number(process.env.E2E_EVENT_LOG_FLUSH_MS ?? '3000');
128-128: Prefer polling for eventual consistency over a fixed delay.Poll until metrics match the expected snapshot instead of sleeping a fixed time. This lowers flakiness across environments.
- await wait(3000); // the event log is async, so let's give it some time to be written to the DB + await waitForMetricsToEqual(beforeMetrics.body);Add helper (top-level in this file):
async function waitForMetricsToEqual(expectedBody: any, timeoutMs = Number(process.env.E2E_EVENT_LOG_WAIT_MAX_MS ?? '7000')) { const intervalMs = Number(process.env.E2E_EVENT_LOG_WAIT_INTERVAL_MS ?? '100'); const start = Date.now(); while (Date.now() - start < timeoutMs) { const res = await niceBackendFetch("/api/v1/internal/metrics", { accessType: 'admin' }); if (JSON.stringify(res.body) === JSON.stringify(expectedBody)) return; await wait(intervalMs); } throw new Error("Timed out waiting for metrics to reach expected state"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page-client.tsx(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/data-vault.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts(2 hunks)apps/e2e/tests/js/data-vault.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/e2e/tests/js/data-vault.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/data-vault.test.ts
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page-client.tsx
🧰 Additional context used
📓 Path-based instructions (3)
apps/e2e/**
📄 CodeRabbit inference engine (CLAUDE.md)
Always add new E2E tests when you change the API or SDK interface
Files:
apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
In tests, prefer .toMatchInlineSnapshot where possible
Files:
apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer ES6 Map over Record where feasible
Files:
apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts
🧬 Code graph analysis (1)
apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts (1)
packages/stack-shared/src/utils/promises.tsx (1)
wait(260-268)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: restart-dev-and-test
- GitHub Check: setup-tests
- GitHub Check: lint_and_build (latest)
- GitHub Check: all-good
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: docker
- GitHub Check: Security Check
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Important
Introduces Data Vaults with client-side and server-side encryption, new API endpoints, UI components, error handling, and tests.
client-side.ts) and server-side (server-side.ts) encryption.route.tsxfor setting and getting encrypted values.page-client.tsxandpage.tsxfor managing stores.DataVaultStoreDoesNotExistandDataVaultStoreHashedKeyDoesNotExisterrors inknown-errors.tsx.data-vault.test.tsanddata-vault.test.ts.schema.tsto include Data Vault configuration.@aws-sdk/client-kmstopackage.jsonfor KMS integration.docker.compose.yamlfor AWS service emulation.This description was created by
for 1e4daf8. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes / Errors
Tests
Chores