feat(cedarling-js): harden runtime examples - #14677
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Signed-off-by: Justin Dah-kenangnon <dah.kenangnon@gmail.com>
f26bf34 to
26bb857
Compare
moabu
left a comment
There was a problem hiding this comment.
Add it to the main README.md in the demos folder
There was a problem hiding this comment.
Actionable comments posted: 38
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
demos/cedarling-js-examples/electron/src/__tests__/oidc.test.ts (1)
1-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for the success and loopback-allowed paths.
Current tests only cover rejection paths. Add a test asserting
assertUserinfoSubjectpasses whensubmatches, and a test assertingremoteJwks("http://localhost:.../jwks")resolves instead of throwing. This protects the security-critical loopback exception and the happy path from silent regressions.test("accepts a signed UserInfo token for the matching subject", () => { expect(() => assertUserinfoSubject({ sub: "alice" }, "alice")).not.toThrow(); }); test("accepts a loopback HTTP JWKS endpoint", async () => { await expect(remoteJwks("http://localhost:4000/jwks")).resolves.toBeDefined(); });🤖 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 `@demos/cedarling-js-examples/electron/src/__tests__/oidc.test.ts` around lines 1 - 10, Add success-path coverage in the OIDC tests: add a matching-subject case for assertUserinfoSubject that does not throw, and add a loopback HTTP case for remoteJwks using a localhost JWKS URL that resolves successfully.
🤖 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 `@demos/cedarling-js-examples/common/policies/create-token.cedar`:
- Around line 8-11: The subject ownership checks currently use contains instead
of requiring an exact subject claim match. In
demos/cedarling-js-examples/common/policies/create-token.cedar lines 8-11 and
demos/cedarling-js-examples/common/policies/modify-token.cedar lines 8-11,
update the localmockidp_userinfo_token sub-tag comparison to use equality with
resource.owner.
In `@demos/cedarling-js-examples/common/ui/theme.css`:
- Around line 34-41: Update the font-family declaration in the body rule to
remove quotes from the single-word Lato font name, while retaining quotes around
Open Sans and leaving the fallback fonts unchanged.
In
`@demos/cedarling-js-examples/electron/.erb/configs/webpack.config.renderer.dev.ts`:
- Around line 51-52: Update the spawn setup around MAIN_ARGS to parse its value
into explicit individual arguments before appending them, rather than passing
the unsplit string as one shell-interpreted argument. Preserve the existing “--”
separator and npm invocation while ensuring MAIN_ARGS is not re-parsed by the
shell.
In `@demos/cedarling-js-examples/electron/src/__tests__/ipc.test.ts`:
- Around line 50-55: The test name around the “rejects unknown and unnecessary
fields at runtime” case does not describe the identity-validation assertion
using userId “mallory”. Rename the test to cover both unsupported fields and
invalid user IDs, or split the tasks:list assertion into a separate test with a
name focused on user ID validation; preserve the existing assertions and
behavior.
- Around line 28-31: Add tests in the IPC authorization suite for the denied and
error outcomes of mockAuthorizeAction. Override the default "allowed" response
in each test, invoke the relevant registered task handler, and assert that both
handlers reject, including a forbidden/denied message check for the denied case.
In `@demos/cedarling-js-examples/electron/src/main/cedarling/authorize.ts`:
- Around line 31-37: Update the authorization result handling around the
!result.ok branch and catch block to log each discarded SDK failure reason
before returning "error". Preserve the existing three-state contract and
decision mapping, and keep the log focused on the failure details without
changing IPC behavior.
In `@demos/cedarling-js-examples/electron/src/main/cedarling/config.ts`:
- Around line 25-31: Update loadCedarlingOptions to create a timeout-backed
AbortSignal and pass it to both issuer fetch calls in Promise.all. Ensure an
unresponsive issuer aborts the requests and causes the IPC operation to fail
rather than remaining pending, while preserving the existing non-OK response
handling.
- Around line 34-41: Update the validation condition in the Cedarling
configuration check to reject an empty jwt.allowedAlgorithms array, while
preserving the existing requirement that every listed algorithm is RS256.
In `@demos/cedarling-js-examples/electron/src/main/cedarling/init.ts`:
- Around line 30-37: Update shutDownCedarling so clientPromise is cleared before
awaiting its initialization result, and handle a rejected promise as an
already-failed initialization with nothing to drain. Only invoke
client.shutDown() and propagate its error when initialization succeeds.
In `@demos/cedarling-js-examples/electron/src/main/ipc.ts`:
- Around line 115-124: Update requireAuthorization to reject task mutations when
signedSession is absent, before calling authorizeForUser; preserve the existing
signed-session identity check and authorization outcomes. Ensure unsigned
requests cannot perform CreateTask, UpdateTask, or DeleteTask, while retaining
the existing authorization flow for permitted non-mutation operations.
- Around line 285-289: Update the tasks:list handler to retrieve tasks once,
evaluate authorization for each task, retain only tasks with an allowed outcome,
and exclude denied tasks from the returned list. Continue propagating
authorization-service errors, but do not let a normal denied result fail the
entire request; use the retrieved task collection for both authorization checks
and the response.
- Around line 66-94: Update the title-length validation messages in
parseCreateRequest and parseUpdateRequest to interpolate MAX_TITLE_LENGTH
instead of hardcoding “120 characters,” keeping the existing validation behavior
unchanged.
- Around line 215-221: Update the UserInfo fetch in the token exchange flow to
use the discovered metadata.userinfo_endpoint value instead of constructing the
endpoint from oidcIssuer() with a hardcoded /me path. Keep the existing
authorization header, JWT validation, and error handling unchanged.
In `@demos/cedarling-js-examples/electron/src/main/main.ts`:
- Around line 64-73: Update the before-quit handler around shutDownCedarling()
to race the shutdown promise against a bounded timeout, ensuring app.quit()
executes when Cedarling shutdown hangs. Preserve the existing error logging and
final quit behavior, and keep the shutdownStarted guard unchanged.
In `@demos/cedarling-js-examples/electron/src/renderer/App.css`:
- Line 4: Update the Stylelint configuration used by App.css to add Tailwind v4
at-rules to scss/at-rule-no-unknown’s ignoreAtRules list, including theme and
the other specified Tailwind directives, so valid Tailwind syntax passes lint.
In `@demos/cedarling-js-examples/electron/src/renderer/App.tsx`:
- Around line 129-136: Update the logout function to catch failures from
window.electron.oidc.logout(), report them through the same setError pattern
used by login(), and ensure the failed operation does not leave the UI
presenting a successfully authenticated session. Preserve the existing
busy-state cleanup in the finally block.
- Around line 64-75: Update the permission-check useEffect to clear permissions
before handling the empty-task early return and before starting each check.
Ensure identity changes, task-list changes, and empty task lists cannot retain
decisions from the previous user or task set, while preserving the existing
asynchronous check and cleanup behavior.
- Around line 92-100: Update createTask so the tasks.create call uses the
trimmed newTitle value, keeping the existing validation and success behavior
unchanged.
In `@demos/cedarling-js-examples/electron/src/renderer/cedarling/permissions.ts`:
- Around line 25-37: Centralize the Cedar identifiers by exporting
CEDAR_NAMESPACE, USER_TYPE, TASK_TYPE, and cedarAction alongside TaskAction in
shared/contracts.ts. Update the renderer request construction and the
main-process authorization flow in authorize.ts to use these shared constants
and formatter instead of hardcoded principal, resource, and action strings,
preserving the existing authorization behavior.
In `@demos/cedarling-js-examples/electron/src/shared/contracts.ts`:
- Around line 57-59: Update the PermissionMap index signature so task ID lookups
are typed as possibly undefined, preserving the existing permission object shape
and allowing App.tsx to safely use its ?? fallback for unknown IDs.
In `@demos/cedarling-js-examples/hono/src/app.test.ts`:
- Around line 1-87: Extend the tests in app.test.ts with a happy-path task
mutation case using createApp and an authorize callback returning { kind:
"allowed" }. Exercise at least one successful POST, PUT, or DELETE route and
assert its response status and returned task shape, covering the mutation wiring
handled by createApp.
In `@demos/cedarling-js-examples/hono/src/cedarling/init.ts`:
- Around line 32-39: Update the validation condition in the Cedarling
configuration initialization block to reject an empty jwt.allowedAlgorithms
array, requiring it to be non-empty in addition to containing only RS256.
Preserve the existing checks for applicationName, jwt presence, array type, and
disallowed algorithms.
- Around line 22-29: Update initialize to apply a bounded AbortSignal timeout to
both issuer configuration fetches for /config/cedarling and
/config/policy-store. Ensure timeout failures reject the shared initialization
promise so callers do not hang indefinitely and subsequent getCedarling attempts
can retry.
In
`@demos/cedarling-js-examples/react-nodejs/backend/cedarling/authz-middleware.js`:
- Around line 58-96: Update the Result-failure handling in the middleware’s
authorizeMultiIssuer/authorizeUnsigned flow to inspect result.error.code rather
than treating every signed failure as 401. Return 401 for signed identity
failures such as AUTHORIZATION_FAILED, and 503 for operational codes including
ISSUER_OPERATION_FAILED, CLIENT_CLOSED, or INVALID_INPUT; preserve the existing
error messages and unsigned fallback behavior. Add a test covering a signed-mode
Result failure alongside the existing unsigned fallback test.
In `@demos/cedarling-js-examples/react-nodejs/backend/cedarling/init.js`:
- Around line 21-39: Update createClient so the fetch request to the Cedarling
configuration endpoint uses a bounded timeout via AbortSignal.timeout or an
equivalent abort mechanism. Preserve the existing response validation, JSON
parsing, and createCedarling initialization flow while ensuring an unreachable
issuer cannot block startup indefinitely.
In `@demos/cedarling-js-examples/react-nodejs/frontend/src/App.tsx`:
- Around line 33-41: Update the signedMode initialization in the App component
to enable signed mode only when initialSession exists and its userId passes
isUserId, matching the guard used in the later session handling logic. Keep the
currentUser fallback to "bob" unchanged while preventing invalid stored sessions
from disabling user selection and sending mismatched credentials.
- Around line 48-67: Update the initialization useEffect to settle initCedarling
and completeLogin independently rather than using Promise.all, ensuring
setClient runs whenever Cedarling initialization succeeds even if login fails.
Preserve the existing login/session state updates for successful completion, and
route login or initialization failures through the existing error handling
without blocking the other operation.
In `@demos/cedarling-js-examples/react-nodejs/frontend/src/cedarling/init.ts`:
- Around line 12-25: Update strictConfig to reject an empty
jwt.allowedAlgorithms array by validating its length is greater than zero before
normalization; preserve rejection of non-RS256 entries and ensure only a
non-empty RS256 allowlist is normalized to ["RS256"].
In `@demos/cedarling-js-examples/react-nodejs/frontend/src/main.tsx`:
- Around line 7-13: Update the pagehide listener around shutDownCedarling to
accept the lifecycle event and skip shutdown when event.persisted is true,
preserving the existing shutdown and error logging for non-persisted page exits.
Remove the once-only listener option so it remains registered for future
pagehide events.
In `@demos/cedarling-js-examples/vercel-nextjs/app/api/tasks/`[id]/route.ts:
- Around line 11-14: In the PUT handler, move resolveRequestIdentity and its 401
response before findById so unauthenticated requests never reveal task
existence. Apply the same ordering change to the DELETE handler, keeping the
existing 404 response and task-processing logic unchanged for authenticated
requests.
In `@demos/cedarling-js-examples/vercel-nextjs/app/page.tsx`:
- Around line 256-257: Update the checking-status paragraph rendered by the
checking condition in the page component to include role="status", so assistive
technology announces “Checking permissions...” while the task actions remain
disabled.
In `@demos/cedarling-js-examples/vercel-nextjs/app/task-api.ts`:
- Around line 75-88: Enhance updateTask and the corresponding task update API to
use optimistic concurrency control instead of unconditional last-write-wins
updates. Add a version or ETag-based validator to the Task update request, have
the server reject stale versions or mismatched If-Match values, and preserve the
existing update behavior only when the client state is current.
- Around line 42-61: Replace the per-task dual requests in checkTaskPermissions
with one batched permission-check request that submits all task IDs and returns
the PermissionMap directly. Add or reuse a batch endpoint while preserving the
existing per-task route for single-check demonstrations, and update the
request/response handling to use the batch result without unbounded fan-out.
In `@demos/cedarling-js-examples/vercel-nextjs/libs/cedarling/authorize.ts`:
- Around line 15-17: Separate getCedarling() acquisition failures from
authorization evaluation failures in authorizeAction and authorizationFailure.
Ensure a rejection while obtaining the Cedarling client, including issuer
configuration fetch failures, maps to status 503 even for signed requests, while
signed authorization failures during evaluation continue mapping to status 401.
In `@demos/cedarling-js-examples/vercel-nextjs/libs/cedarling/init.ts`:
- Around line 23-24: Update the issuer configuration fetch in getCedarling to
use an AbortController with a finite timeout, passing its signal to fetch and
ensuring the timer is cleaned up after completion. Preserve the existing HTTP
error handling and allow the timed-out promise to reject so the cached pending
promise can clear and later requests can retry.
In `@demos/cedarling-js-examples/vercel-nextjs/libs/oidc/auth.ts`:
- Around line 21-31: Harden the session validation in the authentication flow
around verifyUserinfoToken by passing the stored ID-token subject as
expectedSubject, matching the validation already used in
app/api/oidc/callback/route.ts. Ensure the UserInfo token’s subject must agree
with the ID token while preserving the existing issuer verification, clientId
audience handling, and isUserId check.
In `@demos/cedarling-js-examples/vercel-nextjs/libs/oidc/provider.ts`:
- Around line 133-142: Update getRegisteredClient to prefer a persisted
OIDC_CLIENT_ID environment variable for deployed environments, returning a
client built from that identifier instead of calling registerClient. Keep the
existing global registration map and rejection cleanup as the local-development
fallback, so failed dynamic registrations remain retryable.
In `@demos/cedarling-js-examples/vercel-nextjs/libs/oidc/session.ts`:
- Around line 29-34: Update cookieOptions to derive the secure flag from the
configured application origin, reusing the existing getRequestOrigin
configuration source used by the OIDC provider instead of request.url. Ensure
cookies created by setSessionCookies use Secure whenever the configured origin
is HTTPS, including behind TLS-terminating proxies.
---
Outside diff comments:
In `@demos/cedarling-js-examples/electron/src/__tests__/oidc.test.ts`:
- Around line 1-10: Add success-path coverage in the OIDC tests: add a
matching-subject case for assertUserinfoSubject that does not throw, and add a
loopback HTTP case for remoteJwks using a localhost JWKS URL that resolves
successfully.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 20f4aff5-a9cc-4b0e-8e38-86b5442b6182
⛔ Files ignored due to path filters (6)
demos/cedarling-js-examples/common/package-lock.jsonis excluded by!**/package-lock.jsondemos/cedarling-js-examples/electron/package-lock.jsonis excluded by!**/package-lock.jsondemos/cedarling-js-examples/hono/package-lock.jsonis excluded by!**/package-lock.jsondemos/cedarling-js-examples/react-nodejs/backend/package-lock.jsonis excluded by!**/package-lock.jsondemos/cedarling-js-examples/react-nodejs/frontend/package-lock.jsonis excluded by!**/package-lock.jsondemos/cedarling-js-examples/vercel-nextjs/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (126)
demos/cedarling-js-examples/README.mddemos/cedarling-js-examples/common/README.mddemos/cedarling-js-examples/common/cedarling-config.jsondemos/cedarling-js-examples/common/idp.jsdemos/cedarling-js-examples/common/idp.test.jsdemos/cedarling-js-examples/common/package.jsondemos/cedarling-js-examples/common/policies.cedardemos/cedarling-js-examples/common/policies/create-token.cedardemos/cedarling-js-examples/common/policies/create-user.cedardemos/cedarling-js-examples/common/policies/modify-token.cedardemos/cedarling-js-examples/common/policies/modify-user.cedardemos/cedarling-js-examples/common/policies/view-token.cedardemos/cedarling-js-examples/common/policies/view-user.cedardemos/cedarling-js-examples/common/policy-store.jsdemos/cedarling-js-examples/common/policy-store.jsondemos/cedarling-js-examples/common/schema.cedarschemademos/cedarling-js-examples/common/test-config.jsondemos/cedarling-js-examples/common/ui/theme.cssdemos/cedarling-js-examples/electron/.erb/configs/webpack.config.main.dev.tsdemos/cedarling-js-examples/electron/.erb/configs/webpack.config.main.prod.tsdemos/cedarling-js-examples/electron/.erb/configs/webpack.config.preload.dev.tsdemos/cedarling-js-examples/electron/.erb/configs/webpack.config.renderer.dev.dll.tsdemos/cedarling-js-examples/electron/.erb/configs/webpack.config.renderer.dev.tsdemos/cedarling-js-examples/electron/.erb/configs/webpack.config.renderer.prod.tsdemos/cedarling-js-examples/electron/.erb/mocks/fileMock.jsdemos/cedarling-js-examples/electron/.erb/scripts/check-build-exists.tsdemos/cedarling-js-examples/electron/.erb/scripts/delete-source-maps.jsdemos/cedarling-js-examples/electron/README.mddemos/cedarling-js-examples/electron/package.jsondemos/cedarling-js-examples/electron/src/__tests__/App.test.tsxdemos/cedarling-js-examples/electron/src/__tests__/ipc.test.tsdemos/cedarling-js-examples/electron/src/__tests__/oidc.test.tsdemos/cedarling-js-examples/electron/src/main/cedarling/authorize.tsdemos/cedarling-js-examples/electron/src/main/cedarling/config.tsdemos/cedarling-js-examples/electron/src/main/cedarling/init.tsdemos/cedarling-js-examples/electron/src/main/ipc.tsdemos/cedarling-js-examples/electron/src/main/main.tsdemos/cedarling-js-examples/electron/src/main/oidc.tsdemos/cedarling-js-examples/electron/src/main/preload.tsdemos/cedarling-js-examples/electron/src/main/tasks.tsdemos/cedarling-js-examples/electron/src/renderer/App.cssdemos/cedarling-js-examples/electron/src/renderer/App.tsxdemos/cedarling-js-examples/electron/src/renderer/cedarling/exercise-signed.tsdemos/cedarling-js-examples/electron/src/renderer/cedarling/exercise-unsigned.tsdemos/cedarling-js-examples/electron/src/renderer/cedarling/init.tsdemos/cedarling-js-examples/electron/src/renderer/cedarling/permissions.tsdemos/cedarling-js-examples/electron/src/renderer/index.ejsdemos/cedarling-js-examples/electron/src/renderer/index.tsxdemos/cedarling-js-examples/electron/src/renderer/preload.d.tsdemos/cedarling-js-examples/electron/src/shared/contracts.tsdemos/cedarling-js-examples/hono/README.mddemos/cedarling-js-examples/hono/package.jsondemos/cedarling-js-examples/hono/src/app.test.tsdemos/cedarling-js-examples/hono/src/app.tsdemos/cedarling-js-examples/hono/src/cedarling/authorize.tsdemos/cedarling-js-examples/hono/src/cedarling/init.tsdemos/cedarling-js-examples/hono/src/entry.bun.tsdemos/cedarling-js-examples/hono/src/entry.cloudflare.tsdemos/cedarling-js-examples/hono/src/entry.deno.tsdemos/cedarling-js-examples/hono/src/tasks.tsdemos/cedarling-js-examples/hono/tsconfig.jsondemos/cedarling-js-examples/hono/wrangler.tomldemos/cedarling-js-examples/react-nodejs/README.mddemos/cedarling-js-examples/react-nodejs/backend/README.mddemos/cedarling-js-examples/react-nodejs/backend/cedarling/authz-middleware.jsdemos/cedarling-js-examples/react-nodejs/backend/cedarling/exercise-context.jsdemos/cedarling-js-examples/react-nodejs/backend/cedarling/exercise-issuers.jsdemos/cedarling-js-examples/react-nodejs/backend/cedarling/exercise-lifecycle.jsdemos/cedarling-js-examples/react-nodejs/backend/cedarling/exercise-logs.jsdemos/cedarling-js-examples/react-nodejs/backend/cedarling/index.jsdemos/cedarling-js-examples/react-nodejs/backend/cedarling/init.jsdemos/cedarling-js-examples/react-nodejs/backend/package.jsondemos/cedarling-js-examples/react-nodejs/backend/server.jsdemos/cedarling-js-examples/react-nodejs/backend/server.test.jsdemos/cedarling-js-examples/react-nodejs/frontend/README.mddemos/cedarling-js-examples/react-nodejs/frontend/index.htmldemos/cedarling-js-examples/react-nodejs/frontend/package.jsondemos/cedarling-js-examples/react-nodejs/frontend/src/App.tsxdemos/cedarling-js-examples/react-nodejs/frontend/src/cedarling/exercise-signed.tsdemos/cedarling-js-examples/react-nodejs/frontend/src/cedarling/exercise-unsigned.tsdemos/cedarling-js-examples/react-nodejs/frontend/src/cedarling/init.tsdemos/cedarling-js-examples/react-nodejs/frontend/src/cedarling/permissions.test.tsdemos/cedarling-js-examples/react-nodejs/frontend/src/cedarling/permissions.tsdemos/cedarling-js-examples/react-nodejs/frontend/src/index.cssdemos/cedarling-js-examples/react-nodejs/frontend/src/main.tsxdemos/cedarling-js-examples/react-nodejs/frontend/src/model.tsdemos/cedarling-js-examples/react-nodejs/frontend/src/oidc.test.tsdemos/cedarling-js-examples/react-nodejs/frontend/src/oidc.tsdemos/cedarling-js-examples/react-nodejs/frontend/src/task-api.test.tsdemos/cedarling-js-examples/react-nodejs/frontend/src/task-api.tsdemos/cedarling-js-examples/react-nodejs/frontend/src/vite-env.d.tsdemos/cedarling-js-examples/react-nodejs/frontend/tailwind.config.jsdemos/cedarling-js-examples/react-nodejs/frontend/tsconfig.jsondemos/cedarling-js-examples/scripts/install-example.mjsdemos/cedarling-js-examples/scripts/install.mjsdemos/cedarling-js-examples/vercel-nextjs/README.mddemos/cedarling-js-examples/vercel-nextjs/app/api/cedarling/exercises/route.tsdemos/cedarling-js-examples/vercel-nextjs/app/api/check-edge/route.tsdemos/cedarling-js-examples/vercel-nextjs/app/api/check/route.tsdemos/cedarling-js-examples/vercel-nextjs/app/api/config/route.tsdemos/cedarling-js-examples/vercel-nextjs/app/api/oidc/callback/route.tsdemos/cedarling-js-examples/vercel-nextjs/app/api/oidc/logout/callback/route.tsdemos/cedarling-js-examples/vercel-nextjs/app/api/oidc/logout/route.tsdemos/cedarling-js-examples/vercel-nextjs/app/api/oidc/session/route.tsdemos/cedarling-js-examples/vercel-nextjs/app/api/oidc/start/route.tsdemos/cedarling-js-examples/vercel-nextjs/app/api/tasks/[id]/route.tsdemos/cedarling-js-examples/vercel-nextjs/app/api/tasks/route.tsdemos/cedarling-js-examples/vercel-nextjs/app/globals.cssdemos/cedarling-js-examples/vercel-nextjs/app/layout.tsxdemos/cedarling-js-examples/vercel-nextjs/app/page.tsxdemos/cedarling-js-examples/vercel-nextjs/app/task-api.tsdemos/cedarling-js-examples/vercel-nextjs/libs/cedarling/authorize.tsdemos/cedarling-js-examples/vercel-nextjs/libs/cedarling/exercises.tsdemos/cedarling-js-examples/vercel-nextjs/libs/cedarling/init.tsdemos/cedarling-js-examples/vercel-nextjs/libs/demo-domain.tsdemos/cedarling-js-examples/vercel-nextjs/libs/oidc/auth.tsdemos/cedarling-js-examples/vercel-nextjs/libs/oidc/provider.tsdemos/cedarling-js-examples/vercel-nextjs/libs/oidc/session.tsdemos/cedarling-js-examples/vercel-nextjs/libs/permission-check.tsdemos/cedarling-js-examples/vercel-nextjs/libs/tasks.tsdemos/cedarling-js-examples/vercel-nextjs/next.config.tsdemos/cedarling-js-examples/vercel-nextjs/package.jsondemos/cedarling-js-examples/vercel-nextjs/playwright.config.tsdemos/cedarling-js-examples/vercel-nextjs/tailwind.config.tsdemos/cedarling-js-examples/vercel-nextjs/tests/e2e/oidc.spec.tsdemos/cedarling-js-examples/vercel-nextjs/tsconfig.json
💤 Files with no reviewable changes (26)
- demos/cedarling-js-examples/react-nodejs/backend/cedarling/index.js
- demos/cedarling-js-examples/vercel-nextjs/app/api/cedarling/exercises/route.ts
- demos/cedarling-js-examples/electron/.erb/scripts/delete-source-maps.js
- demos/cedarling-js-examples/electron/.erb/mocks/fileMock.js
- demos/cedarling-js-examples/vercel-nextjs/package.json
- demos/cedarling-js-examples/electron/.erb/configs/webpack.config.preload.dev.ts
- demos/cedarling-js-examples/react-nodejs/backend/cedarling/exercise-logs.js
- demos/cedarling-js-examples/vercel-nextjs/libs/cedarling/exercises.ts
- demos/cedarling-js-examples/common/test-config.json
- demos/cedarling-js-examples/scripts/install.mjs
- demos/cedarling-js-examples/vercel-nextjs/app/api/oidc/logout/route.ts
- demos/cedarling-js-examples/vercel-nextjs/next.config.ts
- demos/cedarling-js-examples/vercel-nextjs/playwright.config.ts
- demos/cedarling-js-examples/react-nodejs/backend/cedarling/exercise-lifecycle.js
- demos/cedarling-js-examples/common/policy-store.json
- demos/cedarling-js-examples/react-nodejs/frontend/src/cedarling/exercise-unsigned.ts
- demos/cedarling-js-examples/vercel-nextjs/app/api/config/route.ts
- demos/cedarling-js-examples/electron/.erb/scripts/check-build-exists.ts
- demos/cedarling-js-examples/react-nodejs/backend/cedarling/exercise-context.js
- demos/cedarling-js-examples/electron/.erb/configs/webpack.config.renderer.dev.dll.ts
- demos/cedarling-js-examples/common/policies.cedar
- demos/cedarling-js-examples/electron/src/renderer/cedarling/exercise-unsigned.ts
- demos/cedarling-js-examples/react-nodejs/frontend/src/cedarling/exercise-signed.ts
- demos/cedarling-js-examples/electron/src/renderer/cedarling/exercise-signed.ts
- demos/cedarling-js-examples/react-nodejs/backend/cedarling/exercise-issuers.js
- demos/cedarling-js-examples/vercel-nextjs/tailwind.config.ts
| body { | ||
| margin: 0; | ||
| background: var(--bg-app); | ||
| color: var(--text-primary); | ||
| font-family: "Lato", "Open Sans", Arial, sans-serif; | ||
| -webkit-font-smoothing: antialiased; | ||
| -moz-osx-font-smoothing: grayscale; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove unnecessary quotes around the single-word font name Lato.
Stylelint's font-family-name-quotes rule flags the quotes on "Lato". Quotes are recommended only for names that contain white space, digits, or punctuation. Lato is a single valid CSS identifier, so it does not need quotes. Keep the quotes on "Open Sans" because it contains a space.
🎨 Proposed fix
- font-family: "Lato", "Open Sans", Arial, sans-serif;
+ font-family: Lato, "Open Sans", Arial, sans-serif;📝 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.
| body { | |
| margin: 0; | |
| background: var(--bg-app); | |
| color: var(--text-primary); | |
| font-family: "Lato", "Open Sans", Arial, sans-serif; | |
| -webkit-font-smoothing: antialiased; | |
| -moz-osx-font-smoothing: grayscale; | |
| } | |
| body { | |
| margin: 0; | |
| background: var(--bg-app); | |
| color: var(--text-primary); | |
| font-family: Lato, "Open Sans", Arial, sans-serif; | |
| -webkit-font-smoothing: antialiased; | |
| -moz-osx-font-smoothing: grayscale; | |
| } |
🧰 Tools
🪛 Stylelint (17.14.1)
[error] 38-38: Expected no quotes around "Lato" (font-family-name-quotes)
(font-family-name-quotes)
🤖 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 `@demos/cedarling-js-examples/common/ui/theme.css` around lines 34 - 41, Update
the font-family declaration in the body rule to remove quotes from the
single-word Lato font name, while retaining quotes around Open Sans and leaving
the fallback fonts unchanged.
Source: Linters/SAST tools
| if (process.env.MAIN_ARGS) args.push("--", process.env.MAIN_ARGS); | ||
| spawn("npm", args, { shell: true, stdio: "inherit" }) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
MAIN_ARGS is re-parsed by the shell.
spawn runs with shell: true, so Node joins the argument array into a command string. The MAIN_ARGS value is then interpreted by the shell, including quotes and metacharacters. Split the value into separate arguments to make the argument list explicit and shell-independent.
🔒️ Proposed fix
const args = ["run", "start:main"];
- if (process.env.MAIN_ARGS) args.push("--", process.env.MAIN_ARGS);
- spawn("npm", args, { shell: true, stdio: "inherit" })
+ if (process.env.MAIN_ARGS) {
+ args.push("--", ...process.env.MAIN_ARGS.split(/\s+/).filter(Boolean));
+ }
+ spawn("npm", args, { shell: process.platform === "win32", stdio: "inherit" })
.on("close", (code) => process.exit(code ?? 0))
.on("error", (error) => console.error(error));🤖 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
`@demos/cedarling-js-examples/electron/.erb/configs/webpack.config.renderer.dev.ts`
around lines 51 - 52, Update the spawn setup around MAIN_ARGS to parse its value
into explicit individual arguments before appending them, rather than passing
the unsplit string as one shell-interpreted argument. Preserve the existing “--”
separator and npm invocation while ensuring MAIN_ARGS is not re-parsed by the
shell.
Source: Linters/SAST tools
| beforeEach(() => { | ||
| mockAuthorizeAction.mockReset(); | ||
| mockAuthorizeAction.mockResolvedValue({ allowed: true }); | ||
| mockAuthorizeAction.mockResolvedValue("allowed"); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add coverage for denied and error authorization outcomes.
beforeEach always resolves authorizeAction with "allowed". No test asserts the fail-closed behavior when authorizeAction returns "denied" or "error". These paths are the security-relevant branches of the IPC boundary. Add tests that set each outcome and assert that the handler rejects.
♻️ Suggested additional tests
it("rejects task operations when authorization is denied", async () => {
mockAuthorizeAction.mockResolvedValue("denied");
await expect(
handlers.get("tasks:delete")?.({}, { userId: "bob", id: "task-1" }),
).rejects.toThrow(/[Ff]orbidden|denied/);
});
it("fails closed when Cedarling reports an error", async () => {
mockAuthorizeAction.mockResolvedValue("error");
await expect(
handlers.get("tasks:list")?.({}, { userId: "bob" }),
).rejects.toThrow();
});🤖 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 `@demos/cedarling-js-examples/electron/src/__tests__/ipc.test.ts` around lines
28 - 31, Add tests in the IPC authorization suite for the denied and error
outcomes of mockAuthorizeAction. Override the default "allowed" response in each
test, invoke the relevant registered task handler, and assert that both handlers
reject, including a forbidden/denied message check for the denied case.
| it("rejects unknown and unnecessary fields at runtime", async () => { | ||
| await expect( | ||
| handlers.get("tasks:create")?.({}, { userId: "bob", title: "Task", owner: "alice" }), | ||
| ).rejects.toThrow(/unsupported field/); | ||
| await expect(handlers.get("tasks:list")?.({}, { userId: "mallory" })).rejects.toThrow(/Unknown/); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Align the test name with the assertions.
The second assertion passes userId: "mallory", which is an invalid user id, not an unknown field. The name states "unknown and unnecessary fields". Rename the test or split the identity-validation assertion into its own test so the failure message identifies the actual rule under test.
🤖 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 `@demos/cedarling-js-examples/electron/src/__tests__/ipc.test.ts` around lines
50 - 55, The test name around the “rejects unknown and unnecessary fields at
runtime” case does not describe the identity-validation assertion using userId
“mallory”. Rename the test to cover both unsupported fields and invalid user
IDs, or split the tasks:list assertion into a separate test with a name focused
on user ID validation; preserve the existing assertions and behavior.
| return outcome.signed | ||
| ? ({ status: 401, error: "Invalid or expired signed identity" } as const) | ||
| : ({ status: 503, error: "Authorization service unavailable" } as const); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
An engine outage on a signed request reports 401 instead of 503.
authorizationFailure maps every { kind: "error", signed: true } outcome to 401 "Invalid or expired signed identity". authorizeAction produces that same outcome for two different causes. Line 27 calls getCedarling() inside the try block, and that call rejects when the issuer /config/cedarling fetch fails in demos/cedarling-js-examples/vercel-nextjs/libs/cedarling/init.ts. A signed caller therefore receives 401 during a Cedarling outage. The client treats the outage as an expired session and can drop a valid login.
Separate the client-acquisition failure from the authorization evaluation failure.
🛠️ Proposed fix to distinguish the two error causes
export type AuthorizationOutcome =
| { kind: "allowed" }
| { kind: "denied" }
- | { kind: "error"; signed: boolean };
+ | { kind: "error"; signed: boolean; unavailable?: boolean };
export function authorizationFailure(outcome: AuthorizationOutcome) {
if (outcome.kind === "allowed") return undefined;
if (outcome.kind === "denied") return { status: 403, error: "Forbidden by policy" } as const;
- return outcome.signed
+ return outcome.signed && !outcome.unavailable
? ({ status: 401, error: "Invalid or expired signed identity" } as const)
: ({ status: 503, error: "Authorization service unavailable" } as const);
} token?: string,
): Promise<AuthorizationOutcome> {
+ let client;
+ try {
+ client = await getCedarling();
+ } catch {
+ return { kind: "error", signed: Boolean(token), unavailable: true };
+ }
try {
- const client = await getCedarling();Also applies to: 26-27
🤖 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 `@demos/cedarling-js-examples/vercel-nextjs/libs/cedarling/authorize.ts` around
lines 15 - 17, Separate getCedarling() acquisition failures from authorization
evaluation failures in authorizeAction and authorizationFailure. Ensure a
rejection while obtaining the Cedarling client, including issuer configuration
fetch failures, maps to status 503 even for signed requests, while signed
authorization failures during evaluation continue mapping to status 401.
| const response = await fetch(`${issuer}/config/cedarling`); | ||
| if (!response.ok) throw new Error(`Failed to load Cedarling config: HTTP ${response.status}`); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a timeout to the issuer configuration fetch.
Line 23 calls fetch with no abort signal. getCedarling caches one pending promise per worker, so every concurrent request awaits this single fetch. If the issuer accepts the connection and never responds, all task routes and check routes hang until the platform request timeout expires. A timeout also lets the cached rejection clear so a later request can retry.
⏱️ Proposed fix to bound the fetch
- const response = await fetch(`${issuer}/config/cedarling`);
+ const response = await fetch(`${issuer}/config/cedarling`, {
+ signal: AbortSignal.timeout(5_000),
+ });📝 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.
| const response = await fetch(`${issuer}/config/cedarling`); | |
| if (!response.ok) throw new Error(`Failed to load Cedarling config: HTTP ${response.status}`); | |
| const response = await fetch(`${issuer}/config/cedarling`, { | |
| signal: AbortSignal.timeout(5_000), | |
| }); | |
| if (!response.ok) throw new Error(`Failed to load Cedarling config: HTTP ${response.status}`); |
🤖 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 `@demos/cedarling-js-examples/vercel-nextjs/libs/cedarling/init.ts` around
lines 23 - 24, Update the issuer configuration fetch in getCedarling to use an
AbortController with a finite timeout, passing its signal to fetch and ensuring
the timer is cleaned up after completion. Preserve the existing HTTP error
handling and allow the timed-out promise to reject so the cached pending promise
can clear and later requests can retry.
| if (hasSessionCookie) { | ||
| if (!session.clientId || !session.idToken || !session.userinfoToken) return null; | ||
| try { | ||
| const claims = await verifyUserinfoToken( | ||
| session.userinfoToken, | ||
| await getDiscovery(), | ||
| session.clientId, | ||
| ); | ||
| return isUserId(claims.sub) | ||
| ? { userId: claims.sub, token: session.userinfoToken } | ||
| : null; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
The verification audience comes from a client-controlled cookie.
session.clientId is read from the taskapp_oidc_client_id cookie and passed to verifyUserinfoToken as the expected audience. The caller therefore selects the audience that its own token is checked against. An attacker who can set cookies can present a UserInfo JWT issued to a different client at the same issuer together with the matching clientId.
The exposure is limited here. The signature must verify against the issuer JWKS, isUserId(claims.sub) restricts the subject to the demo user set, and the cookies are HttpOnly. The idToken and userinfoToken are also written together in one setSessionCookies call.
Two hardening options for the example:
- Pass the ID-token subject as
expectedSubjectso the two stored tokens must agree, asapp/api/oidc/callback/route.tsalready does at login. - Bind the session to a server-side registration record and resolve the expected
clientIdfrom it instead of from the cookie.
🤖 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 `@demos/cedarling-js-examples/vercel-nextjs/libs/oidc/auth.ts` around lines 21
- 31, Harden the session validation in the authentication flow around
verifyUserinfoToken by passing the stored ID-token subject as expectedSubject,
matching the validation already used in app/api/oidc/callback/route.ts. Ensure
the UserInfo token’s subject must agree with the ID token while preserving the
existing issuer verification, clientId audience handling, and isUserId check.
| export function getRegisteredClient(origin: string): Promise<RegisteredClient> { | ||
| const registrations = globalForOidc.taskAppRegistrations ??= new Map(); | ||
| let pending = registrations.get(origin); | ||
| if (!pending) { | ||
| pending = registerClient(origin).catch((error) => { | ||
| registrations.delete(origin); | ||
| throw error; | ||
| }); | ||
| registrations.set(origin, pending); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial
Note the per-instance registration cost in a serverless deployment.
globalForOidc.taskAppRegistrations lives in the module global, so it is scoped to one serverless instance. Every cold start and every new isolate performs a fresh dynamic client registration for the same origin. The provider accumulates one client record per instance.
The rejection cleanup on line 138 is correct and allows a retry after a failed registration. For the deployed example, a persisted OIDC_CLIENT_ID environment variable, with dynamic registration used only for local development, would avoid the unbounded client growth.
🤖 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 `@demos/cedarling-js-examples/vercel-nextjs/libs/oidc/provider.ts` around lines
133 - 142, Update getRegisteredClient to prefer a persisted OIDC_CLIENT_ID
environment variable for deployed environments, returning a client built from
that identifier instead of calling registerClient. Keep the existing global
registration map and rejection cleanup as the local-development fallback, so
failed dynamic registrations remain retryable.
| function cookieOptions(request: NextRequest, maxAge: number) { | ||
| return { | ||
| httpOnly: true, | ||
| sameSite: 'lax' as const, | ||
| secure: new URL(request.url).protocol === 'https:', | ||
| path: '/', | ||
| sameSite: "lax" as const, | ||
| secure: new URL(request.url).protocol === "https:", | ||
| path: "/", |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Derive secure from the configured origin, not from request.url.
Line 33 sets secure from the protocol of request.url. Behind a TLS-terminating proxy, request.url is http: even when the browser used HTTPS. The cookies are then set without the Secure attribute. setSessionCookies stores the UserInfo JWT and the ID token in these cookies, so a plaintext request would transmit them in the clear.
getRequestOrigin in libs/oidc/provider.ts already requires APP_ORIGIN in production. Use the same source here so the attribute follows the deployed scheme.
🔒️ Proposed fix
function cookieOptions(request: NextRequest, maxAge: number) {
+ const configuredOrigin = process.env.APP_ORIGIN;
+ const scheme = configuredOrigin
+ ? new URL(configuredOrigin).protocol
+ : new URL(request.url).protocol;
return {
httpOnly: true,
sameSite: "lax" as const,
- secure: new URL(request.url).protocol === "https:",
+ secure: scheme === "https:",
path: "/",
maxAge,
};
}📝 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.
| function cookieOptions(request: NextRequest, maxAge: number) { | |
| return { | |
| httpOnly: true, | |
| sameSite: 'lax' as const, | |
| secure: new URL(request.url).protocol === 'https:', | |
| path: '/', | |
| sameSite: "lax" as const, | |
| secure: new URL(request.url).protocol === "https:", | |
| path: "/", | |
| function cookieOptions(request: NextRequest, maxAge: number) { | |
| const configuredOrigin = process.env.APP_ORIGIN; | |
| const scheme = configuredOrigin | |
| ? new URL(configuredOrigin).protocol | |
| : new URL(request.url).protocol; | |
| return { | |
| httpOnly: true, | |
| sameSite: "lax" as const, | |
| secure: scheme === "https:", | |
| path: "/", |
🤖 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 `@demos/cedarling-js-examples/vercel-nextjs/libs/oidc/session.ts` around lines
29 - 34, Update cookieOptions to derive the secure flag from the configured
application origin, reusing the existing getRequestOrigin configuration source
used by the OIDC provider instead of request.url. Ensure cookies created by
setSessionCookies use Secure whenever the configured origin is HTTPS, including
behind TLS-terminating proxies.
|
function identity(req) {
const userId = req.get("x-user-id");
...
}Even in signed mode, Concretely reachable from the UI: in Suggest deriving
None of these suites exercise an explicit
Behavior change worth calling out explicitly (not just implied by the |
- unquote the single-word Lato font fallback in the shared theme - split MAIN_ARGS into individual arguments for the Electron dev server - bound issuer configuration fetches with abort timeouts - reject empty RS256 algorithm allowlists in Cedarling config loading - map signed identity failures to 401 and operational failures to 503 - filter tasks:list to outcomes the user is authorized to view - centralize Cedar identifiers and actions in shared contracts - reset permission state, trim titles, and catch logout errors in renderers - time-box Cedarling shutdown and drain pending IPC work on quit - bind the stored ID-token subject to the UserInfo token - derive cookie Secure from APP_ORIGIN behind TLS-terminating proxies - authenticate before task existence checks and announce permission checks - add tests for denied/error authorization outcomes and mutation happy paths Signed-off-by: Justin Dah-kenangnon <dah.kenangnon@gmail.com>
Prepare
Description
Target issue
Contributes to #14674
Implementation Details
This PR hardens, refactors, and updates the Cedarling JS example applications (Electron, Hono, React/Node.js, and Next.js) to align with recent updates to the Cedarling JS SDK.
Key changes include:
Common Assets & Policy Modularization:
policies.cedarinto modular, domain-specific policy files (create-token,create-user,modify-token,modify-user,view-token,view-user) to improve maintenance and scoping.policy-store.jsto dynamically load policies and configure Cedarling.common/ui/theme.css.Electron Demo:
App.tsxand updated IPC routing contracts.Hono Demo:
init.ts) and authorization (authorize.ts) routines.app.test.ts).React & Node.js Demo:
Vercel & Next.js Demo:
app/page.tsx) to match the new Cedarling API design.tests/e2e/oidc.spec.ts) to cover user interaction and authentication redirection.Test and Document the changes
Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with
docs:to indicate documentation changes or if the below checklist is not selected.Summary by CodeRabbit