feat(core): add complete schema validation to all authentication flows#162
feat(core): add complete schema validation to all authentication flows#162halvaradop wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR refactors the identity schema validation layer by introducing a universal validator adapter and a schema registry/derivation system. Type constraints are narrowed for Valibot and ArkType schemas, test cases are updated to use the new derivation functions, and build dependencies are adjusted. ChangesSchema Validation Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/core/src/@types/config.tsOops! Something went wrong! :( ESLint: 9.39.4 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by packages/core/src/validator/registry.tsOops! Something went wrong! :( ESLint: 9.39.4 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by packages/core/src/shared/assert.tsOops! Something went wrong! :( ESLint: 9.39.4 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by
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: 5
🧹 Nitpick comments (2)
packages/core/tsconfig.json (1)
8-8: ⚡ Quick winFor published packages, keep
skipLibCheckdisabled to catch type declaration errors.Setting
skipLibChecktotrueskips type-checking of.d.tsfiles from dependencies, which can hide incompatible declaration-file problems. For a published core package, TypeScript's recommended practice is to keep this disabled when validating the package's own types. Instead of masking issues, address root causes like dependency conflicts.Proposed change
- "skipLibCheck": true, + "skipLibCheck": false,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/tsconfig.json` at line 8, The tsconfig currently enables "skipLibCheck": true which hides declaration-file type errors; change the compiler option to "skipLibCheck": false (or remove the option) in the project's tsconfig so dependency .d.ts files are type-checked, then run the TypeScript build/emit and fix any revealed declaration or dependency type issues (resolve version mismatches, update types, or add necessary type overrides) until the build passes; look for the "skipLibCheck" key in the tsconfig to locate and update it.packages/core/src/validator/validator.ts (1)
46-50: ⚡ Quick winTypeBox validation provides no error details.
When TypeBox validation fails, the code returns a generic
Error("Validation failed")with no details about what failed. This makes debugging difficult for users. Consider using TypeBox's error reporting utilities to provide more context.💡 Consider using TypeBox's Errors utility for detailed validation errors
if (IsObject(schema)) { const isValid = Check(schema, data) - return isValid - ? { success: true, data: data as T, error: null } - : { success: false, data: null, error: new Error("Validation failed") } + if (isValid) { + return { success: true, data: data as T, error: null } + } + // Import Errors from 'typebox/value' at the top + const errors = Errors(schema, data) + return { success: false, data: null, error: Array.from(errors) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/validator/validator.ts` around lines 46 - 50, The current IsObject(schema) branch returns a generic Error("Validation failed") when Check(schema, data) is false; replace that generic error with a detailed validation error using TypeBox's Errors utility: call Errors(schema, data) (or Errors(CheckSchema, data) as appropriate) to collect the validation error items, format or stringify those items into a concise message, and return that message (or the error array) inside the returned error (e.g., new Error(formattedErrors)) so the function (in packages/core/src/validator/validator.ts around the IsObject/Check logic) returns meaningful validation details instead of the generic message.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/package.json`:
- Line 89: The package manifest was changed to bump the dependency
"@aura-stack/router" in package.json but the pnpm lockfile is out of sync; run
pnpm install at the repo root to regenerate pnpm-lock.yaml (or pnpm install
--lockfile-only if preferred), verify the updated pnpm-lock.yaml includes the
new "@aura-stack/router@^0.7.0" entry, and commit the updated lockfile alongside
the package.json change so CI no longer errors with ERR_PNPM_OUTDATED_LOCKFILE.
In `@packages/core/src/validator/registry.ts`:
- Around line 68-77: The error-handling checks inside parseAsPartial use the
wrong schema variable; update the isZodSchema checks and the AuthValidationError
cause to reference partialSchema (the schema validated by partialValidator)
instead of schema so zod errors are formatted and attached correctly—modify the
conditional calls to isZodSchema(partialSchema), use formatZodError(error) when
partialSchema is Zod, and pass cause: isZodSchema(partialSchema) ? error :
undefined in the AuthValidationError instantiation within parseAsPartial
(references: parseAsPartial, partialValidator, partialSchema, isZodSchema,
formatZodError, AuthValidationError).
- Around line 57-66: The parse function currently only formats validation errors
for Zod (using isZodSchema and formatZodError) which leaves Valibot and ArkType
errors unformatted; update parse (the block that calls validator.validate and
throws AuthValidationError) to detect Valibot and ArkType schemas (e.g., via
isValibotSchema / isArkTypeSchema or by inspecting validator/error shape),
generate human-friendly error details with appropriate formatters (e.g.,
formatValibotError, formatArkTypeError or inline formatting logic), include
those details in the JSON.stringify output instead of an empty object, and set
the AuthValidationError cause to the original error for non-Zod cases as well so
all schema types produce consistent, informative error messages.
- Line 19: The code calls the removed Zod v4 instance method schema.loose() in
the passthrough logic; replace that call with the top-level z.looseObject(...)
usage by passing the object shape (the same schema shape used before) into
z.looseObject to produce a loose object schema. Locate the passthrough branch
that references schema.loose() in registry.ts and change it to call
z.looseObject(shape) (using the original schema shape variable) so it conforms
to Zod v4's API.
In `@packages/core/src/validator/validator.ts`:
- Around line 34-44: The current error detection in validator.ts uses shape
inspection of parsed.summary; instead compute validation failure using ArkType's
API by replacing that logic with const isError = !schema.allows(data) (keep the
existing parsed = schema(data) call so you can still return the parsed error
when isError is true). Update the block guarded by isArkType(schema) to use
schema.allows(data) for the isError check instead of checking for a summary
property, preserving the return branches that use parsed for success/error
cases.
---
Nitpick comments:
In `@packages/core/src/validator/validator.ts`:
- Around line 46-50: The current IsObject(schema) branch returns a generic
Error("Validation failed") when Check(schema, data) is false; replace that
generic error with a detailed validation error using TypeBox's Errors utility:
call Errors(schema, data) (or Errors(CheckSchema, data) as appropriate) to
collect the validation error items, format or stringify those items into a
concise message, and return that message (or the error array) inside the
returned error (e.g., new Error(formattedErrors)) so the function (in
packages/core/src/validator/validator.ts around the IsObject/Check logic)
returns meaningful validation details instead of the generic message.
In `@packages/core/tsconfig.json`:
- Line 8: The tsconfig currently enables "skipLibCheck": true which hides
declaration-file type errors; change the compiler option to "skipLibCheck":
false (or remove the option) in the project's tsconfig so dependency .d.ts files
are type-checked, then run the TypeScript build/emit and fix any revealed
declaration or dependency type issues (resolve version mismatches, update types,
or add necessary type overrides) until the build passes; look for the
"skipLibCheck" key in the tsconfig to locate and update it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e04da9e7-5dc5-4d7e-ab8a-3ec168e873d8
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
package.jsonpackages/core/package.jsonpackages/core/src/@types/config.tspackages/core/src/schema-registry.tspackages/core/src/shared/assert.tspackages/core/src/validator/registry.tspackages/core/src/validator/validator.tspackages/core/test/actions/updateSession/updateSession.test.tspackages/core/test/identity.test.tspackages/core/tsconfig.json
💤 Files with no reviewable changes (1)
- package.json
| "dependencies": { | ||
| "@aura-stack/jose": "workspace:*", | ||
| "@aura-stack/router": "^0.6.0", | ||
| "@aura-stack/router": "^0.7.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification of manifest/lockfile drift related to current CI failure.
set -euo pipefail
echo "== package specifiers mentioning typebox/router =="
rg -n --glob '**/package.json' '"(`@aura-stack/router`|typebox)"\s*:'
echo
echo "== lockfile entries mentioning typebox/router =="
rg -n '(`@aura-stack/router`|typebox)' pnpm-lock.yaml
echo
echo "== CI-sensitive lockfile drift indicators =="
rg -n 'specifiers:|dependencies:' pnpm-lock.yaml | head -n 40Repository: aura-stack-ts/auth
Length of output: 2518
Regenerate pnpm-lock.yaml after dependency version changes.
Line 89 updates @aura-stack/router to ^0.7.0, but the lockfile must be synchronized. CI currently fails with ERR_PNPM_OUTDATED_LOCKFILE because frozen-lockfile installations cannot proceed until the manifest and lockfile are in sync. Run pnpm install to regenerate the lockfile and resolve this blocker.
🧰 Tools
🪛 GitHub Actions: CI / 0_Node.js.txt
[error] Lockfile specifiers do not match package.json specifiers during pnpm install --frozen-lockfile (ERR_PNPM_OUTDATED_LOCKFILE).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/package.json` at line 89, The package manifest was changed to
bump the dependency "@aura-stack/router" in package.json but the pnpm lockfile
is out of sync; run pnpm install at the repo root to regenerate pnpm-lock.yaml
(or pnpm install --lockfile-only if preferred), verify the updated
pnpm-lock.yaml includes the new "@aura-stack/router@^0.7.0" entry, and commit
the updated lockfile alongside the package.json change so CI no longer errors
with ERR_PNPM_OUTDATED_LOCKFILE.
1 participant
Description
It's time to go to U
Summary by CodeRabbit
Bug Fixes
Chores