Repository navigation
feat(security): Phase 2 opaque token architecture - #187
Conversation
Complete migration from JWT service tokens to opaque session tokens: - Add EnforcedAuthContext class for immutable auth context from validated sessions - Update processor middleware to use sessionService.validateSession() - Migrate validated-service-token.ts to use sessionService.createSession() - Add driveId column to sessions schema for processor validation - Update all processor API endpoints to use req.auth (EnforcedAuthContext) - Remove legacy JWT service-auth.ts and related code Breaking changes: - Processor now requires opaque tokens (ps_svc_*) instead of JWT tokens - req.serviceAuth replaced with req.auth (EnforcedAuthContext) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughReplaces JWT service-token auth with a session-based model: middleware validates sessions via Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ProcessorAPI
participant AuthMiddleware
participant SessionService
participant Database
Client->>ProcessorAPI: HTTP request with session token
ProcessorAPI->>AuthMiddleware: authenticateService(req)
AuthMiddleware->>SessionService: validateSession(token)
SessionService->>Database: SELECT session by tokenHash
Database-->>SessionService: session row (userId, scopes, resourceBinding, driveId)
SessionService-->>AuthMiddleware: SessionClaims
AuthMiddleware->>AuthMiddleware: EnforcedAuthContext.fromSession(claims)
AuthMiddleware-->>ProcessorAPI: attach req.auth
ProcessorAPI->>ProcessorAPI: authorize via req.auth.hasScope / isBoundToResource
ProcessorAPI-->>Client: 200 or 401/403
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 1
🤖 Fix all issues with AI agents
In @apps/processor/src/api/ingest.ts:
- Around line 13-20: The two auth checks in ingest.ts are returning inconsistent
messages: change the first check that currently does res.status(401).json({
error: 'Authentication required' }) so it uses the same string as the second
check (res.status(401).json({ error: 'Service authentication required' }));
update the error literal where req.auth is falsy (and ensure you leave the
subsequent userId check and returns intact) so both checks use "Service
authentication required" for consistent API responses.
🧹 Nitpick comments (4)
packages/lib/src/services/__tests__/validated-service-token.test.ts (1)
505-508: Minor: Redundant mock clearing.
vi.clearAllMocks()already clears all mocks includingmockFindFirst, making the explicitmockFindFirst.mockClear()call redundant.♻️ Simplify beforeEach
beforeEach(() => { vi.clearAllMocks(); - mockFindFirst.mockClear(); });apps/processor/src/api/optimize.ts (1)
136-136: Consider replacinganytype with a proper interface.Per coding guidelines, avoid
anytypes. While this line wasn't changed in this PR, consider defining a type for the results object:🔧 Suggested type definition
interface BatchPresetResult { cached: boolean; url?: string; jobId?: string; status: 'completed' | 'queued'; error?: string; } const results: Record<string, BatchPresetResult> = {};packages/lib/src/permissions/__tests__/enforced-context.test.ts (1)
18-25: Private constructor test doesn't verify runtime behavior.The test defines
attemptConstructionbut never calls it. While TypeScript enforcement is the primary goal, the comment on line 23 suggests runtime behavior should be tested. Consider either:
- Actually invoking the function to document what happens at runtime, or
- Removing the runtime comment since TS compilation is the intended enforcement
Option 1: Invoke and document runtime behavior
it('cannot be constructed directly (TypeScript enforced)', () => { // TypeScript prevents direct construction via private constructor - // This test documents the design intent - TS compilation enforces it // @ts-expect-error - Constructor is private and inaccessible const attemptConstruction = () => new EnforcedAuthContext('u', 'user', [], undefined); - // At runtime JS allows it, but TS prevents compilation - expect(typeof attemptConstruction).toBe('function'); + // At runtime JS allows it since private is a TS-only concept + const context = attemptConstruction(); + expect(context).toBeDefined(); });packages/lib/src/auth/session-service.ts (1)
101-105: Consider logging lastUsedAt update failures for observability.The non-blocking update pattern is appropriate for performance, but silently swallowing errors could mask persistent database issues.
🔧 Optional: Add debug-level logging
// Update last used (non-blocking) db.update(sessions) .set({ lastUsedAt: new Date() }) .where(eq(sessions.tokenHash, tokenHash)) - .catch(() => {}); + .catch((err) => { + // Log at debug level - not critical but useful for monitoring + if (process.env.NODE_ENV !== 'production') { + console.debug('Failed to update session lastUsedAt:', err); + } + });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
apps/processor/src/api/avatar.tsapps/processor/src/api/ingest.tsapps/processor/src/api/optimize.tsapps/processor/src/api/serve.tsapps/processor/src/api/upload.tsapps/processor/src/middleware/auth.tsapps/processor/src/middleware/rate-limit.tsapps/processor/src/types/express.d.tspackages/db/drizzle/0037_slimy_bishop.sqlpackages/db/drizzle/meta/0037_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/sessions.tspackages/lib/src/auth/auth-utils.tspackages/lib/src/auth/session-service.tspackages/lib/src/index.tspackages/lib/src/permissions/__tests__/enforced-context.test.tspackages/lib/src/permissions/enforced-context.tspackages/lib/src/permissions/index.tspackages/lib/src/services/__tests__/service-auth.test.tspackages/lib/src/services/__tests__/validated-service-token.test.tspackages/lib/src/services/service-auth.tspackages/lib/src/services/validated-service-token.ts
💤 Files with no reviewable changes (3)
- packages/lib/src/auth/auth-utils.ts
- packages/lib/src/services/tests/service-auth.test.ts
- packages/lib/src/services/service-auth.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Never useanytypes - always use proper TypeScript types
Use camelCase for variable and function names
Use UPPER_SNAKE_CASE for constants
Use PascalCase for type and enum names
Use kebab-case for filenames, except React hooks (camelCase withuseprefix), Zustand stores (camelCase withuseprefix), and React components (PascalCase)
Lint with Next/ESLint as configured inapps/web/eslint.config.mjs
Message content should always use the message parts structure with{ parts: [{ type: 'text', text: '...' }] }
Use centralized permission functions from@pagespace/lib/permissions(e.g.,getUserAccessLevel,canUserEditPage) instead of implementing permission logic locally
Always use Drizzle client from@pagespace/dbpackage for database access
Use ESM modules throughout the codebase
**/*.{ts,tsx}: Never useanytypes - always use proper TypeScript types
Write code that is explicit over implicit and self-documenting
Files:
apps/processor/src/api/serve.tsapps/processor/src/types/express.d.tspackages/lib/src/permissions/enforced-context.tspackages/lib/src/permissions/index.tsapps/processor/src/api/ingest.tsapps/processor/src/api/optimize.tsapps/processor/src/middleware/rate-limit.tspackages/lib/src/index.tspackages/lib/src/auth/session-service.tspackages/db/src/schema/sessions.tsapps/processor/src/api/avatar.tspackages/lib/src/permissions/__tests__/enforced-context.test.tsapps/processor/src/api/upload.tspackages/lib/src/services/validated-service-token.tspackages/lib/src/services/__tests__/validated-service-token.test.tsapps/processor/src/middleware/auth.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: React hook files should use camelCase matching the exported hook name (e.g.,useAuth.ts)
Zustand store files should use camelCase withuseprefix (e.g.,useAuthStore.ts)
Files:
apps/processor/src/api/serve.tsapps/processor/src/types/express.d.tspackages/lib/src/permissions/enforced-context.tspackages/lib/src/permissions/index.tsapps/processor/src/api/ingest.tsapps/processor/src/api/optimize.tsapps/processor/src/middleware/rate-limit.tspackages/lib/src/index.tspackages/lib/src/auth/session-service.tspackages/db/src/schema/sessions.tsapps/processor/src/api/avatar.tspackages/lib/src/permissions/__tests__/enforced-context.test.tsapps/processor/src/api/upload.tspackages/lib/src/services/validated-service-token.tspackages/lib/src/services/__tests__/validated-service-token.test.tsapps/processor/src/middleware/auth.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
apps/processor/src/api/serve.tsapps/processor/src/types/express.d.tspackages/lib/src/permissions/enforced-context.tspackages/lib/src/permissions/index.tsapps/processor/src/api/ingest.tsapps/processor/src/api/optimize.tsapps/processor/src/middleware/rate-limit.tspackages/lib/src/index.tspackages/db/drizzle/meta/_journal.jsonpackages/lib/src/auth/session-service.tspackages/db/src/schema/sessions.tsapps/processor/src/api/avatar.tspackages/lib/src/permissions/__tests__/enforced-context.test.tsapps/processor/src/api/upload.tspackages/lib/src/services/validated-service-token.tspackages/lib/src/services/__tests__/validated-service-token.test.tsapps/processor/src/middleware/auth.ts
packages/db/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for database queries with PostgreSQL
Files:
packages/db/src/schema/sessions.ts
packages/db/src/schema/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Database schema changes must be made in
packages/db/src/schema/and thenpnpm db:generatemust be run to create migrations
Files:
packages/db/src/schema/sessions.ts
**/*auth*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*auth*.{ts,tsx}: Use custom JWT authentication with jose library for user management
Use bcryptjs for password hashing
Files:
apps/processor/src/middleware/auth.ts
🧠 Learnings (8)
📚 Learning: 2025-12-22T20:04:40.910Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.910Z
Learning: Applies to **/*.{ts,tsx} : Use centralized permission functions from `pagespace/lib/permissions` (e.g., `getUserAccessLevel`, `canUserEditPage`) instead of implementing permission logic locally
Applied to files:
packages/lib/src/permissions/index.tsapps/processor/src/api/ingest.tsapps/processor/src/api/upload.ts
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to app/**/*.{ts,tsx} : Import and use `getUserAccessLevel()` and `canUserEditPage()` from `pagespace/lib/permissions` for centralized permission logic
Applied to files:
packages/lib/src/permissions/index.ts
📚 Learning: 2025-12-23T18:49:41.966Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.966Z
Learning: Applies to apps/web/src/app/**/route.{ts,tsx} : For permission logic, use centralized functions from `pagespace/lib/permissions`: `getUserAccessLevel()`, `canUserEditPage()`
Applied to files:
apps/processor/src/api/ingest.ts
📚 Learning: 2025-12-22T20:04:40.910Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.910Z
Learning: Applies to packages/db/src/schema.ts : Update database schema in `packages/db/src/schema.ts` and generate migrations with `pnpm db:generate`
Applied to files:
packages/db/src/schema/sessions.ts
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to packages/db/src/schema.ts : Maintain the Drizzle ORM database schema in `packages/db/src/schema.ts` as the single entry point for schema definitions
Applied to files:
packages/db/src/schema/sessions.ts
📚 Learning: 2025-12-22T20:04:40.910Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.910Z
Learning: Applies to packages/db/src/schema.ts : Database schema entry point is at `packages/db/src/schema.ts`; migrations emit to `packages/db/drizzle/`
Applied to files:
packages/db/src/schema/sessions.ts
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to packages/db/{src/schema.ts,drizzle/**/*.ts} : Database schema must be defined in `packages/db/src/schema.ts` and migrations must be emitted to `packages/db/drizzle/`
Applied to files:
packages/db/src/schema/sessions.ts
📚 Learning: 2025-12-23T18:49:41.966Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.966Z
Learning: Applies to packages/db/src/schema/**/*.{ts,tsx} : Database schema changes must be made in `packages/db/src/schema/` and then `pnpm db:generate` must be run to create migrations
Applied to files:
packages/db/src/schema/sessions.ts
🧬 Code graph analysis (5)
packages/lib/src/permissions/enforced-context.ts (3)
packages/lib/src/index.ts (3)
ResourceBinding(53-53)EnforcedAuthContext(53-53)SessionClaims(52-52)apps/processor/src/middleware/auth.ts (1)
EnforcedAuthContext(182-182)packages/lib/src/auth/session-service.ts (1)
SessionClaims(8-18)
apps/processor/src/api/avatar.ts (1)
apps/processor/src/middleware/auth.ts (1)
hasAuthScope(167-172)
packages/lib/src/permissions/__tests__/enforced-context.test.ts (3)
packages/lib/src/auth/session-service.ts (1)
SessionClaims(8-18)packages/lib/src/index.ts (2)
SessionClaims(52-52)EnforcedAuthContext(53-53)packages/lib/src/permissions/enforced-context.ts (1)
EnforcedAuthContext(13-84)
apps/processor/src/api/upload.ts (2)
apps/processor/src/middleware/auth.ts (1)
hasAuthScope(167-172)apps/processor/src/server.ts (1)
contentStore(25-25)
apps/processor/src/middleware/auth.ts (3)
packages/lib/src/auth/session-service.ts (1)
sessionService(144-144)packages/lib/src/index.ts (2)
sessionService(52-52)EnforcedAuthContext(53-53)packages/lib/src/permissions/enforced-context.ts (1)
EnforcedAuthContext(13-84)
⏰ 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). (1)
- GitHub Check: Unit Tests
🔇 Additional comments (42)
packages/lib/src/services/__tests__/validated-service-token.test.ts (7)
32-38: LGTM! Session service mock correctly configured.The mock setup properly returns an opaque token with the expected
ps_svc_prefix, aligning with the Phase 2 architecture migration.
61-86: LGTM! Scope filtering tests correctly verify session creation.The test properly validates that only authorized scopes are passed to
sessionService.createSession.
332-357: LGTM! driveId claim propagation correctly tested.The test ensures
driveIdis properly passed through to session creation, which is critical for processor validation per the PR objectives.
387-448: LGTM! Convenience functions correctly set resource context.Tests properly verify that each helper passes the correct
resourceId,resourceType, and type-specific identifiers to session creation.
450-475: LGTM! Expiration conversion correctly validated.The test properly verifies that the string-based
expiresIn('10m') is converted to numericexpiresInMs(600000) for the session service API.
783-815: LGTM! Error bubbling correctly distinguishes error types.The test properly validates that infrastructure errors from
sessionService.createSessionbubble up as regularErrorinstances rather thanPermissionDeniedError, enabling callers to differentiate between authorization failures and service failures.
818-841: LGTM! Type guard tests are thorough.Comprehensive coverage of the
isPermissionDeniedErrortype guard including edge cases for non-Error values and duck-typing scenarios with incorrect error codes.packages/db/drizzle/0037_slimy_bishop.sql (1)
1-1: LGTM!The migration correctly adds the nullable
drive_idcolumn to the sessions table, aligning with the schema definition insessions.ts. The nullable column is appropriate sincedriveIdis optional for resource binding.packages/db/drizzle/meta/_journal.json (1)
263-270: LGTM!The journal entry is correctly formatted and references the
0037_slimy_bishopmigration. The entry follows the established pattern with consistent version and breakpoints settings.apps/processor/src/api/serve.ts (3)
15-28: LGTM!The authentication migration from
req.serviceAuthtoreq.authis correctly implemented. The auth check pattern with null check followed by userId validation is appropriate.
130-143: LGTM!Consistent auth migration pattern applied to the cached file serving route.
206-219: LGTM!Consistent auth migration pattern applied to the metadata route.
apps/processor/src/api/avatar.ts (5)
5-5: LGTM!Import correctly updated from
hasServiceScopetohasAuthScopeto align with the newEnforcedAuthContext-based authentication system.
45-48: LGTM!Authentication source correctly migrated to
req.authfor the upload endpoint.
64-70: LGTM!The authorization check correctly uses
hasAuthScopewith theEnforcedAuthContext. The logic appropriately allows users to modify their own avatar or requires theavatars:write:anyscope for modifying another user's avatar.
130-133: LGTM!Authentication source correctly migrated to
req.authfor the delete endpoint.
144-150: LGTM!Authorization check correctly migrated to use
hasAuthScope, maintaining the same security logic for avatar deletion.packages/db/src/schema/sessions.ts (1)
20-23: ThedriveIdcolumn is correctly added as a nullable text field for resource binding.The addition is sound—driveId joins resourceType and resourceId as metadata for scoping sessions to specific drives. However, the suggestion about future indexing is not applicable at this time: session queries only filter by tokenHash, userId, expiresAt, and revokedAt. There is no evidence of sessions being queried by driveId in the codebase.
apps/processor/src/api/optimize.ts (1)
14-17: LGTM on auth migration.The switch from
req.serviceAuthtoreq.authis consistent across all three routes (/,/batch,/prepare-for-ai). The authentication checks and userId extraction logic remain intact.Also applies to: 110-113, 193-196
apps/processor/src/middleware/rate-limit.ts (1)
12-26: LGTM on rate-limit auth migration.The
getBucketKeyfunction correctly migrates toreq.authwhile preserving the rate-limiting semantics:
- User-based limiting via
auth.userIdwhen authenticated- IP-based fallback for unauthenticated requests
The comment on line 14 clearly documents the intent.
packages/lib/src/permissions/__tests__/enforced-context.test.ts (2)
49-88: Good scope-checking test coverage.The
hasScopetests comprehensively cover:
- Exact scope matching
- Global wildcard (
*)- Namespace wildcards (
files:*)- Empty scope arrays
This aligns well with the
hasScopeimplementation logic.
108-141: Resource binding tests cover key authorization scenarios.Good coverage of
isBoundToResource:
- Unrestricted access when no binding exists (returns
true)- Exact type+id match
- Type mismatch rejection
- ID mismatch rejection
packages/lib/src/permissions/index.ts (1)
21-23: LGTM on public API export.The re-export of
enforced-contextmakesEnforcedAuthContextandResourceBindingavailable through the centralized permissions module, consistent with the existing export patterns and the project's approach to centralized permission logic.apps/processor/src/types/express.d.ts (1)
1-11: LGTM!Clean type declaration update. The Express.Request augmentation correctly exposes
auth?: EnforcedAuthContext, aligning with the new session-based authentication flow across the processor module.packages/lib/src/auth/session-service.ts (2)
17-17: LGTM - driveId addition to SessionClaims.The optional
driveIdfield is correctly added to support processor validation as mentioned in the PR objectives.
107-117: LGTM - validateSession return structure.The return object correctly maps all session fields including the new
driveId, with consistent nullish coalescing for optional properties.packages/lib/src/permissions/enforced-context.ts (3)
1-50: LGTM - EnforcedAuthContext design.Excellent security pattern with private constructor ensuring contexts can only be created from validated sessions. The
Object.freeze(this)provides immutability guarantees, andReadonlySetfor scopes prevents modification.
52-70: LGTM - Scope matching logic.The three-tier scope checking (global wildcard → exact match → namespace wildcard) is well-structured and handles the defined
ServiceScopepatterns correctly.
76-83: LGTM - Resource binding validation.The semantic of "no binding means unrestricted" is appropriate for service tokens that may operate across resources. The strict matching when a binding exists correctly enforces resource-scoped access.
apps/processor/src/api/upload.ts (4)
85-90: LGTM - Global auth guard middleware.Clean early return pattern ensuring all upload routes require authentication before proceeding.
104-136: LGTM - Single upload authorization flow.Comprehensive validation chain: resource binding → driveId → pageId → user authorization. The scope check for
files:write:anycorrectly guards cross-user uploads.
268-302: LGTM - Multiple upload authorization flow.Correctly mirrors single upload validation with appropriate flexibility for batch uploads where per-file page binding may be optional.
159-164: LGTM - Consistent metadata handling.The metadata structure using
auth.userIdastenantIdand'processor'as service is consistent across all upload paths (dedupe, save, single, and multiple).packages/lib/src/services/validated-service-token.ts (4)
18-32: LGTM - ServiceScope type definition.Well-defined union type covering all permission scopes with clear organization (read, write, delete, admin).
34-51: LGTM - Duration parsing helper.Clean implementation with sensible default fallback. The supported units (s/m/h/d) cover typical token expiration scenarios.
186-200: LGTM - Token creation via sessionService.Clean migration from JWT to session-based tokens. The
createdByService: 'web'attribution provides audit trail clarity.
448-462: LGTM - Upload token creation.Correctly creates page-scoped service sessions with
driveIdfor processor validation. TheUPLOAD_SCOPESconstant (['files:write']) provides appropriate minimal permissions.apps/processor/src/middleware/auth.ts (4)
1-3: LGTM - Import updates.Clean separation of concerns with
sessionServicefrom auth module andEnforcedAuthContextfrom permissions module.
89-136: LGTM - Authentication middleware.Clean flow: token extraction → session validation → context construction → optional scope inference. Generic error messages on failure prevent information leakage.
138-165: LGTM - Scope requirement middleware.Well-structured middleware factory with proper 401/403 distinction. The conditional logging respects test environment configuration.
167-182: LGTM - Helper utilities and re-export.
hasAuthScopeprovides null-safe scope checking for use in route handlers.requireUserContextoffers a clean way to extract authenticated user ID. Re-exportingEnforcedAuthContextmaintains backward compatibility for imports from this module.packages/lib/src/index.ts (1)
52-64: LGTM - Public API surface updates correctly implement new auth architecture.The exports properly expose the new session-based authentication primitives (
sessionService,EnforcedAuthContext) and addServiceScopetype visibility, aligning with the PR's migration from JWT to opaque tokens. The legacyservice-authmodule exports have been fully removed with no remaining references in the codebase, confirming the migration is complete.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f89746e1e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| switch (unit) { | ||
| case 's': return value * 1000; | ||
| case 'm': return value * 60 * 1000; | ||
| case 'h': return value * 60 * 60 * 1000; | ||
| case 'd': return value * 24 * 60 * 60 * 1000; |
There was a problem hiding this comment.
The new durationToMs just converts the requested duration to milliseconds and then createSession uses it directly for expiresAt, so any caller that passes expiresIn: '0m' will mint an already-expired token (upload/reprocess calls will 401), and any caller that passes a very large duration (e.g. '365d') will create near-permanent service tokens. The previous JWT path capped/validated service token lifetimes; without similar bounds here, a single malformed or user-influenced duration value can break the processor flow or weaken short‑lived token guarantees.
Useful? React with 👍 / 👎.
Change first auth check from "Authentication required" to "Service authentication required" for consistent API responses. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Restore security bounds that existed in old service-auth.ts: - Reject zero/negative values (return 5m default) - Enforce 10s minimum (prevent near-instant expiry) - Cap at 30 days maximum (prevent near-permanent tokens) - Log warnings for all invalid/capped durations Prevents callers from creating already-expired tokens (0m) or near-permanent service tokens (365d+). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ation - Add validation to reject non-service session types in processor auth middleware (service-to-service only) - Fix sessions-schema test FK violation by improving test isolation: - Use cuid for email uniqueness instead of Date.now() - Add defensive cleanup of orphaned sessions in beforeEach - Delete sessions before users in afterEach - Wrap afterEach in try-catch for cascade delete test - Reset testUserId at end of afterEach Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/lib/src/services/validated-service-token.ts (1)
172-236:additionalClaimsoption is accepted but not passed tosessionService.createSession.The
additionalClaimsparameter is destructured from options (line 181) but is never used in thecreateSessioncall. If this is intentional (session service doesn't support arbitrary claims), consider removing the option fromValidatedTokenOptionsto avoid confusion.🔧 If additionalClaims is no longer supported, remove it from the interface
export interface ValidatedTokenOptions { /** User requesting the token */ userId: string; /** Type of resource being accessed */ resourceType: ResourceType; /** ID of the resource (pageId, driveId, or userId for 'user' type) */ resourceId: string; /** Scopes being requested */ requestedScopes: ServiceScope[]; /** Drive ID for drive-scoped tokens (required for processor validation) */ driveId?: string; /** Token expiration (jose duration string, default '5m') */ expiresIn?: string; - /** Additional context for the token */ - additionalClaims?: Record<string, unknown>; }
🧹 Nitpick comments (3)
packages/db/src/__tests__/sessions-schema.test.ts (1)
16-24: Defensive cleanup approach is solid but can be simplified.The individual delete statements for each known tokenHash work, but they could be consolidated into a single query using
inArrayfor better maintainability.♻️ Optional consolidation
+import { inArray } from 'drizzle-orm'; + +const TEST_TOKEN_HASHES = ['abc123hash', 'unique-hash', 'cascade-test', 'hash-1', 'hash-2']; + beforeEach(async () => { const uniqueId = createId(); // Clean any orphaned test sessions (defensive cleanup) - await db.delete(sessions).where(eq(sessions.tokenHash, 'abc123hash')); - await db.delete(sessions).where(eq(sessions.tokenHash, 'unique-hash')); - await db.delete(sessions).where(eq(sessions.tokenHash, 'cascade-test')); - await db.delete(sessions).where(eq(sessions.tokenHash, 'hash-1')); - await db.delete(sessions).where(eq(sessions.tokenHash, 'hash-2')); + await db.delete(sessions).where(inArray(sessions.tokenHash, TEST_TOKEN_HASHES));apps/processor/src/middleware/auth.ts (2)
137-141: Consider logging the actual error for debugging, not just the message.The current implementation logs only the error message, which may lose stack trace information useful for debugging authentication issues in production.
♻️ Include full error in logs for debugging
} catch (error) { const message = error instanceof Error ? error.message : 'Invalid token'; - console.error('Authentication failed:', message); + console.error('Authentication failed:', error); respondUnauthorized(res, 'Invalid token'); }
180-186:requireUserContextis a misleading name since it doesn't "require" anything.The function returns
nullwhen auth is missing rather than throwing or enforcing. Consider renaming togetUserIdorextractUserIdto better reflect its behavior.♻️ Consider renaming for clarity
-export function requireUserContext(req: Request): string | null { +export function getUserIdFromAuth(req: Request): string | null { const auth = req.auth; if (!auth) { return null; } return auth.userId; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/processor/src/api/ingest.tsapps/processor/src/middleware/auth.tspackages/db/src/__tests__/sessions-schema.test.tspackages/lib/src/services/__tests__/validated-service-token.test.tspackages/lib/src/services/validated-service-token.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Never useanytypes - always use proper TypeScript types
Use camelCase for variable and function names
Use UPPER_SNAKE_CASE for constants
Use PascalCase for type and enum names
Use kebab-case for filenames, except React hooks (camelCase withuseprefix), Zustand stores (camelCase withuseprefix), and React components (PascalCase)
Lint with Next/ESLint as configured inapps/web/eslint.config.mjs
Message content should always use the message parts structure with{ parts: [{ type: 'text', text: '...' }] }
Use centralized permission functions from@pagespace/lib/permissions(e.g.,getUserAccessLevel,canUserEditPage) instead of implementing permission logic locally
Always use Drizzle client from@pagespace/dbpackage for database access
Use ESM modules throughout the codebase
**/*.{ts,tsx}: Never useanytypes - always use proper TypeScript types
Write code that is explicit over implicit and self-documenting
Files:
packages/db/src/__tests__/sessions-schema.test.tsapps/processor/src/middleware/auth.tspackages/lib/src/services/validated-service-token.tspackages/lib/src/services/__tests__/validated-service-token.test.tsapps/processor/src/api/ingest.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: React hook files should use camelCase matching the exported hook name (e.g.,useAuth.ts)
Zustand store files should use camelCase withuseprefix (e.g.,useAuthStore.ts)
Files:
packages/db/src/__tests__/sessions-schema.test.tsapps/processor/src/middleware/auth.tspackages/lib/src/services/validated-service-token.tspackages/lib/src/services/__tests__/validated-service-token.test.tsapps/processor/src/api/ingest.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
packages/db/src/__tests__/sessions-schema.test.tsapps/processor/src/middleware/auth.tspackages/lib/src/services/validated-service-token.tspackages/lib/src/services/__tests__/validated-service-token.test.tsapps/processor/src/api/ingest.ts
packages/db/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for database queries with PostgreSQL
Files:
packages/db/src/__tests__/sessions-schema.test.ts
**/*auth*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*auth*.{ts,tsx}: Use custom JWT authentication with jose library for user management
Use bcryptjs for password hashing
Files:
apps/processor/src/middleware/auth.ts
🧬 Code graph analysis (4)
packages/db/src/__tests__/sessions-schema.test.ts (3)
packages/db/src/index.ts (4)
db(20-20)sessions(45-45)eq(8-8)users(27-27)packages/db/src/schema/sessions.ts (1)
sessions(6-42)packages/db/src/schema/auth.ts (1)
users(10-33)
apps/processor/src/middleware/auth.ts (3)
packages/lib/src/index.ts (2)
sessionService(52-52)EnforcedAuthContext(53-53)packages/lib/src/auth/session-service.ts (1)
sessionService(144-144)packages/lib/src/permissions/enforced-context.ts (1)
EnforcedAuthContext(13-84)
packages/lib/src/services/validated-service-token.ts (3)
packages/lib/src/index.ts (2)
ServiceScope(63-63)sessionService(52-52)packages/lib/src/logging/logger-config.ts (1)
loggers(8-18)packages/lib/src/auth/session-service.ts (1)
sessionService(144-144)
packages/lib/src/services/__tests__/validated-service-token.test.ts (3)
packages/lib/src/services/validated-service-token.ts (1)
createValidatedServiceToken(172-236)packages/lib/src/index.ts (1)
createValidatedServiceToken(55-55)packages/lib/src/logging/logger-config.ts (1)
loggers(8-18)
⏰ 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). (1)
- GitHub Check: Unit Tests
🔇 Additional comments (12)
packages/db/src/__tests__/sessions-schema.test.ts (2)
26-36: LGTM!Using
createId()for bothidand email uniqueness is a solid approach for test isolation. This prevents collisions across parallel test runs.
39-50: LGTM!The improved teardown handles the cascade delete test case gracefully with try-catch, and resetting
testUserIdto empty string prevents stale state in subsequent tests.packages/lib/src/services/validated-service-token.ts (3)
18-32: LGTM!Well-defined union type for service scopes with clear permission granularity. The scope naming follows a consistent
resource:actionpattern.
48-86: Thorough bounds validation with good defensive logging.The implementation correctly handles invalid formats, zero/negative values, minimum enforcement, and maximum capping. The regex pattern
^(\d+)([smhd])$is appropriate for the expected formats.One minor observation: the
defaultcase on line 70 is unreachable since the regex only allowss|m|h|d, but it serves as a defensive fallback which is fine.
483-492: LGTM!The upload token creation correctly passes all required session parameters including
driveId,resourceType, andresourceId. ThecreatedByService: 'web'provides good audit context.packages/lib/src/services/__tests__/validated-service-token.test.ts (3)
32-38: LGTM!The session service mock is correctly set up with
mockCreateSessionreturning an opaque token format (ps_svc_mock-session-token), and the mock is properly wired throughvi.mock.
504-617: Excellent test coverage for duration bounds validation.The tests thoroughly cover:
- Zero duration → defaults to 5m
- Invalid format → defaults to 5m
- Excessive duration (365d) → caps at 30d
- Below minimum (1s) → floors to 10s
- Valid duration (5m) → no warnings
This matches the implementation in
durationToMs()perfectly.
873-896: LGTM!The default expiration tests correctly verify that upload tokens default to 10m (600000ms) while custom expirations (15m → 900000ms) are properly converted.
apps/processor/src/middleware/auth.ts (3)
2-3: LGTM!Clean imports from the centralized auth and permissions packages, aligning with the project's module organization.
103-119: Solid session validation flow with proper type enforcement.The middleware correctly:
- Validates the session token via
sessionService.validateSession- Rejects non-service session types (line 112) - important for service-to-service isolation
- Builds an immutable
EnforcedAuthContextfrom validated claimsThis enforces that the processor only accepts opaque service tokens (
ps_svc_*), as intended by the PR.
173-178: LGTM!Simple, well-typed helper that safely delegates to
EnforcedAuthContext.hasScope(). The undefined check is necessary sincereq.authis optional.apps/processor/src/api/ingest.ts (1)
13-20: LGTM!The authentication migration from
req.serviceAuthtoreq.authis correctly implemented. The dual check (auth exists + userId exists) provides good defense-in-depth, thoughEnforcedAuthContextshould always have auserIdsince it's a required field infromSession.
- Remove destructive `db.delete(users)` that deleted ALL users - Add targeted afterEach cleanup for test-specific users only - Wrap cleanup in try-catch for cascade delete edge cases - Fixes FK violation caused by cross-package database interference Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…eview cleanup Security fix: - Add createSignedBroadcastHeaders to notification broadcasts (was rejected with 401) Code review cleanup: - Remove unused additionalClaims from ValidatedTokenOptions - Simplify test cleanup using inArray instead of multiple deletes - Log full error object in auth middleware for better debugging - Rename requireUserContext to getUserId (reflects getter behavior) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/lib/src/notifications/notifications.ts (1)
20-27: Consider logging non-2xx responses for debugging.The broadcast is fire-and-forget, which is appropriate. However, silently ignoring non-2xx responses (e.g., 401, 500) can hide issues. Adding a status check would help catch regressions after this HMAC fix.
♻️ Optional: Log failed broadcast responses
- await fetch(`${realtimeUrl}/api/broadcast`, { + const response = await fetch(`${realtimeUrl}/api/broadcast`, { method: 'POST', headers: createSignedBroadcastHeaders(requestBody), body: requestBody, }); + if (!response.ok) { + console.warn(`Broadcast failed with status ${response.status}`); + } } catch (error) { console.error('Failed to broadcast notification:', error); }apps/processor/src/middleware/auth.ts (1)
12-12: Avoidanytype cast.Express
Requestextends Node'sIncomingMessage, which already has aurl?: stringproperty. The cast is unnecessary and violates the coding guideline to never useanytypes.♻️ Suggested fix
- (req as any).url, + req.url,packages/lib/src/services/validated-service-token.ts (1)
150-156: Outdated comment: tokens are now opaque, not JWT.The comment on line 152 still says "The signed JWT token" but after this migration, tokens are opaque session tokens (
ps_svc_*). Consider updating the comment for accuracy.📝 Suggested comment update
/** * Result of a validated token creation */ export interface ValidatedTokenResult { - /** The signed JWT token */ + /** The opaque session token */ token: string; /** Scopes that were actually granted (may be subset of requested) */ grantedScopes: ServiceScope[]; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/processor/src/middleware/auth.tspackages/db/src/__tests__/sessions-schema.test.tspackages/lib/src/notifications/notifications.tspackages/lib/src/services/validated-service-token.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/db/src/tests/sessions-schema.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Never useanytypes - always use proper TypeScript types
Use camelCase for variable and function names
Use UPPER_SNAKE_CASE for constants
Use PascalCase for type and enum names
Use kebab-case for filenames, except React hooks (camelCase withuseprefix), Zustand stores (camelCase withuseprefix), and React components (PascalCase)
Lint with Next/ESLint as configured inapps/web/eslint.config.mjs
Message content should always use the message parts structure with{ parts: [{ type: 'text', text: '...' }] }
Use centralized permission functions from@pagespace/lib/permissions(e.g.,getUserAccessLevel,canUserEditPage) instead of implementing permission logic locally
Always use Drizzle client from@pagespace/dbpackage for database access
Use ESM modules throughout the codebase
**/*.{ts,tsx}: Never useanytypes - always use proper TypeScript types
Write code that is explicit over implicit and self-documenting
Files:
packages/lib/src/notifications/notifications.tsapps/processor/src/middleware/auth.tspackages/lib/src/services/validated-service-token.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: React hook files should use camelCase matching the exported hook name (e.g.,useAuth.ts)
Zustand store files should use camelCase withuseprefix (e.g.,useAuthStore.ts)
Files:
packages/lib/src/notifications/notifications.tsapps/processor/src/middleware/auth.tspackages/lib/src/services/validated-service-token.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
packages/lib/src/notifications/notifications.tsapps/processor/src/middleware/auth.tspackages/lib/src/services/validated-service-token.ts
**/*auth*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*auth*.{ts,tsx}: Use custom JWT authentication with jose library for user management
Use bcryptjs for password hashing
Files:
apps/processor/src/middleware/auth.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.910Z
Learning: Applies to **/*auth*.{ts,tsx} : Use custom JWT authentication with jose library for user management
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to **/*.{ts,tsx} : Always use the Drizzle client and database exports from `pagespace/db` (e.g., `import { db, pages } from 'pagespace/db'`) for all database access
Applied to files:
packages/lib/src/notifications/notifications.ts
📚 Learning: 2025-12-23T18:49:41.966Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.966Z
Learning: Applies to apps/web/src/**/*.{ts,tsx} : For database access, always use Drizzle client from `pagespace/db`: `import { db, pages } from 'pagespace/db';`
Applied to files:
packages/lib/src/notifications/notifications.ts
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/**/*.ts : Always use Drizzle client and queries from `pagespace/db` for database access instead of direct queries
Applied to files:
packages/lib/src/notifications/notifications.ts
📚 Learning: 2025-12-22T20:04:40.910Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.910Z
Learning: Applies to **/*.{ts,tsx} : Always use Drizzle client from `pagespace/db` package for database access
Applied to files:
packages/lib/src/notifications/notifications.ts
📚 Learning: 2025-12-22T20:04:40.910Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.910Z
Learning: Applies to packages/db/**/*.{ts,tsx} : Use Drizzle ORM for database queries with PostgreSQL
Applied to files:
packages/lib/src/notifications/notifications.ts
📚 Learning: 2025-12-23T18:49:41.966Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.966Z
Learning: Applies to apps/web/src/**/*.{ts,tsx} : Use centralized Drizzle ORM with PostgreSQL for all database operations - no direct SQL or other ORMs
Applied to files:
packages/lib/src/notifications/notifications.ts
🧬 Code graph analysis (1)
packages/lib/src/notifications/notifications.ts (1)
apps/web/src/lib/auth/auth-fetch.ts (1)
fetch(44-211)
⏰ 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). (1)
- GitHub Check: Unit Tests
🔇 Additional comments (13)
packages/lib/src/notifications/notifications.ts (2)
4-4: LGTM on the import.The addition of
createSignedBroadcastHeadersaligns with the security fix to add HMAC signatures for broadcast authentication.
14-24: The code is correct.createSignedBroadcastHeadersreturns bothContent-Type: application/jsonand the signature header, so no changes are needed.Likely an incorrect or invalid review comment.
apps/processor/src/middleware/auth.ts (6)
1-3: LGTM!The imports align with the new session-based authentication architecture, bringing in
sessionServicefor validation andEnforcedAuthContextfrom the centralized permissions module.
27-76: LGTM!The scope inference logic correctly maps request URLs to the appropriate scopes. The separate loops provide clear, readable code for each scope category.
89-141: LGTM!The middleware correctly implements the new session-based authentication flow:
- Validates the opaque token via
sessionService.validateSession- Rejects non-service sessions with 403 (proper distinction from 401)
- Constructs the immutable
EnforcedAuthContextand attaches it toreq.auth- Performs scope inference with appropriate logging for debugging
The generic error messages in the catch block appropriately avoid leaking sensitive validation details.
143-170: LGTM!The middleware factory correctly leverages
EnforcedAuthContext.hasScopefor authorization checks and provides informative error messages including the missing scope.
172-185: LGTM!Both helper functions properly handle the case where
req.authmay be undefined (e.g., whenAUTH_REQUIRED=false), returning safe fallback values.
187-187: LGTM!Re-exporting
EnforcedAuthContextprovides a convenient single import point for consumers of this middleware module.packages/lib/src/services/validated-service-token.ts (5)
18-32: Well-defined scope type with exhaustive literals.Good use of a string literal union type for
ServiceScope. This provides compile-time safety when checking scopes and aligns with the TypeScript coding guidelines.
34-37: LGTM!Constants follow UPPER_SNAKE_CASE convention and have clear inline comments. The bounds (10s min, 30 days max) are reasonable for service tokens.
48-86: Solid duration parsing with proper bounds validation.The function handles edge cases well: invalid formats, zero/negative values, and enforces min/max bounds with logging. The regex pattern correctly constrains valid inputs.
Minor note: The
defaultcase in the switch (line 70) is unreachable since the regex already ensuresunitis one of[smhd], but it's acceptable as defensive coding.
480-489: LGTM!Consistent with
createValidatedServiceToken. ThedriveIdis required here (perUploadTokenOptions), which correctly supports the processor's drive-based validation for uploads.
218-227: Clean migration to session-based token creation.The payload structure is well-organized. The
as string[]cast on line 221 is safe sinceServiceScopeis a union of string literals. ThesessionService.createSessionmethod accepts all the fields being passed, including the optionaldriveIdandcreatedByServiceparameters.
Use separate ID tracking variables with individual try/catch blocks to prevent cleanup failures from affecting subsequent tests. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Match the pattern from permissions.test.ts - clean ALL users at the start of beforeEach instead of selective cleanup in afterEach. This ensures a clean database state even if previous tests crashed. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/lib/src/__tests__/notifications.test.ts (1)
1-11: UnusedafterEachimport.The
afterEachimport on line 1 is no longer used after the cleanup logic was moved intobeforeEach. Consider removing it to keep imports clean.The import cleanup on line 10 (removing
sqlandnotifications) looks good—only importing what's needed.Suggested fix
-import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest' +import { describe, it, expect, beforeEach, vi } from 'vitest'
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/lib/src/__tests__/notifications.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Never useanytypes - always use proper TypeScript types
Use camelCase for variable and function names
Use UPPER_SNAKE_CASE for constants
Use PascalCase for type and enum names
Use kebab-case for filenames, except React hooks (camelCase withuseprefix), Zustand stores (camelCase withuseprefix), and React components (PascalCase)
Lint with Next/ESLint as configured inapps/web/eslint.config.mjs
Message content should always use the message parts structure with{ parts: [{ type: 'text', text: '...' }] }
Use centralized permission functions from@pagespace/lib/permissions(e.g.,getUserAccessLevel,canUserEditPage) instead of implementing permission logic locally
Always use Drizzle client from@pagespace/dbpackage for database access
Use ESM modules throughout the codebase
**/*.{ts,tsx}: Never useanytypes - always use proper TypeScript types
Write code that is explicit over implicit and self-documenting
Files:
packages/lib/src/__tests__/notifications.test.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: React hook files should use camelCase matching the exported hook name (e.g.,useAuth.ts)
Zustand store files should use camelCase withuseprefix (e.g.,useAuthStore.ts)
Files:
packages/lib/src/__tests__/notifications.test.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
packages/lib/src/__tests__/notifications.test.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.910Z
Learning: Applies to **/*auth*.{ts,tsx} : Use custom JWT authentication with jose library for user management
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to **/*.{ts,tsx} : Always use the Drizzle client and database exports from `pagespace/db` (e.g., `import { db, pages } from 'pagespace/db'`) for all database access
Applied to files:
packages/lib/src/__tests__/notifications.test.ts
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/**/*.ts : Always use Drizzle client and queries from `pagespace/db` for database access instead of direct queries
Applied to files:
packages/lib/src/__tests__/notifications.test.ts
📚 Learning: 2025-12-23T18:49:41.966Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.966Z
Learning: Applies to apps/web/src/**/*.{ts,tsx} : For database access, always use Drizzle client from `pagespace/db`: `import { db, pages } from 'pagespace/db';`
Applied to files:
packages/lib/src/__tests__/notifications.test.ts
🧬 Code graph analysis (1)
packages/lib/src/__tests__/notifications.test.ts (1)
packages/db/src/index.ts (1)
db(20-20)
⏰ 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). (2)
- GitHub Check: Lint & TypeScript Check
- GitHub Check: Unit Tests
🔇 Additional comments (1)
packages/lib/src/__tests__/notifications.test.ts (1)
22-36: LGTM!The refactored test setup is clean and well-documented. Moving cleanup to the start of
beforeEach(delete-then-create) ensures a fresh state for each test, and the comments clearly explain the cascade deletion behavior. The mock reset placement before test data is appropriate.
1 participant
Summary
Complete migration from JWT service tokens to opaque session tokens for the processor service:
sessionService.validateSession()instead of JWT verificationvalidated-service-token.tsnow creates opaque sessions viasessionService.createSession()driveIdcolumn to sessions table for processor validationservice-auth.tsand all JWT service token codeBreaking Changes
ps_svc_*) instead of JWT tokensreq.serviceAuthreplaced withreq.auth(EnforcedAuthContext)Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.
NOTE: Not done yet: user auth/refresh remains JWT-based, realtime has a JWT fallback for desktop, and desktop WS auth still uses JWT.
Full legacy JWT deprecation is tracked in plan P5-T5.