Skip to content

feat(cedarling-js): harden runtime examples - #14677

Open
Dahkenangnon wants to merge 2 commits into
jans-cedarling-14582-review-sdkfrom
jans-cedarling-14582-review-examples
Open

feat(cedarling-js): harden runtime examples#14677
Dahkenangnon wants to merge 2 commits into
jans-cedarling-14582-review-sdkfrom
jans-cedarling-14582-review-examples

Conversation

@Dahkenangnon

@Dahkenangnon Dahkenangnon commented Aug 3, 2026

Copy link
Copy Markdown

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:

  1. Common Assets & Policy Modularization:

    • Split the single policies.cedar into modular, domain-specific policy files (create-token, create-user, modify-token, modify-user, view-token, view-user) to improve maintenance and scoping.
    • Replaced static configurations with policy-store.js to dynamically load policies and configure Cedarling.
    • Shared unified styling variables via a new common/ui/theme.css.
  2. Electron Demo:

    • Streamlined Webpack, preload, and IPC configuration scripts.
    • Refactored the core renderer UI in App.tsx and updated IPC routing contracts.
    • Added unit tests for OIDC and IPC communication workflows.
  3. Hono Demo:

    • Decoupled and refactored Cedarling initialization (init.ts) and authorization (authorize.ts) routines.
    • Modernized the API entrypoints for Bun, Cloudflare, and Deno runtimes.
    • Introduced integration tests (app.test.ts).
  4. React & Node.js Demo:

    • Upgraded both frontend and backend configurations with proper TypeScript types, unified mock OIDC handlers, and middleware.
    • Added automated tests for task APIs, OIDC, and server endpoints.
  5. Vercel & Next.js Demo:

    • Standardized routing and page components (app/page.tsx) to match the new Cedarling API design.
    • Updated Playwright end-to-end integration tests (tests/e2e/oidc.spec.ts) to cover user interaction and authentication redirection.

Test and Document the changes

  • Static code analysis has been run locally and issues have been fixed
  • Relevant unit and integration tests have been added/updated
  • Relevant documentation has been updated if any (i.e. user guides, installation and configuration guides, technical design docs etc)

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.

  • I confirm that there is no impact on the docs due to the code changes in this PR.

Summary by CodeRabbit

  • New Features
    • Added streamlined task-management experiences across Electron, React, Hono, and Next.js demos.
    • Added OIDC/PKCE sign-in, signed UserInfo validation, session handling, and permission-aware task actions.
    • Added configurable origins, issuer-based authorization, secure CORS, input validation, and fail-closed authorization outcomes.
    • Added shared themes and clearer runtime configuration.
  • Bug Fixes
    • Improved handling of invalid identities, forged ownership, malformed requests, unsupported actions, and authorization failures.
    • Restricted insecure HTTP usage to local development and strengthened browser security policies.
  • Documentation
    • Simplified setup, deployment, architecture, security, and verification guidance across demos.
  • Tests
    • Added integration, security, API, permission, OIDC, and end-to-end coverage.

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 809e7453-e6a4-4f51-a38f-886148477752

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mo-auto

mo-auto commented Aug 3, 2026

Copy link
Copy Markdown
Member

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 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>
@Dahkenangnon
Dahkenangnon force-pushed the jans-cedarling-14582-review-examples branch from f26bf34 to 26bb857 Compare August 3, 2026 11:09

@moabu moabu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add it to the main README.md in the demos folder

@coderabbitai coderabbitai Bot added comp-jans-cedarling Touching folder /jans-cedarling kind-feature Issue or PR is a new feature request labels Aug 3, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Add tests for the success and loopback-allowed paths.

Current tests only cover rejection paths. Add a test asserting assertUserinfoSubject passes when sub matches, and a test asserting remoteJwks("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

📥 Commits

Reviewing files that changed from the base of the PR and between 8a2f5f5 and 26bb857.

⛔ Files ignored due to path filters (6)
  • demos/cedarling-js-examples/common/package-lock.json is excluded by !**/package-lock.json
  • demos/cedarling-js-examples/electron/package-lock.json is excluded by !**/package-lock.json
  • demos/cedarling-js-examples/hono/package-lock.json is excluded by !**/package-lock.json
  • demos/cedarling-js-examples/react-nodejs/backend/package-lock.json is excluded by !**/package-lock.json
  • demos/cedarling-js-examples/react-nodejs/frontend/package-lock.json is excluded by !**/package-lock.json
  • demos/cedarling-js-examples/vercel-nextjs/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (126)
  • demos/cedarling-js-examples/README.md
  • demos/cedarling-js-examples/common/README.md
  • demos/cedarling-js-examples/common/cedarling-config.json
  • demos/cedarling-js-examples/common/idp.js
  • demos/cedarling-js-examples/common/idp.test.js
  • demos/cedarling-js-examples/common/package.json
  • demos/cedarling-js-examples/common/policies.cedar
  • demos/cedarling-js-examples/common/policies/create-token.cedar
  • demos/cedarling-js-examples/common/policies/create-user.cedar
  • demos/cedarling-js-examples/common/policies/modify-token.cedar
  • demos/cedarling-js-examples/common/policies/modify-user.cedar
  • demos/cedarling-js-examples/common/policies/view-token.cedar
  • demos/cedarling-js-examples/common/policies/view-user.cedar
  • demos/cedarling-js-examples/common/policy-store.js
  • demos/cedarling-js-examples/common/policy-store.json
  • demos/cedarling-js-examples/common/schema.cedarschema
  • demos/cedarling-js-examples/common/test-config.json
  • demos/cedarling-js-examples/common/ui/theme.css
  • demos/cedarling-js-examples/electron/.erb/configs/webpack.config.main.dev.ts
  • demos/cedarling-js-examples/electron/.erb/configs/webpack.config.main.prod.ts
  • demos/cedarling-js-examples/electron/.erb/configs/webpack.config.preload.dev.ts
  • demos/cedarling-js-examples/electron/.erb/configs/webpack.config.renderer.dev.dll.ts
  • demos/cedarling-js-examples/electron/.erb/configs/webpack.config.renderer.dev.ts
  • demos/cedarling-js-examples/electron/.erb/configs/webpack.config.renderer.prod.ts
  • demos/cedarling-js-examples/electron/.erb/mocks/fileMock.js
  • demos/cedarling-js-examples/electron/.erb/scripts/check-build-exists.ts
  • demos/cedarling-js-examples/electron/.erb/scripts/delete-source-maps.js
  • demos/cedarling-js-examples/electron/README.md
  • demos/cedarling-js-examples/electron/package.json
  • demos/cedarling-js-examples/electron/src/__tests__/App.test.tsx
  • demos/cedarling-js-examples/electron/src/__tests__/ipc.test.ts
  • demos/cedarling-js-examples/electron/src/__tests__/oidc.test.ts
  • demos/cedarling-js-examples/electron/src/main/cedarling/authorize.ts
  • demos/cedarling-js-examples/electron/src/main/cedarling/config.ts
  • demos/cedarling-js-examples/electron/src/main/cedarling/init.ts
  • demos/cedarling-js-examples/electron/src/main/ipc.ts
  • demos/cedarling-js-examples/electron/src/main/main.ts
  • demos/cedarling-js-examples/electron/src/main/oidc.ts
  • demos/cedarling-js-examples/electron/src/main/preload.ts
  • demos/cedarling-js-examples/electron/src/main/tasks.ts
  • demos/cedarling-js-examples/electron/src/renderer/App.css
  • demos/cedarling-js-examples/electron/src/renderer/App.tsx
  • demos/cedarling-js-examples/electron/src/renderer/cedarling/exercise-signed.ts
  • demos/cedarling-js-examples/electron/src/renderer/cedarling/exercise-unsigned.ts
  • demos/cedarling-js-examples/electron/src/renderer/cedarling/init.ts
  • demos/cedarling-js-examples/electron/src/renderer/cedarling/permissions.ts
  • demos/cedarling-js-examples/electron/src/renderer/index.ejs
  • demos/cedarling-js-examples/electron/src/renderer/index.tsx
  • demos/cedarling-js-examples/electron/src/renderer/preload.d.ts
  • demos/cedarling-js-examples/electron/src/shared/contracts.ts
  • demos/cedarling-js-examples/hono/README.md
  • demos/cedarling-js-examples/hono/package.json
  • demos/cedarling-js-examples/hono/src/app.test.ts
  • demos/cedarling-js-examples/hono/src/app.ts
  • demos/cedarling-js-examples/hono/src/cedarling/authorize.ts
  • demos/cedarling-js-examples/hono/src/cedarling/init.ts
  • demos/cedarling-js-examples/hono/src/entry.bun.ts
  • demos/cedarling-js-examples/hono/src/entry.cloudflare.ts
  • demos/cedarling-js-examples/hono/src/entry.deno.ts
  • demos/cedarling-js-examples/hono/src/tasks.ts
  • demos/cedarling-js-examples/hono/tsconfig.json
  • demos/cedarling-js-examples/hono/wrangler.toml
  • demos/cedarling-js-examples/react-nodejs/README.md
  • demos/cedarling-js-examples/react-nodejs/backend/README.md
  • demos/cedarling-js-examples/react-nodejs/backend/cedarling/authz-middleware.js
  • demos/cedarling-js-examples/react-nodejs/backend/cedarling/exercise-context.js
  • demos/cedarling-js-examples/react-nodejs/backend/cedarling/exercise-issuers.js
  • demos/cedarling-js-examples/react-nodejs/backend/cedarling/exercise-lifecycle.js
  • demos/cedarling-js-examples/react-nodejs/backend/cedarling/exercise-logs.js
  • demos/cedarling-js-examples/react-nodejs/backend/cedarling/index.js
  • demos/cedarling-js-examples/react-nodejs/backend/cedarling/init.js
  • demos/cedarling-js-examples/react-nodejs/backend/package.json
  • demos/cedarling-js-examples/react-nodejs/backend/server.js
  • demos/cedarling-js-examples/react-nodejs/backend/server.test.js
  • demos/cedarling-js-examples/react-nodejs/frontend/README.md
  • demos/cedarling-js-examples/react-nodejs/frontend/index.html
  • demos/cedarling-js-examples/react-nodejs/frontend/package.json
  • demos/cedarling-js-examples/react-nodejs/frontend/src/App.tsx
  • demos/cedarling-js-examples/react-nodejs/frontend/src/cedarling/exercise-signed.ts
  • demos/cedarling-js-examples/react-nodejs/frontend/src/cedarling/exercise-unsigned.ts
  • demos/cedarling-js-examples/react-nodejs/frontend/src/cedarling/init.ts
  • demos/cedarling-js-examples/react-nodejs/frontend/src/cedarling/permissions.test.ts
  • demos/cedarling-js-examples/react-nodejs/frontend/src/cedarling/permissions.ts
  • demos/cedarling-js-examples/react-nodejs/frontend/src/index.css
  • demos/cedarling-js-examples/react-nodejs/frontend/src/main.tsx
  • demos/cedarling-js-examples/react-nodejs/frontend/src/model.ts
  • demos/cedarling-js-examples/react-nodejs/frontend/src/oidc.test.ts
  • demos/cedarling-js-examples/react-nodejs/frontend/src/oidc.ts
  • demos/cedarling-js-examples/react-nodejs/frontend/src/task-api.test.ts
  • demos/cedarling-js-examples/react-nodejs/frontend/src/task-api.ts
  • demos/cedarling-js-examples/react-nodejs/frontend/src/vite-env.d.ts
  • demos/cedarling-js-examples/react-nodejs/frontend/tailwind.config.js
  • demos/cedarling-js-examples/react-nodejs/frontend/tsconfig.json
  • demos/cedarling-js-examples/scripts/install-example.mjs
  • demos/cedarling-js-examples/scripts/install.mjs
  • demos/cedarling-js-examples/vercel-nextjs/README.md
  • demos/cedarling-js-examples/vercel-nextjs/app/api/cedarling/exercises/route.ts
  • demos/cedarling-js-examples/vercel-nextjs/app/api/check-edge/route.ts
  • demos/cedarling-js-examples/vercel-nextjs/app/api/check/route.ts
  • demos/cedarling-js-examples/vercel-nextjs/app/api/config/route.ts
  • demos/cedarling-js-examples/vercel-nextjs/app/api/oidc/callback/route.ts
  • demos/cedarling-js-examples/vercel-nextjs/app/api/oidc/logout/callback/route.ts
  • demos/cedarling-js-examples/vercel-nextjs/app/api/oidc/logout/route.ts
  • demos/cedarling-js-examples/vercel-nextjs/app/api/oidc/session/route.ts
  • demos/cedarling-js-examples/vercel-nextjs/app/api/oidc/start/route.ts
  • demos/cedarling-js-examples/vercel-nextjs/app/api/tasks/[id]/route.ts
  • demos/cedarling-js-examples/vercel-nextjs/app/api/tasks/route.ts
  • demos/cedarling-js-examples/vercel-nextjs/app/globals.css
  • demos/cedarling-js-examples/vercel-nextjs/app/layout.tsx
  • demos/cedarling-js-examples/vercel-nextjs/app/page.tsx
  • demos/cedarling-js-examples/vercel-nextjs/app/task-api.ts
  • demos/cedarling-js-examples/vercel-nextjs/libs/cedarling/authorize.ts
  • demos/cedarling-js-examples/vercel-nextjs/libs/cedarling/exercises.ts
  • demos/cedarling-js-examples/vercel-nextjs/libs/cedarling/init.ts
  • demos/cedarling-js-examples/vercel-nextjs/libs/demo-domain.ts
  • demos/cedarling-js-examples/vercel-nextjs/libs/oidc/auth.ts
  • demos/cedarling-js-examples/vercel-nextjs/libs/oidc/provider.ts
  • demos/cedarling-js-examples/vercel-nextjs/libs/oidc/session.ts
  • demos/cedarling-js-examples/vercel-nextjs/libs/permission-check.ts
  • demos/cedarling-js-examples/vercel-nextjs/libs/tasks.ts
  • demos/cedarling-js-examples/vercel-nextjs/next.config.ts
  • demos/cedarling-js-examples/vercel-nextjs/package.json
  • demos/cedarling-js-examples/vercel-nextjs/playwright.config.ts
  • demos/cedarling-js-examples/vercel-nextjs/tailwind.config.ts
  • demos/cedarling-js-examples/vercel-nextjs/tests/e2e/oidc.spec.ts
  • demos/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

Comment thread demos/cedarling-js-examples/common/policies/create-token.cedar
Comment on lines +34 to +41
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;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 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.

Suggested change
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

Comment on lines +51 to +52
if (process.env.MAIN_ARGS) args.push("--", process.env.MAIN_ARGS);
spawn("npm", args, { shell: true, stdio: "inherit" })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 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

Comment on lines 28 to 31
beforeEach(() => {
mockAuthorizeAction.mockReset();
mockAuthorizeAction.mockResolvedValue({ allowed: true });
mockAuthorizeAction.mockResolvedValue("allowed");
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 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.

Comment on lines +50 to +55
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/);
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 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.

Comment on lines +15 to +17
return outcome.signed
? ({ status: 401, error: "Invalid or expired signed identity" } as const)
: ({ status: 503, error: "Authorization service unavailable" } as const);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 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.

Comment on lines +23 to +24
const response = await fetch(`${issuer}/config/cedarling`);
if (!response.ok) throw new Error(`Failed to load Cedarling config: HTTP ${response.status}`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 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.

Suggested change
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.

Comment on lines +21 to +31
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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 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 expectedSubject so the two stored tokens must agree, as app/api/oidc/callback/route.ts already does at login.
  • Bind the session to a server-side registration record and resolve the expected clientId from 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.

Comment on lines +133 to +142
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);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 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.

Comment on lines 29 to +34
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: "/",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 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.

Suggested change
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.

@olehbozhok

Copy link
Copy Markdown
Contributor

demos/cedarling-js-examples/react-nodejs/backend/cedarling/authz-middleware.js:10-19,41,74,92

function identity(req) {
  const userId = req.get("x-user-id");
  ...
}

Even in signed mode, owner/req.userId come from the client-supplied
x-user-id header (line 41, line 74's context.userId, line 92), not from
the verified token subject that authorizeMultiIssuer resolves internally.
The header and the token subject are never cross-checked, so a request can
carry a valid signed token for one user while asserting a different
x-user-id. server.js:93 then persists the task with
owner: req.userId, i.e. the spoofed header value.

Concretely reachable from the UI: in frontend/src/App.tsx, the user
dropdown is only disabled while signedMode is true (line 188). Switch to
unsigned mode, pick a different user, click "Use signed UserInfo" again
(onClick={() => setSignedMode(true)}, line 221) — currentUser is never
reset to signedSession.userId, so the app sends the old session's
Authorization bearer token together with the newly picked x-user-id.

Suggest deriving owner/userId from the verified token subject when a
token is present, and only falling back to x-user-id in unsigned mode.


demos/cedarling-js-examples/react-nodejs/backend/server.test.js,
demos/cedarling-js-examples/react-nodejs/frontend/src/cedarling/permissions.test.ts,
demos/cedarling-js-examples/electron/src/__tests__/ipc.test.ts

None of these suites exercise an explicit decision: false (owner-mismatch)
response end-to-end. Coverage stops at "SDK call failed → treated as deny";
the actual policy-deny branch (authz-middleware.js:89-91, the electron
authorizeForUser mismatch check at main/ipc.ts:115-117) is untested.
Given this PR's purpose is hardening the authorization boundary, a small
test asserting the 403 / decision:false path in each example would close
the main blind spot the change is meant to address.


demos/cedarling-js-examples/common/policies/create-token.cedar,
demos/cedarling-js-examples/common/policies/modify-token.cedar

Behavior change worth calling out explicitly (not just implied by the
README): the old viewAndCreate-token policy let any holder of a valid
signed token create a task for any owner; these new policies require
resource.owner to match the token's sub tag for the signed path too.
Looks intentional given the PR title, but since it's a policy semantics
change and not just a file split, it'd help reviewers to say so in the PR
description rather than leaving it to be inferred from the diff.

- 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp-jans-cedarling Touching folder /jans-cedarling kind-feature Issue or PR is a new feature request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants