Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
WalkthroughLarge-scale schema API reorganization renaming DTOs and read-only types to Input/Output equivalents, restructuring schema files to separate input and output concerns into dedicated modules, and updating all dependent imports and type signatures throughout the codebase consistently. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Frontend Client
participant Hook as React Hook<br/>(use-credentials)
participant Router as oRPC Router<br/>(credential)
participant Entity as Credential Entity
participant DB as Database
rect rgb(200, 230, 255)
Note over Client,DB: Old Flow (CredentialOutput/DTO)
Client->>Hook: useCredential(id)
Hook->>Router: getCredential(id)
Router->>Entity: CredentialEntity.getRo(data)
Entity->>DB: fetch credential
DB-->>Entity: raw credential data
Entity-->>Router: CredentialOutput
Router-->>Hook: CredentialOutput
Hook-->>Client: CredentialOutput
end
rect rgb(200, 255, 200)
Note over Client,DB: New Flow (CredentialSimpleOutput/Input)
Client->>Hook: useCredential(id)
Hook->>Router: getCredential(id)
Router->>Entity: CredentialEntity.getSimpleRo(data)
Entity->>DB: fetch credential
DB-->>Entity: raw credential data
Entity-->>Router: CredentialSimpleOutput
Router-->>Hook: CredentialSimpleOutput
Hook-->>Client: CredentialSimpleOutput
end
rect rgb(255, 230, 200)
Note over Entity: Enhanced Security Path (New)
Entity->>Entity: getSecuritySettings(data)
Entity->>DB: fetch metadata
DB-->>Entity: metadata with encryption
Entity->>Entity: decrypt security fields
Entity-->>Router: { passwordProtection, twoFactorAuth, accessLogging }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes This PR involves extensive changes across 150+ files with highly consistent patterns (import path updates, type renames, schema reorganization) applied homogeneously. While the scope is large and affects multiple layers (schemas, components, entities, routers, hooks), the changes follow a predictable naming convention (DTO/Ro → Input/Output) and are largely mechanical. The heterogeneity is limited to structural file reorganization and method return type updates. Primary review focus should be on schema integrity, circular dependency prevention, and credential router/entity logic changes (including new Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (13)
entities/secrets/secret/entity.ts (1)
19-19: Critical: Remove encryption ID from client-safe output.The
valueEncryptionIdfield exposes internal encryption infrastructure to the client, violating the guideline to exclude sensitive/internal fields like encryption IDs from RO outputs. Simple Return Objects must not contain sensitive data or internal implementation details.Apply this diff to remove the encryption ID:
userId: entity.userId, containerId: entity.containerId, - - valueEncryptionId: entity.valueEncryptionId, }Based on coding guidelines.
entities/encryption/entity.ts (2)
6-19: CRITICAL: Encryption key exposure in client-facing output.The
getSimpleRomethod returnsencryptionKey(line 14), which violates security best practices. Encryption keys are sensitive internal fields that must never be exposed in Return Objects intended for client consumption.Based on learnings
Apply this diff to remove the encryption key from the output:
static getSimpleRo( entity: EncryptedDataEntitySimpleDbData ): EncryptedDataSimpleOutput { return { id: entity.id, iv: entity.iv, encryptedValue: entity.encryptedValue, - encryptionKey: entity.encryptionKey, createdAt: entity.createdAt, updatedAt: entity.updatedAt, } }Note: This also requires updating the
EncryptedDataSimpleOutputschema inschemas/encryption/output.tsto excludeencryptionKey.
21-28: Remove encryptionKey from getSelect.The
getSelectmethod includesencryptionKey(line 26), which should not be selected when the intent is to exclude it from client-facing data.Apply this diff:
static getSelect() { return { id: true, iv: true, encryptedValue: true, - encryptionKey: true, } satisfies Prisma.EncryptedDataSelect }orpc/routers/platform.ts (1)
24-67: UsePlatformEntity.getSimpleRo()instead of manual object mapping for consistency.The
PlatformEntity.getSimpleRo()helper exists and returns the exact same output shape. Replace lines 50-59 of the handler with:platforms: platforms.map((platform) => PlatformEntity.getSimpleRo(platform))This aligns with the coding guideline to "Always transform database entities to client-safe ROs before returning responses" and maintains consistency across the codebase.
components/app/dashboard-create-container-dialog.tsx (1)
74-80: Set default type without Prisma runtime.Use LIST_CONTAINER_TYPES or a DEFAULT constant; remove the Prisma enum.
- defaultValues: { + defaultValues: { name: "", icon: "📁", description: "", - type: ContainerType.MIXED, + type: (LIST_CONTAINER_TYPES[0] as ContainerInput["type"]), tags: [], },orpc/hooks/use-cards.ts (1)
88-95: Use CardExpiryDateUtils.processServerExpiryDate() for consistent and safe expiry date handling.The code uses
new Date(expiryDate)which cannot parse MM/YY format strings (e.g., "12/25"). SinceUpdateCardInput.expiryDateisDate | string | undefined, andCardSimpleOutput.expiryDatemust be a validDate, the optimistic update will set invalid dates when a MM/YY string is provided. The server-side handler correctly usesCardExpiryDateUtils.processServerExpiryDate()to convert both string and Date inputs; use the same utility here for consistency and safety.if (previousCard) { const { expiryDate, ...safeInput } = input queryClient.setQueryData<CardSimpleOutput>(cardKeys.detail(input.id), { ...previousCard, ...safeInput, - ...(expiryDate && { expiryDate: new Date(expiryDate) }), + ...(expiryDate && { expiryDate: CardExpiryDateUtils.processServerExpiryDate(expiryDate) }), }) }Ensure
CardExpiryDateUtilsis imported at the top oforpc/hooks/use-cards.ts.orpc/routers/card.ts (3)
218-253: Make encryption updates atomic with the card update.createEncryptedData calls and the subsequent card.update run outside a transaction, risking orphaned encryption rows or partial updates on failure. Wrap them in a single $transaction and pass tx into createEncryptedData. As per coding guidelines.
export const updateCard = authProcedure .input(updateCardInputSchema) .output(cardSimpleOutputSchema) .handler(async ({ input, context }): Promise<CardSimpleOutput> => { @@ - // Handle encryption updates if provided - if (updateData.cvvEncryption) { - const cvvEncryptionResult = await createEncryptedData({ + // Handle encryption updates if provided (inside a transaction) + const updatedCard = await database.$transaction(async (tx) => { + // Rebuild updatePayload inside tx to avoid races + const payload: Prisma.CardUpdateInput = { ...updatePayload } + + if (updateData.cvvEncryption) { + const cvvEncryptionResult = await createEncryptedData({ encryptedValue: updateData.cvvEncryption.encryptedValue, encryptionKey: updateData.cvvEncryption.encryptionKey, iv: updateData.cvvEncryption.iv, - }) + }, tx) @@ - updatePayload.cvvEncryption = { - connect: { id: cvvEncryptionResult.encryptedData.id }, - } - } + payload.cvvEncryption = { connect: { id: cvvEncryptionResult.encryptedData.id } } + } - if (updateData.numberEncryption) { - const numberEncryptionResult = await createEncryptedData({ + if (updateData.numberEncryption) { + const numberEncryptionResult = await createEncryptedData({ encryptedValue: updateData.numberEncryption.encryptedValue, encryptionKey: updateData.numberEncryption.encryptionKey, iv: updateData.numberEncryption.iv, - }) + }, tx) @@ - updatePayload.numberEncryption = { - connect: { id: numberEncryptionResult.encryptedData.id }, - } - } - - const updatedCard = await database.card.update({ - where: { id }, - data: updatePayload, - }) + payload.numberEncryption = { connect: { id: numberEncryptionResult.encryptedData.id } } + } + + return tx.card.update({ where: { id }, data: payload }) + })Also applies to: 254-257
33-47: Remove encryption identifiers from public API outputs.The review comment is correct. Verification confirms:
- CardEntity.getSimpleRo and CredentialEntity transformers expose encryption IDs (numberEncryptionId, cvvEncryptionId, passwordEncryptionId) which are sensitive/internal fields that violate the guideline to exclude them from RO outputs.
- cardSimpleOutputSchema explicitly includes numberEncryptionId and cvvEncryptionId (schemas/card/output.ts, lines 29-30).
- CardEntity.getSimpleRo returns these fields (entities/card/card/entity.ts, lines 37-38).
- All four card.ts procedures (getCard, createCard, updateCard, deleteCard) expose these fields via the schema.
- Credential schemas also expose passwordEncryptionId.
The proposed diffs accurately target the necessary changes. Remove encryption IDs from:
- schemas/card/output.ts: cardSimpleOutputSchema (lines 29-30)
- entities/card/card/entity.ts: CardEntity.getSimpleRo return object (lines 37-38)
- Apply equivalent fixes to credential schemas and transformers.
35-41: Use include helpers in all card queries to reduce over-fetching and align with client-safe data access patterns.All queries (lines 35–41, 71–79, 254–257, and others) fetch raw card objects without include/select parameters, then transform them. Update these to use
CardQuery.getSimpleInclude()in each query's include clause. This optimizes database queries to fetch only needed fields and establishes consistent patterns for client-safe data access across the router, per coding guidelines for oRPC routers.components/app/dashboard-credential-detail-view.tsx (1)
262-266: Avoid console in production code.Replace console.error with a logger or remove; rely on toast for UX feedback. As per coding guidelines.
- console.error("Save failed:", error) toast("Failed to save changes", "error")entities/credential/credential/entity.ts (2)
101-112: Ensure switch is exhaustive and always returns.If Prisma adds a new enum, this can return undefined. Add a default branch or an exhaustive check.
static convertPrismaToAccountStatus( status: AccountStatus ): AccountStatusInfer { switch (status) { case AccountStatus.ACTIVE: return accountStatusEnum.ACTIVE case AccountStatus.SUSPENDED: return accountStatusEnum.SUSPENDED case AccountStatus.DELETED: return accountStatusEnum.DELETED case AccountStatus.ARCHIVED: return accountStatusEnum.ARCHIVED + default: + throw new Error(`Unhandled AccountStatus: ${status}`) } }
35-36: Remove encryption identifiers from client outputs and fix related issues.
passwordEncryptionIdis an internal identifier and must not be exposed in client-safe return objects. This violates the coding guideline to exclude sensitive/internal fields (e.g., encryption IDs/keys) from RO outputs.Additionally:
- Remove
console.errorfromgetSecuritySettings(line ~64); replace with logger or silent fallback- Add default cases to non-exhaustive switch statements in
convertAccountStatusTo*methods- Extend removal to other encryption ID fields:
valueEncryptionIdinCredentialKeyValuePairEntity.getSimpleRo, and similar IDs in card and secrets entitiesstatic getSimpleRo( entity: CredentialEntitySimpleDbData ): CredentialSimpleOutput { return { @@ - passwordEncryptionId: entity.passwordEncryptionId, } }Remove
passwordEncryptionIdfromCredentialHistoryEntity.getSimpleRoas well, then update corresponding schema outputs.orpc/routers/credential.ts (1)
579-596: Remove passwordEncryption handling from updateCredential to enforce audit-compliant password updates via dedicated route.The updateCredentialPassword procedure correctly uses database transactions and creates credentialHistory entries, providing proper audit tracking. The updateCredential procedure (lines 579-596) bypasses this history mechanism entirely, creating a security gap.
Two issues must be addressed:
- Remove the passwordEncryption handling block at lines 579-596 as suggested
- Remove
passwordEncryption: encryptedDataInputSchema.optional()from updateCredentialInputSchema in schemas/credential/input.ts (line 46) to prevent the field from being accepted at allThe suggested diff for the router is correct. Additionally, update the schema to remove passwordEncryption from updateCredentialInputSchema and keep it only in updateCredentialPasswordInputSchema and createCredentialInputSchema.
🧹 Nitpick comments (44)
entities/utils/tag/entity.ts (1)
2-2: Use type-only import for TagSimpleOutput.Since
TagSimpleOutputis only used as a type annotation, import it withimport typefor better tree-shaking and clarity.Apply this diff:
-import { TagSimpleOutput } from "@/schemas/utils" +import type { TagSimpleOutput } from "@/schemas/utils"As per coding guidelines.
schemas/credential/output.ts (2)
14-15: Prefer centralized enum schema over z.nativeEnum hereDefine and reuse a shared accountStatusSchema (z.enum([...])) to keep enum exposure consistent across schemas and to export LIST_* values for consumers. Replace the inline z.nativeEnum with the shared schema. As per coding guidelines.
47-53: Add bounds to list pagination fieldsEnforce invariant ranges to avoid invalid paging states.
export const listCredentialsOutputSchema = z.object({ credentials: z.array(credentialIncludeOutputSchema), - total: z.number().int(), + total: z.number().int().min(0), hasMore: z.boolean(), - page: z.number().int(), - limit: z.number().int(), + page: z.number().int().min(1), + limit: z.number().int().min(1).max(100), // adjust max to your standard })Please confirm the canonical max page size used elsewhere so we can match it. As per coding guidelines.
schemas/credential/key-value/input.ts (1)
8-11: Confirm DB nullability and consider .nullish()If credentialMetadataId is DB-nullable (not strictly present), accept null or undefined rather than only omitting the field.
export const keyValueInputSchema = genericEncryptedKeyValuePairInputSchema.extend({ - credentialMetadataId: z.string().optional(), + credentialMetadataId: z.string().nullish(), })Please confirm the Prisma model for credentialMetadataId (nullable vs required). As per coding guidelines.
entities/credential/credential-key-value/entity.ts (2)
126-129: Avoid await in loops; parallelize encryptionUse Promise.all to encrypt pairs concurrently and follow the “don’t use await in loops” guideline.
- for (const pair of validPairs) { - const encryptedPair = await this.encryptKeyValuePair(pair) - encryptedPairs.push(encryptedPair) - } - - return encryptedPairs + const encryptedPairs = await Promise.all( + validPairs.map((pair) => this.encryptKeyValuePair(pair)) + ) + return encryptedPairsAs per coding guidelines.
98-101: Avoid unsafe casts; type generateEncryptionKey to return CryptoKeyRemove “as CryptoKey” by updating generateEncryptionKey’s return type. Small safety/readability win.
schemas/card/enums.ts (1)
8-52: Consider usingz.nativeEnum()for TypeScript enums.The current implementation uses
z.enum([CardProvider.AMEX, ...])which requires manually listing all enum members. SinceCardProvider,CardType, andCardStatusare Prisma-generated TypeScript enums, usingz.nativeEnum()would be more idiomatic and maintainable.Apply this diff for consistency with the pattern used in
schemas/credential/input.ts(line 13):-export const cardProviderSchema = z.enum([ - CardProvider.AMEX, - CardProvider.DISCOVER, - CardProvider.MASTERCARD, - CardProvider.VISA, - CardProvider.JCB, - CardProvider.UNIONPAY, - CardProvider.DINERS_CLUB, -]) +export const cardProviderSchema = z.nativeEnum(CardProvider) -export const cardTypeSchema = z.enum([ - CardType.CREDIT, - CardType.DEBIT, - CardType.PREPAID, - CardType.VIRTUAL, - CardType.NATIONAL, -]) +export const cardTypeSchema = z.nativeEnum(CardType) -export const cardStatusSchema = z.enum([ - CardStatus.ACTIVE, - CardStatus.EXPIRED, - CardStatus.INACTIVE, - CardStatus.BLOCKED, - CardStatus.LOST, -]) +export const cardStatusSchema = z.nativeEnum(CardStatus)schemas/credential/input.ts (1)
24-31: Track the TODO for metadata schema definition.Line 28 includes a TODO comment for the
metadatafield placeholder. This should be addressed to maintain type safety.Would you like me to open a new issue to track defining a proper metadata schema to replace
z.any().optional()?lib/utils/index.ts (1)
2-2: Prefer barrel import for consistency.Line 2 imports directly from
@/schemas/credential/output, while lines 1 and 3 use barrel imports (@/schemas/cardand@/schemas/secrets). For consistency and to align with the established pattern of using barrel exports for public types, import from the credential barrel instead.Apply this diff to use the barrel import:
-import { CredentialSimpleOutput } from "@/schemas/credential/output" +import { CredentialSimpleOutput } from "@/schemas/credential"schemas/credential/with-metadata/output.ts (1)
9-14: Consider a discriminated union for stricter type safety.The current schema allows invalid states (e.g.,
success: truewithoutcredential, orsuccess: falsewithcredential). A discriminated union would enforce mutual exclusivity at the type level.Apply this diff to use a discriminated union:
-export const createWithMetadataOutputSchema = z.object({ - success: z.boolean(), - credential: credentialSimpleOutputSchema.optional(), - error: z.string().optional(), - issues: z.array(z.string()).optional(), -}) +export const createWithMetadataOutputSchema = z.discriminatedUnion("success", [ + z.object({ + success: z.literal(true), + credential: credentialSimpleOutputSchema, + }), + z.object({ + success: z.literal(false), + error: z.string().optional(), + issues: z.array(z.string()).optional(), + }), +])schemas/user/roadmap/output.ts (1)
7-10: Consider a discriminated union for stricter type safety.Similar to the credential metadata output schema, this schema allows invalid states (e.g.,
success: truewitherrorpresent, orsuccess: falsewithouterror). A discriminated union would enforce correctness at the type level.Apply this diff to use a discriminated union:
-export const roadmapSubscribeOutputSchema = z.object({ - success: z.boolean(), - error: z.string().optional(), -}) +export const roadmapSubscribeOutputSchema = z.discriminatedUnion("success", [ + z.object({ + success: z.literal(true), + }), + z.object({ + success: z.literal(false), + error: z.string(), + }), +])components/app/dashboard-credential-cards-view.tsx (1)
6-7: Use type-only imports for types.Both
CredentialIncludeOutputandPlatformSimpleOutputare types and should be imported withimport typesyntax for clarity and to enable better tree-shaking.Apply this diff:
-import type { CredentialIncludeOutput } from "@/schemas/credential" -import type { PlatformSimpleOutput } from "@/schemas/utils" +import type { CredentialIncludeOutput, PlatformSimpleOutput } from "@/schemas/credential"Wait, let me check the actual imports again. Line 6 already has
import type, and line 7 also hasimport type. These are already type-only imports. Let me reconsider.Actually, looking at the annotated code:
- Line 6:
import type { CredentialIncludeOutput } from "@/schemas/credential"- Line 7:
import type { PlatformSimpleOutput } from "@/schemas/utils"These are already using
import type. They could potentially be combined into a single import if both are from the same module, but PlatformSimpleOutput is from "@/schemas/utils" while CredentialIncludeOutput is from "@/schemas/credential", so they can't be combined. The imports are correct.Let me check if there are other issues.
[scratchpad_end] -->components/app/dashboard-add-card-dialog.tsx (1)
5-6: Consider separating type and value imports.For consistency with coding guidelines preferring type-only imports, consider separating the type import from the schema import.
-import { CardInput, cardInputSchema } from "@/schemas/card" -import { TagInput } from "@/schemas/utils" +import type { CardInput, TagInput } from "@/schemas/card" +import { cardInputSchema } from "@/schemas/card"Wait,
TagInputis from@/schemas/utils, not@/schemas/card. Let me correct:-import { CardInput, cardInputSchema } from "@/schemas/card" -import { TagInput } from "@/schemas/utils" +import type { CardInput } from "@/schemas/card" +import type { TagInput } from "@/schemas/utils" +import { cardInputSchema } from "@/schemas/card"orpc/hooks/use-cards.ts (1)
65-66: Remove console usage in hooks.Replace console.error with a logger utility or surface errors to the caller/UI (toast, error boundary). Keeps hooks UI‑agnostic and adheres to the “Don’t use console” guideline.
Also applies to: 107-108, 151-152
orpc/hooks/use-secrets.ts (1)
65-66: Remove console usage in hooks.Use a shared logger or surface errors to UI; avoid console.* in production code.
Also applies to: 108-109, 155-156
components/app/dashboard-create-container-dialog.tsx (1)
209-225: Loading UX: add spinner and disabled style per guidelines.Show a spinner icon and add disabled opacity class; avoid “Creating...” alone.
- <Button + <Button type="submit" - disabled={createContainerMutation.isPending} + disabled={createContainerMutation.isPending} + className="disabled:opacity-50" > - {createContainerMutation.isPending - ? "Creating..." - : "Create Container"} + {createContainerMutation.isPending ? ( + <span className="inline-flex items-center gap-2"> + <Icons.spinner className="size-4 animate-spin" /> + Creating + </span> + ) : ( + "Create Container" + )} </Button>If Icons.spinner isn’t available, use a lightweight spinner component and keep size-4 + animate-spin. As per coding guidelines.
orpc/hooks/use-containers.ts (1)
64-66: Remove console usage in hooks.Replace console.error with a shared logger or propagate errors to UI layers.
Also applies to: 93-96, 138-139, 186-187
orpc/routers/card.ts (1)
100-155: Map Prisma errors to ORPCError codes.Wrap create/update/delete in try/catch and map known Prisma codes (e.g., P2002→CONFLICT, P2025→NOT_FOUND, P2003→BAD_REQUEST). Improves API ergonomics and consistency. As per coding guidelines.
I can add the error mapper utility and wire it here if you want.
Also applies to: 254-257, 279-283
components/app/dashboard-credential-detail-view.tsx (1)
73-81: Use enum constant for default status.Prefer AccountStatus.ACTIVE over string literal "ACTIVE" to keep types strict.
- status: "ACTIVE", + status: AccountStatus.ACTIVE,schemas/card/metadata/input.ts (3)
17-19: Prefer z.unknown over z.any to avoid inferred any.Avoid any in inferred types; use z.unknown for arbitrary payloads.
- otherInfo: z.array(z.any()).optional(), + otherInfo: z.array(z.unknown()).optional(),
35-38: Don’t allow changing the foreign key via update.Prevent updates to cardId; omit it from the updatable shape.
-export const updateCardMetadataInputSchema = z.object({ +export const updateCardMetadataInputSchema = z.object({ id: z.string().min(1, "Metadata ID is required"), - data: cardMetadataInputSchema.partial(), + data: cardMetadataInputSchema.omit({ cardId: true }).partial(), })
53-56: Add pagination to list input.List endpoints should include page/limit with defaults; keep cardId filter. As per coding guidelines.
-export const listCardMetadataInputSchema = z.object({ - cardId: z.string().min(1, "Card ID is required"), -}) +export const listCardMetadataInputSchema = z.object({ + cardId: z.string().min(1, "Card ID is required"), + page: z.number().int().min(1).default(1), + limit: z.number().int().min(1).max(100).default(10), +})entities/credential/credential/entity.ts (3)
87-90: Avoid console; don’t log sensitive errors.Use a safe logger (redacted) or omit logging here; decryption errors should not leak. As per coding guidelines.
- } catch (error) { - console.error("Failed to decrypt security settings:", error) + } catch { + // Intentionally swallow to avoid leaking sensitive details
117-127: Mirror exhaustive handling for reverse conversion.Add default to guarantee a return value.
static convertAccountStatusToPrisma( status: AccountStatusInfer ): AccountStatus { switch (status) { case accountStatusEnum.ACTIVE: return AccountStatus.ACTIVE case accountStatusEnum.SUSPENDED: return AccountStatus.SUSPENDED case accountStatusEnum.DELETED: return AccountStatus.DELETED case accountStatusEnum.ARCHIVED: return AccountStatus.ARCHIVED + default: + throw new Error(`Unhandled AccountStatusInfer: ${status}`) } }
129-140: Keep string conversion exhaustive too.Add default branch.
static convertAccountStatusToString(status: AccountStatusInfer): string { switch (status) { case accountStatusEnum.ACTIVE: return "Active" case accountStatusEnum.SUSPENDED: return "Suspended" case accountStatusEnum.DELETED: return "Deleted" case accountStatusEnum.ARCHIVED: return "Archived" + default: + return String(status) } }orpc/hooks/use-credentials.ts (3)
3-19: Sort imports per project order (React, third‑party, internal, type‑only).Move @tanstack/react-query above internal "@/..." imports; keep type-only imports grouped last. Improves consistency and avoids churn. As per coding guidelines.
196-211: Avoid caching fields not present in CredentialSimpleOutput during optimistic update.Spreading full input into the cached RO can stash extra fields and drift types. Only merge allowed fields.
Apply a minimal merge:
- queryClient.setQueryData<CredentialSimpleOutput>( - credentialKeys.detail(input.id), - { - ...previousCredential, - ...input, - } - ) + queryClient.setQueryData<CredentialSimpleOutput>( + credentialKeys.detail(input.id), + { + ...previousCredential, + identifier: + input.identifier ?? previousCredential.identifier, + description: + input.description ?? previousCredential.description, + status: input.status ?? previousCredential.status, + platformId: + input.platformId ?? previousCredential.platformId, + containerId: + input.containerId ?? previousCredential.containerId, + } + )
151-153: Replace console.error with structured logger and avoid sensitive data.Console usage is disallowed; use an app logger (levels, redaction). Ensure no PII/secrets in messages. As per coding guidelines.
Also applies to: 177-179, 223-224, 266-269, 317-319, 347-348, 370-371, 409-410
schemas/card/input.ts (2)
49-51: Make tags optional with a safe default.Most callers don’t have tags; default to [] to simplify handlers.
- tags: z.array(tagInputSchema), + tags: z.array(tagInputSchema).default([]),
1-4: Reorder imports per convention (third‑party before internal).Place zod and @prisma/client imports before "@/schemas/...".
-import { encryptedDataInputSchema } from "@/schemas/encryption" -import { tagInputSchema } from "@/schemas/utils/tag/input" -import { CardProvider, CardStatus, CardType } from "@prisma/client" -import { z } from "zod" +import { z } from "zod" +import { CardProvider, CardStatus, CardType } from "@prisma/client" +import { encryptedDataInputSchema } from "@/schemas/encryption" +import { tagInputSchema } from "@/schemas/utils/tag/input"schemas/credential/metadata/input.ts (3)
12-17: Unify optional+empty-string pattern for consistency.Prefer union(...).optional() used elsewhere (e.g., cardholderEmail) for clarity.
- recoveryEmail: z.string().email().optional().or(z.literal("")), - phoneNumber: z.string().optional().or(z.literal("")), + recoveryEmail: z.union([z.string().email(), z.literal("")]).optional(), + phoneNumber: z.union([z.string(), z.literal("")]).optional(),
16-17: Default empty arrays to avoid null checks downstream.- keyValuePairs: z.array(metadataKeyValueInputSchema).optional(), + keyValuePairs: z.array(metadataKeyValueInputSchema).default([]).optional(),
1-3: Import order: third‑party before internal.-import { genericEncryptedKeyValuePairInputSchema } from "@/schemas/encryption" -import { z } from "zod" +import { z } from "zod" +import { genericEncryptedKeyValuePairInputSchema } from "@/schemas/encryption"orpc/routers/user.ts (2)
98-100: Use a structured logger and redact PII; avoid console in server code.Swap console.* for a logger (context, levels, error codes). Don’t log raw emails/DB meta in prod. As per coding guidelines.
Also applies to: 111-116, 134-141, 145-151, 155-155, 246-248, 259-264, 282-289, 293-299, 303-307, 341-343
1-35: Group and sort imports.Order: third‑party (zod, @orpc/server, @prisma/*), then internal "@/...".
orpc/routers/credential.ts (5)
809-811: Use top‑level node:crypto import; avoid dynamic imports.Reduces overhead and aligns with Node import convention.
+// at top-level imports +import { randomBytes } from "node:crypto" @@ - const crypto = await import("crypto") - const encryptionKey = crypto.randomBytes(32).toString("base64") // 256-bit key + const encryptionKey = randomBytes(32).toString("base64") // 256-bit key @@ - const crypto = await import("crypto") - const encryptionKey = crypto.randomBytes(32).toString("base64") + const encryptionKey = randomBytes(32).toString("base64") @@ - const crypto = await import("crypto") - const encryptionKey = crypto.randomBytes(32).toString("base64") + const encryptionKey = randomBytes(32).toString("base64")Also applies to: 1138-1140, 1182-1184
128-135: Limit include for key‑only pairs.Fetching full include to return only id/key wastes bandwidth. Select just metadata.keyValuePairs { id, key, createdAt, updatedAt }.
250-257: Fetch single key‑value pair directly instead of loading all and filtering in memory.Query the specific pair with a join that checks credentialId/userId, and select only valueEncryption. Lowers memory and latency.
// Example const kv = await database.credentialKeyValuePair.findFirst({ where: { id: input.keyValuePairId, credentialMetadata: { credential: { id: input.credentialId, userId: context.user.id }, }, }, include: { valueEncryption: true }, })
1110-1115: Normalize keys when computing deletions to avoid whitespace mismatches.Trim incoming keys before comparing to existing; prevents accidental deletes.
- const newKeys = keyValuePairs.map((kv) => kv.key) + const newKeys = keyValuePairs.map((kv) => kv.key.trim())Also applies to: 1121-1124, 1127-1133
220-221: Swap console. for structured logging and redact secrets.*Don’t log decrypted values or encryption details; prefer error codes and context.
Also applies to: 346-346, 1026-1026
orpc/routers/container.ts (4)
21-25: Remove unused type import.
CreateWithSecretsInputisn’t used.-import { - createWithSecretsInputSchema, - createWithSecretsOutputSchema, - type CreateWithSecretsInput, - type CreateWithSecretsOutput, -} from "@/schemas/utils/container/with-secrets" +import { + createWithSecretsInputSchema, + createWithSecretsOutputSchema, + type CreateWithSecretsOutput, +} from "@/schemas/utils/container/with-secrets"
117-119: Map Prisma unique violations to ORPC CONFLICT on create.Wrap creation with a try/catch and translate
P2002toORPCError("CONFLICT")to provide predictable errors..handler(async ({ input, context }): Promise<ContainerSimpleOutput> => { - if (context.permissions?.canOnlyAccessDefaultContainers) { + if (context.permissions?.canOnlyAccessDefaultContainers) { throw new ORPCError("FORBIDDEN", { message: "Your plan only allows access to default containers.", }) } - - const container = await database.$transaction(async (tx) => { - const tagConnections = await createTagsAndGetConnections( - input.tags, - context.user.id, - undefined, - tx - ) - - return await tx.container.create({ - data: { - name: input.name, - icon: input.icon, - description: input.description, - type: input.type, - userId: context.user.id, - tags: tagConnections, - }, - }) - }) + try { + const container = await database.$transaction(async (tx) => { + const tagConnections = await createTagsAndGetConnections( + input.tags, + context.user.id, + undefined, + tx + ) + return tx.container.create({ + data: { + name: input.name, + icon: input.icon, + description: input.description, + type: input.type, + userId: context.user.id, + tags: tagConnections, + }, + }) + }) + return ContainerEntity.getSimpleRo(container) + } catch (e) { + if (e instanceof Prisma.PrismaClientKnownRequestError && e.code === "P2002") { + throw new ORPCError("CONFLICT", { message: "Container with this name already exists." }) + } + throw e + } - - return ContainerEntity.getSimpleRo(container) +As per coding guidelines.
Also applies to: 125-146
183-195: Make update+tags atomic and pass tx to tag helper.Currently tag creation and container update are separate calls; wrap in a single transaction and pass
txto avoid partial updates.- // Handle tags if provided - if (updateData.tags !== undefined) { - const tagConnections = await createTagsAndGetConnections( - updateData.tags, - context.user.id, - undefined - ) - updatePayload.tags = tagConnections - } - - const updatedContainer = await database.container.update({ - where: { id }, - data: updatePayload, - }) + const updatedContainer = await database.$transaction(async (tx) => { + // Handle tags if provided (within tx) + if (updateData.tags !== undefined) { + const tagConnections = await createTagsAndGetConnections( + updateData.tags, + context.user.id, + undefined, + tx + ) + updatePayload.tags = tagConnections + } + return tx.container.update({ + where: { id }, + data: updatePayload, + }) + })As per coding guidelines.
249-258: Also map Prisma errors in createWithSecrets.Add
P2002mapping for conflicts (e.g., unique names) similar tocreateContainerto keep error semantics consistent.} catch (e) { if (e instanceof Prisma.PrismaClientKnownRequestError && e.code === "P2002") { throw new ORPCError("CONFLICT", { message: "A unique constraint was violated." }) } ... }As per coding guidelines.
| const { credential, platforms } = data | ||
| const platform = platforms.platforms.find( | ||
| (p) => p.id === credential.platformId | ||
| (p: { id: string; name: string }) => p.id === credential.platformId |
There was a problem hiding this comment.
🧩 Analysis chain
Consider using the schema-defined platform output type instead of the inline object type.
The explicit type annotation improves type safety, but given the PR's focus on schema reorganization and the context mentioning PlatformSimpleOutput, the inline { id: string; name: string } may not align with the refactored schema structure. This type duplication could make future schema updates error-prone.
Can you verify whether a PlatformSimpleOutput or similar platform-output type exists in the reorganized schemas, and whether it should be imported and used here? This will help ensure consistency with the schema refactoring objectives.
🏁 Script executed:
#!/bin/bash
# Search for PlatformSimpleOutput or similar platform-related output types in schemas
# Check for platform output types in schemas folder
fd -t f '\.ts$' 'schemas' --exec grep -l 'PlatformSimpleOutput\|PlatformOutput' {} \;
# Also search for any platform schema definitions with output/output suffixes
rg -n 'export.*Platform.*Output|type PlatformSimpleOutput|interface PlatformSimpleOutput' --type tsLength of output: 481
🏁 Script executed:
#!/bin/bash
# View the platformSimpleOutputSchema definition
cat schemas/utils/platform/output.tsLength of output: 1082
🏁 Script executed:
#!/bin/bash
# View page component imports and line 61 context
head -70 app/'(dashboard)'/dashboard/accounts/'[id]'/page.tsx | tail -20Length of output: 674
🏁 Script executed:
#!/bin/bash
# View full context around line 61, including imports
sed -n '1,70p' app/'(dashboard)'/dashboard/accounts/'[id]'/page.tsxLength of output: 1951
🏁 Script executed:
#!/bin/bash
# Check for other inline type annotations in the page file
rg -n '\(.*:\s*\{' app/'(dashboard)'/dashboard/accounts/'[id]'/page.tsxLength of output: 74
Replace the inline type annotation with PlatformSimpleOutput from the reorganized schemas.
At line 61, the code defines an inline type { id: string; name: string } that should instead use the schema-defined PlatformSimpleOutput type. The schema file schemas/utils/platform/output.ts exports PlatformSimpleOutput, which is the proper type for platform objects returned from serverClient.platforms.list().
Add the import and update line 61:
import { PlatformSimpleOutput } from "@/schemas/utils"Then replace:
(p: { id: string; name: string }) => p.id === credential.platformIdWith:
(p: PlatformSimpleOutput) => p.id === credential.platformIdThis ensures consistency with the schema refactoring objectives and follows the coding guideline to use centralized exported types rather than inline duplicates.
🤖 Prompt for AI Agents
In app/(dashboard)/dashboard/accounts/[id]/page.tsx around line 61, replace the
inline type annotation for platform objects with the centralized schema type:
add an import for PlatformSimpleOutput from "@/schemas/utils" at the top of the
file, then change the predicate signature from the inline { id: string; name:
string } to use PlatformSimpleOutput so the line becomes a
PlatformSimpleOutput-typed parameter checking p.id === credential.platformId;
ensure TypeScript references the new import and remove the inline type.
| type ContainerInput, | ||
| } from "@/schemas/utils" | ||
| import { zodResolver } from "@hookform/resolvers/zod" | ||
| import { ContainerType } from "@prisma/client" |
There was a problem hiding this comment.
Don’t import Prisma enums in client components.
@prisma/client should not ship to the browser. Use LIST_CONTAINER_TYPES (and optionally export DEFAULT_CONTAINER_TYPE) from your schemas barrel instead.
-import { ContainerType } from "@prisma/client"
+// Use shared enum values to avoid bundling Prisma on the clientCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In components/app/dashboard-create-container-dialog.tsx around line 11, remove
the direct import of ContainerType from @prisma/client (Prisma enums must not be
shipped to the browser) and instead import LIST_CONTAINER_TYPES (and optionally
DEFAULT_CONTAINER_TYPE) from your server/schemas barrel export; update any code
that used ContainerType to use the client-safe LIST_CONTAINER_TYPES array or the
DEFAULT_CONTAINER_TYPE value (or derive a string union type from that array) so
the component only depends on browser-safe constants exported from your schemas
barrel.
| import { usePlatforms } from "@/orpc/hooks/use-platforms" | ||
| import { AccountStatusInfer } from "@/schemas/credential/credential" | ||
| import type { CredentialOutput } from "@/schemas/credential/dto" | ||
| import { AccountStatusInfer } from "@/schemas/credential" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify if AccountStatusInfer needs type-only import.
Based on the naming convention (suffix "Infer"), AccountStatusInfer appears to be a type. If it's only used for type annotations (lines 32, 50, 78), it should use import type syntax.
Run this script to check how AccountStatusInfer is used:
🏁 Script executed:
#!/bin/bash
# Check if AccountStatusInfer is used as a type or value
rg -nP '\bAccountStatusInfer\b' --type=tsx -C2Length of output: 27
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for AccountStatusInfer usage - try without file type restriction
rg -nP '\bAccountStatusInfer\b' -C2 --glob '*.tsx' --glob '*.ts'
Length of output: 4988
🏁 Script executed:
#!/bin/bash
# Also directly inspect the file mentioned in the review
cat components/app/dashboard-credential-sidebar.tsxLength of output: 5903
🏁 Script executed:
#!/bin/bash
# Check the schema file where it's imported from
cat components/app/dashboard-credential-sidebar.tsx | head -20Length of output: 789
🏁 Script executed:
#!/bin/bash
# Find and inspect the credential schema
find . -type f -name "*credential*" -path "*/schemas/*" | head -5Length of output: 0
🏁 Script executed:
#!/bin/bash
# Verify it's a type from the schema file name pattern - types ending in "Infer" are typically TypeScript types
echo "AccountStatusInfer naming pattern: 'Infer' suffix suggests TypeScript type (e.g., z.infer<typeof schema>)"Length of output: 105
Change line 6 to use type-only import syntax.
The import of AccountStatusInfer should use import type because it is only used for type annotations (lines 32, 50, 78) and never as a runtime value. This aligns with the coding guideline to use type-only imports when possible.
Change:
import { AccountStatusInfer } from "@/schemas/credential"
To:
import type { AccountStatusInfer } from "@/schemas/credential"
🤖 Prompt for AI Agents
In components/app/dashboard-credential-sidebar.tsx around line 6, the import for
AccountStatusInfer is used only as a type and should use type-only import
syntax; update the import statement to use "import type { AccountStatusInfer }
from '@/schemas/credential'" so the compiler/tree-shaker knows it's a type-only
import and no runtime value is imported.
| @@ -1,10 +1,10 @@ | |||
| import { PlatformSimpleRo } from "@/schemas/utils/platform" | |||
| import { PlatformSimpleOutput } from "@/schemas/utils" | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use type-only import for PlatformSimpleOutput.
Since PlatformSimpleOutput is only used in type positions (return type annotations), it should be imported as a type-only import to improve tree-shaking and clarify intent.
Apply this diff:
-import { PlatformSimpleOutput } from "@/schemas/utils"
+import type { PlatformSimpleOutput } from "@/schemas/utils"As per coding guidelines.
🤖 Prompt for AI Agents
In entities/utils/platform/entity.ts around line 1, the import of
PlatformSimpleOutput is used only as a type and should be a type-only import;
change the import to a type-only form (e.g., use "import type {
PlatformSimpleOutput } from \"@/schemas/utils\"") so the compiler and bundler
can treat it as purely a type import while leaving any other imports unchanged.
| queryClient.setQueryData<SecretSimpleOutput>( | ||
| secretKeys.detail(input.id), | ||
| { | ||
| ...previousSecret, | ||
| ...input, | ||
| } | ||
| ) |
There was a problem hiding this comment.
Security: Don’t merge plaintext secret fields into client cache.
Spreading ...input may include secret value fields, leaking them into React Query cache. Omit sensitive props before updating cache.
- queryClient.setQueryData<SecretSimpleOutput>(
- secretKeys.detail(input.id),
- {
- ...previousSecret,
- ...input,
- }
- )
+ const { value, secret, secretValue, ...safeInput } = input as Record<string, unknown>
+ queryClient.setQueryData<SecretSimpleOutput>(
+ secretKeys.detail(input.id),
+ { ...(previousSecret as SecretSimpleOutput), ...(safeInput as Partial<SecretSimpleOutput>) }
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| queryClient.setQueryData<SecretSimpleOutput>( | |
| secretKeys.detail(input.id), | |
| { | |
| ...previousSecret, | |
| ...input, | |
| } | |
| ) | |
| const { value, secret, secretValue, ...safeInput } = input as Record<string, unknown> | |
| queryClient.setQueryData<SecretSimpleOutput>( | |
| secretKeys.detail(input.id), | |
| { ...(previousSecret as SecretSimpleOutput), ...(safeInput as Partial<SecretSimpleOutput>) } | |
| ) |
🤖 Prompt for AI Agents
In orpc/hooks/use-secrets.ts around lines 89 to 95, the cache update spreads the
entire input into the React Query cache which may contain plaintext secret
fields; instead sanitize the data before calling queryClient.setQueryData by
constructing a new object that merges previousSecret with only non-sensitive
fields from input (or explicitly omits keys like value, secret, plaintext,
token), or use a whitelist of allowed fields to copy; ensure previousSecret is
preserved as fallback and only safe properties are written to the cache.
| export const encryptedDataSimpleOutputSchema = z.object({ | ||
| id: z.string(), | ||
| iv: z.string(), | ||
| encryptionKey: z.string(), | ||
| encryptedValue: z.string(), | ||
| createdAt: z.date(), | ||
| updatedAt: z.date(), | ||
| }) | ||
|
|
||
| export type EncryptedDataSimpleOutput = z.infer< | ||
| typeof encryptedDataSimpleOutputSchema | ||
| > |
There was a problem hiding this comment.
CRITICAL: Encryption key included in output schema.
The encryptedDataSimpleOutputSchema includes encryptionKey (line 10), which is a sensitive internal field that must never be exposed in client-facing outputs. This violates security best practices.
Based on learnings
Apply this diff to remove the encryption key from the output schema:
export const encryptedDataSimpleOutputSchema = z.object({
id: z.string(),
iv: z.string(),
- encryptionKey: z.string(),
encryptedValue: z.string(),
createdAt: z.date(),
updatedAt: z.date(),
})Note: This change must be coordinated with the fix in entities/encryption/entity.ts to ensure consistency across the schema and entity layers.
🤖 Prompt for AI Agents
In schemas/encryption/output.ts around lines 7 to 18, the output schema
incorrectly exposes the sensitive field encryptionKey; remove encryptionKey from
encryptedDataSimpleOutputSchema so client-facing outputs no longer include it,
update the exported type accordingly (infer from the revised schema), and
coordinate the change with entities/encryption/entity.ts to ensure that the
entity layer still stores/handles encryptionKey internally but does not rely on
the output schema exposing it.
| export * as credential from "./credential" | ||
| export * as card from "./card" | ||
| export * as utils from "./utils" | ||
| export * as secret from "./secrets" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the namespace name for secrets module.
The folder is named secrets (plural), but it's exported as secret (singular). This naming inconsistency could cause confusion. Verify whether this is intentional.
If the singular form is intentional for consistency with other modules, consider renaming the folder from secrets to secret for clarity.
🏁 Script executed:
#!/bin/bash
# Check how the secrets module is imported elsewhere in the codebase
rg -n "from ['\"]@/schemas['\"]" --type=ts --type=tsx -A1 | rg -P "(secret|secrets)" -C1Length of output: 27
🏁 Script executed:
#!/bin/bash
# 1. List the directory structure of schemas folder
echo "=== Schemas directory structure ==="
find schemas -type f -name "*.ts" | head -20
echo -e "\n=== Looking for secrets folder ==="
find schemas -type d -name "*secret*"
echo -e "\n=== Contents of schemas/index.ts (around line 15) ==="
sed -n '10,20p' schemas/index.ts
echo -e "\n=== Search for imports using @/schemas pattern ==="
rg -n "from ['\"]@/schemas['\"]" -A2 -B0 2>/dev/null | head -30
echo -e "\n=== Search for secret/secrets usage in imports ==="
rg -n "\.secret|\.secrets" -A0 -B0 --type=ts 2>/dev/null | head -20Length of output: 2932
🏁 Script executed:
#!/bin/bash
# Check actual imports of the secret/secrets schema
echo "=== Direct imports from @/schemas/secret or @/schemas/secrets ==="
rg -n "from ['\"]@/schemas/secret" --type=ts 2>/dev/null | head -20
echo -e "\n=== Check orpc structure to understand .secrets usage ==="
find . -path ./node_modules -prune -o -type f -name "*.ts" -exec grep -l "orpc\.secrets" {} \; 2>/dev/null | head -5
echo -e "\n=== Look at orpc main structure ==="
find orpc -name "index.ts" -o -name "routers.ts" | head -5
echo -e "\n=== Check what's in orpc/routers directory ==="
ls -la orpc/routers/ 2>/dev/null | grep -E "\.ts$"Length of output: 1781
Rename the secrets folder to secret for consistency with other schema modules.
The review comment is valid. The folder is named secrets (plural) while exported as secret (singular), inconsistent with all other schema modules (credential, card, utils, encryption, user) which use singular names for both folder and export. Additionally, the codebase currently imports directly from @/schemas/secrets (bypassing the namespace export), indicating the export name mismatch isn't being used. Renaming the folder to secret aligns with the established pattern and coding guideline requiring naming consistency across the codebase.
🤖 Prompt for AI Agents
In schemas/index.ts around line 15, the export uses "secret" while the actual
folder is named "secrets", causing an inconsistency with other schema modules
and imports bypassing the namespace; rename the folder "schemas/secrets" to
"schemas/secret" and update any direct imports (e.g., "@/schemas/secrets") to
"@/schemas/secret" so the namespace export and filesystem match, then run a
project-wide search/replace and update any import paths or tests accordingly.
| export const secretInputSchema = z.object({ | ||
| name: z.string().min(1, "Name is required"), | ||
| note: z.string().optional(), | ||
|
|
||
| valueEncryption: encryptedDataInputSchema, | ||
| metadata: z.array(secretMetadataInputSchema), | ||
|
|
||
| containerId: z.string(), | ||
| }) |
There was a problem hiding this comment.
Fix create shape: optional metadata and no secretId requirement.
metadata is required and each item requires secretId, which isn't known at create time. Make metadata optional with default [] and use a create-specific metadata schema without secretId. As per coding guidelines.
-import { secretMetadataInputSchema } from "./metadata/input"
+import { secretMetadataInputSchema } from "./metadata/input"
+// Create-time metadata omits FK; server assigns it
+const secretMetadataCreateInputSchema = secretMetadataInputSchema.omit({ secretId: true })
@@
-export const secretInputSchema = z.object({
+export const secretInputSchema = z.object({
name: z.string().min(1, "Name is required"),
note: z.string().optional(),
valueEncryption: encryptedDataInputSchema,
- metadata: z.array(secretMetadataInputSchema),
- containerId: z.string(),
+ metadata: z.array(secretMetadataCreateInputSchema).optional().default([]),
+ containerId: z.string(),
})
@@
-export const createSecretInputSchema = secretInputSchema
+export const createSecretInputSchema = secretInputSchemaAlso applies to: 26-30
🤖 Prompt for AI Agents
In schemas/secrets/input.ts around lines 10-18 (and also apply same change to
lines 26-30), the secret input currently requires metadata and each metadata
item requires secretId which is not available at create time; change metadata to
be optional with a default empty array and replace the referenced schema with a
create-specific metadata schema that does not require secretId (e.g.,
secretMetadataCreateInputSchema), i.e., update the metadata field to use
z.array(secretMetadataCreateInputSchema).optional().default([]) and ensure the
create-specific schema omits the secretId requirement.
| export const secretMetadataInputSchema = z.object({ | ||
| type: z.nativeEnum(SecretType), | ||
| status: z.nativeEnum(SecretStatus), | ||
| expiresAt: z.date().optional(), | ||
| otherInfo: z.array(z.any()), | ||
| secretId: z.string(), | ||
| }) |
There was a problem hiding this comment.
Replace z.any() with a more type-safe alternative.
Line 12 uses z.array(z.any()) for otherInfo, which defeats type safety and violates the coding guideline "Disallow any and implicit any."
Consider one of these alternatives:
- otherInfo: z.array(z.any()),
+ otherInfo: z.array(z.unknown()),Or define a specific schema if the structure is known:
+const secretOtherInfoSchema = z.object({
+ // Define expected fields here
+})
+
export const secretMetadataInputSchema = z.object({
type: z.nativeEnum(SecretType),
status: z.nativeEnum(SecretStatus),
expiresAt: z.date().optional(),
- otherInfo: z.array(z.any()),
+ otherInfo: z.array(secretOtherInfoSchema),
secretId: z.string(),
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const secretMetadataInputSchema = z.object({ | |
| type: z.nativeEnum(SecretType), | |
| status: z.nativeEnum(SecretStatus), | |
| expiresAt: z.date().optional(), | |
| otherInfo: z.array(z.any()), | |
| secretId: z.string(), | |
| }) | |
| export const secretMetadataInputSchema = z.object({ | |
| type: z.nativeEnum(SecretType), | |
| status: z.nativeEnum(SecretStatus), | |
| expiresAt: z.date().optional(), | |
| otherInfo: z.array(z.unknown()), | |
| secretId: z.string(), | |
| }) |
🤖 Prompt for AI Agents
In schemas/secrets/metadata/input.ts around lines 8 to 14, replace the unsafe
z.array(z.any()) for otherInfo with a type-safe alternative: if the shape is
known, define and use a specific zod schema (e.g., OtherInfoSchema) and use
z.array(OtherInfoSchema); if items are heterogeneous but enumerable, use
z.array(z.union([ ...candidateSchemas ])); if truly unknown but you want to
avoid any, use z.array(z.unknown()) to preserve safety; update the export/type
definitions accordingly so otherInfo has a concrete Zod schema and TypeScript
type instead of any.
| type: z.nativeEnum(SecretType), | ||
| status: z.nativeEnum(SecretStatus), | ||
| expiresAt: z.date().nullable(), | ||
| otherInfo: z.array(z.any()), |
There was a problem hiding this comment.
Replace z.any() with a specific schema.
Using z.any() defeats runtime validation and violates the coding guideline against the any type. Define a proper schema for otherInfo items or use z.unknown() if the shape is truly dynamic.
Apply this diff:
- otherInfo: z.array(z.any()),
+ otherInfo: z.array(z.record(z.string(), z.unknown())),Or define a specific shape if otherInfo has a known structure:
- otherInfo: z.array(z.any()),
+ otherInfo: z.array(z.object({
+ key: z.string(),
+ value: z.string(),
+ })),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| otherInfo: z.array(z.any()), | |
| otherInfo: z.array(z.record(z.string(), z.unknown())), |
🤖 Prompt for AI Agents
In schemas/secrets/metadata/output.ts around line 13, the array uses z.any()
which bypasses runtime validation; replace z.any() with either a concrete Zod
schema that matches the known structure of otherInfo items (e.g.,
z.object({...}) or z.string()/z.number() as appropriate) or, if the shape is
truly unknown, use z.unknown() to avoid allowing any unsafe values; update the
type definitions accordingly so the exported schema and TypeScript types reflect
the new specific schema and run tests/typing to ensure compatibility.
1 participant
Summary by CodeRabbit
New Features
Refactor