Chore/remove step up from session flow - #168
Merged
Merged
Conversation
Contributor
|
Warning Review limit reached
Next review available in: 5 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesThe PR removes automatic step-up authentication from session refresh, bootstrap, middleware, API, and socket authorization. It deletes challenge services, routes, UI, and the Session authentication simplification
Web database warmup
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant WebAuth
participant refreshService
participant SessionRepository
Client->>WebAuth: request authenticated resource
WebAuth->>refreshService: refresh expired access token
refreshService->>SessionRepository: validate and rotate session token
SessionRepository-->>refreshService: active session
refreshService-->>WebAuth: refreshed tokens
WebAuth-->>Client: retry authenticated request
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@apps/socket/tests/internal-web-bridge.test.ts`:
- Around line 23-39: Update the test around both postToInternalWebApi calls to
store their returned values, then assert each result equals { allowed: true,
role: "user" }. Keep the existing call-count and requested-path assertions
unchanged.
In `@apps/web/instrumentation.node.mjs`:
- Around line 1-3: Update the module-loading comment in the file header to state
that apps/web/instrumentation.ts loads this module via new Function, removing
the inaccurate webpackIgnore claim while preserving the explanation that this
prevents Next/webpack from bundling mongoose.
In `@apps/web/instrumentation.ts`:
- Around line 1-6: The nodeModuleFileUrl function in apps/web/instrumentation.ts
must stop deriving module paths from process.cwd(); use a stable startup root or
module-relative resolution so the warmup import consistently targets the sibling
module regardless of the launch directory. Apply the corresponding resolution
update in apps/web/instrumentation.node.mjs at lines 8-10; both sites must use
the same CWD-independent behavior.
In `@docs/ARCHITECTURE.md`:
- Around line 164-167: In the “Internal — socket → web authorization” section,
update the sentence beginning “These are how” to “These endpoints let the
stateless socket server make trust decisions,” leaving the endpoint list and
surrounding documentation unchanged.
In `@docs/architecture/authentication-and-session-model.md`:
- Around line 157-175: Update the new Markdown code fences around the
access-token and invalid-refresh flows to include the text language identifier,
ensuring both opening fences use ```text and the document passes MD040.
- Line 456: Restore the route-level CSRF caveat in the authentication and
session model documentation, explicitly noting that cookie-authenticated API
POST/PUT/PATCH/DELETE handlers lack demonstrated CSRF token validation or
equivalent origin checks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: aed28d4f-07bd-4afa-8cd1-b035aaf8868f
📒 Files selected for processing (46)
.changeset/remove-step-up-from-session-flow.mdapps/socket/tests/internal-web-bridge.test.tsapps/web/app/api/auth/challenge/otp/send/route.tsapps/web/app/api/auth/challenge/otp/verify/route.tsapps/web/app/api/auth/challenge/password/route.tsapps/web/app/api/auth/refresh/route.tsapps/web/app/api/internal/auth/step-up-status/route.tsapps/web/app/auth/challenge/ChallengeForm.tsxapps/web/app/auth/challenge/page.tsxapps/web/app/login/page.tsxapps/web/instrumentation.node.mjsapps/web/instrumentation.tsapps/web/lib/auth/authBootstrap.tsapps/web/lib/utils/api.tsapps/web/lib/utils/auth/client-session.tsapps/web/middleware.tsapps/web/test/auth-bootstrap.no-step-up.test.tsapps/web/test/authorize-conversation-access.route.test.tsapps/web/test/client-session.refresh.test.tsapps/web/test/refresh-flow.integration.test.tsdocs/ARCHITECTURE.mddocs/architecture/authentication-and-session-model.mddocs/architecture/shared-package-design.mddocs/operations/PRODUCTION_REQUIREMENTS.mddocs/operations/SECURITY_AUDIT.mdpackages/auth/__tests__/e2e/auth-lifecycle.e2e.test.tspackages/auth/__tests__/helpers/factories/request-context.factory.tspackages/auth/__tests__/helpers/factories/session.factory.tspackages/auth/__tests__/helpers/factories/step-up-challenge.factory.tspackages/auth/__tests__/integration/repositories/session.repo.test.tspackages/auth/__tests__/integration/services/refresh.service.test.tspackages/auth/__tests__/integration/services/step-up-otp.service.test.tspackages/auth/__tests__/integration/services/step-up-password.service.test.tspackages/auth/__tests__/step-up-flow.integration.test.tspackages/auth/errors/auth-errors.tspackages/auth/index.tspackages/auth/repositories/authEventModel.tspackages/auth/repositories/session.repo.tspackages/auth/repositories/sessionModel.tspackages/auth/services/list-auth-events.service.tspackages/auth/services/refresh.service.tspackages/auth/services/security-event-logger.tspackages/auth/services/step-up-otp.service.tspackages/auth/services/step-up-password.service.tspackages/auth/session/fingerprint.tspackages/db/models/StepUpChallenge.ts
💤 Files with no reviewable changes (21)
- packages/auth/errors/auth-errors.ts
- apps/web/app/api/internal/auth/step-up-status/route.ts
- apps/web/app/api/auth/challenge/otp/verify/route.ts
- packages/auth/tests/integration/services/step-up-password.service.test.ts
- apps/web/app/api/auth/challenge/password/route.ts
- apps/web/app/api/auth/refresh/route.ts
- packages/auth/services/step-up-otp.service.ts
- packages/auth/tests/integration/services/step-up-otp.service.test.ts
- packages/auth/services/step-up-password.service.ts
- apps/web/app/api/auth/challenge/otp/send/route.ts
- packages/auth/tests/helpers/factories/step-up-challenge.factory.ts
- apps/web/app/auth/challenge/ChallengeForm.tsx
- packages/db/models/StepUpChallenge.ts
- apps/web/app/auth/challenge/page.tsx
- packages/auth/services/security-event-logger.ts
- packages/auth/index.ts
- packages/auth/tests/step-up-flow.integration.test.ts
- apps/web/lib/utils/api.ts
- packages/auth/tests/helpers/factories/session.factory.ts
- apps/web/app/login/page.tsx
- apps/web/lib/auth/authBootstrap.ts
Comment on lines
+23
to
+39
| await postToInternalWebApi({ | ||
| path: "/api/internal/socket/authorize-identity", | ||
| body: { userId: "u1", tokenVersion: 0 }, | ||
| timeoutMs: 1_000, | ||
| }); | ||
| await postToInternalWebApi({ | ||
| path: "/api/internal/socket/authorize-conversation-access", | ||
| body: { userId: "u1", conversationId: "c1" }, | ||
| timeoutMs: 1_000, | ||
| }); | ||
|
|
||
| assert.equal(fetchMock.mock.callCount(), 2); | ||
| for (const url of requestedPaths) { | ||
| assert.equal(url.includes("step-up"), false); | ||
| assert.equal(url.includes("/auth/challenge"), false); | ||
| assert.equal(url.includes("/api/auth/challenge"), false); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert both authorization responses.
postToInternalWebApi returns null when the bridge cannot produce a usable response. Lines 23-32 discard both results. The test can pass after two failed authorization calls if the bridge returns null.
Store both results and assert the expected { allowed: true, role: "user" } response.
Proposed fix
- await postToInternalWebApi({
+ const identityResult = await postToInternalWebApi({
path: "/api/internal/socket/authorize-identity",
body: { userId: "u1", tokenVersion: 0 },
timeoutMs: 1_000,
});
- await postToInternalWebApi({
+ const conversationResult = await postToInternalWebApi({
path: "/api/internal/socket/authorize-conversation-access",
body: { userId: "u1", conversationId: "c1" },
timeoutMs: 1_000,
});
+ assert.deepEqual(identityResult, { allowed: true, role: "user" });
+ assert.deepEqual(conversationResult, { allowed: true, role: "user" });
assert.equal(fetchMock.mock.callCount(), 2);📝 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
| await postToInternalWebApi({ | |
| path: "/api/internal/socket/authorize-identity", | |
| body: { userId: "u1", tokenVersion: 0 }, | |
| timeoutMs: 1_000, | |
| }); | |
| await postToInternalWebApi({ | |
| path: "/api/internal/socket/authorize-conversation-access", | |
| body: { userId: "u1", conversationId: "c1" }, | |
| timeoutMs: 1_000, | |
| }); | |
| assert.equal(fetchMock.mock.callCount(), 2); | |
| for (const url of requestedPaths) { | |
| assert.equal(url.includes("step-up"), false); | |
| assert.equal(url.includes("/auth/challenge"), false); | |
| assert.equal(url.includes("/api/auth/challenge"), false); | |
| } | |
| const identityResult = await postToInternalWebApi({ | |
| path: "/api/internal/socket/authorize-identity", | |
| body: { userId: "u1", tokenVersion: 0 }, | |
| timeoutMs: 1_000, | |
| }); | |
| const conversationResult = await postToInternalWebApi({ | |
| path: "/api/internal/socket/authorize-conversation-access", | |
| body: { userId: "u1", conversationId: "c1" }, | |
| timeoutMs: 1_000, | |
| }); | |
| assert.deepEqual(identityResult, { allowed: true, role: "user" }); | |
| assert.deepEqual(conversationResult, { allowed: true, role: "user" }); | |
| assert.equal(fetchMock.mock.callCount(), 2); | |
| for (const url of requestedPaths) { | |
| assert.equal(url.includes("step-up"), false); | |
| assert.equal(url.includes("/auth/challenge"), false); | |
| assert.equal(url.includes("/api/auth/challenge"), false); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/socket/tests/internal-web-bridge.test.ts` around lines 23 - 39, Update
the test around both postToInternalWebApi calls to store their returned values,
then assert each result equals { allowed: true, role: "user" }. Keep the
existing call-count and requested-path assertions unchanged.
Comment on lines
+1
to
+6
| function nodeModuleFileUrl(relativeToCwd: string): string { | ||
| const absolute = `${process.cwd()}/${relativeToCwd}`.replace(/\\/g, "/"); | ||
| return absolute.startsWith("/") | ||
| ? `file://${absolute}` | ||
| : `file:///${absolute}`; | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 \
'next (dev|start)|standalone/server\.js|WORKDIR|process\.cwd|outputFileTracing' \
--glob 'package.json' \
--glob 'Dockerfile*' \
--glob '*.yml' \
--glob '*.yaml' \
--glob '*.ts' \
--glob '*.mjs' . || true
for path in apps/web/instrumentation.node.mjs packages/db/dist/db.js; do
if [ -f "$path" ]; then
echo "present: $path"
else
echo "missing: $path"
fi
doneRepository: harshlocham/semantask
Length of output: 3499
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tracked Docker/workflow deploy start files =="
git ls-files | rg '(^|/)(Dockerfile.*|[.]?(github/workflows/.*\.ya?ml))|server\.js|start.*\.tsx|standalone|package-lock\.json|pnpm-lock\.yaml|bun\.lockb|yarn\.lock|\.next' || true
echo
echo "== next config files =="
for f in $(git ls-files '*/next.config.*' 'next.config.*'); do
echo "--- $f"
sed -n '1,180p' "$f"
done
echo
echo "== root scripts/config =="
for f in package.json pnpm-workspace.yaml turbo.json .github/workflows/*.yml .github/workflows/*.yaml; do
[ -f "$f" ] || continue
echo "--- $f"
sed -n '1,220p' "$f"
done
echo
echo "== files importing instrumentation.node.mjs or instrumenting warmup =="
rg -n -C 3 'instrumentation\.node\.mjs|nodeModuleFileUrl|node --import|--import|node --experimental-import-meta-resolve|export async function register|register' --glob '*.ts' --glob '*.mjs' --glob '*.js' --glob '*.tsx' . || true
echo
echo "== relevant app/web files =="
sed -n '1,220p' apps/web/instrumentation.ts
wc -l apps/web/instrumentation.node.mjs
sed -n '1,80p' apps/web/instrumentation.node.mjsRepository: harshlocham/semantask
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== all non-test references to apps/web/instrumentation file/name =="
git ls-files | grep -vE '(/tests/|tests/)' | xargs -r rg -n 'apps/web/instrumentation|instrumentation\.ts|nodeModuleFileUrl|\.\././\.\./instrumentation\.node\.mjs' || true
echo
echo "== all direct references to relativeWarmup path =="
rg -n -C 3 'relativeWarmup|\.\/instrumentation\.node\.mjs|node --import|--import' --glob '*.ts' --glob '*.mjs' --glob '*.js' --glob '*.tsx' --exclude-dir node_modules --exclude-dir .next . || true
echo
echo "== deterministic cwd resolution for root and web working dirs =="
node - <<'JS'
const path = require('node:path');
const fs = require('node:fs');
function nodeModuleFileUrl(relativeToCwd) {
const absolute = `${process.cwd()}/${relativeToCwd}`.replace(/\\/g, "/");
return absolute.startsWith("/")
? `file://${absolute}`
: `file:///${absolute}`;
}
const cases = {
root: [
process.cwd(),
path.resolve(process.cwd(), 'apps/web/instrumentation.ts'),
path.resolve(process.cwd(), '../../packages/db/dist/db.js'),
],
appsWeb: [
path.resolve(process.cwd(), 'apps/web'),
path.resolve(process.cwd(), 'apps/web/instrumentation.ts'),
path.resolve(process.cwd(), '../../packages/db/dist/db.js'),
],
};
for (const [name, root] of Object.entries(cases)) {
process.chdir(root);
console.log(name, JSON.stringify({
cwd: process.cwd(),
nodeModuleUrl: nodeModuleFileUrl('instrumentation.ts'),
dbEntry: `file://${path.resolve(process.cwd(), '../../packages/db/dist/db.js')}`
}));
}
JSRepository: harshlocham/semantask
Length of output: 1464
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== deterministic cwd resolution for root and web working dirs =="
node - <<'JS'
const path = require('node:path');
const fs = require('node:fs');
function nodeModuleFileUrl(relativeToCwd) {
const absolute = `${process.cwd()}/${relativeToCwd}`.replace(/\\/g, "/");
return absolute.startsWith("/")
? `file://${absolute}`
: `file:///${absolute}`;
}
const root = process.cwd();
const appRoot = path.resolve(root, 'apps/web');
const cases = {
root: {
cwd: root,
nodeModuleUrl: nodeModuleFileUrl('instrumentation.ts'),
dbEntry: `file://${path.resolve(root, '../../packages/db/dist/db.js')}`,
},
appsWeb: {
cwd: appRoot,
nodeModuleUrl: nodeModuleFileUrl('instrumentation.ts'),
dbEntry: `file://${path.resolve(appRoot, '../../packages/db/dist/db.js')}`,
},
};
for (const name of ['root', 'appsWeb']) {
process.chdir(cases[name].cwd);
console.log(name, JSON.stringify({
cwd: process.cwd(),
nodeModuleUrl: nodeModuleFileUrl('instrumentation.ts'),
dbEntry: cases[name].dbEntry,
}));
}
process.chdir(root);
JS
echo
echo "== inspect apps/web/instrumentation.ts relevant import path =="
sed -n '30,55p' apps/web/instrumentation.ts
echo
echo "== check whether packages/db/dist/db.js exists in committed tree =="
node - <<'JS'
const fs = require('node:fs');
const root = require('node:process').cwd();
for (const p of [
'packages/db/dist/db.js',
'packages/db/package.json',
'apps/web/instrumentation.ts'
]) {
const abs = require('node:path').resolve(root, p);
console.log(p, require('node:fs').existsSync(abs) ? 'exists' : 'missing');
}
JSRepository: harshlocham/semantask
Length of output: 1567
Remove the working-directory dependency from apps/web/instrumentation.ts.
The warmup import is only reached from this instrumentation entry point and currently resolves only when Node starts in apps/web; another CWD makes it resolve to apps/web/../instrumentation.ts and miss the sibling warmup module. Use a known startup root or module-relative resolution instead of process.cwd().
📍 Affects 2 files
apps/web/instrumentation.ts#L1-L6(this comment)apps/web/instrumentation.node.mjs#L8-L10
🤖 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 `@apps/web/instrumentation.ts` around lines 1 - 6, The nodeModuleFileUrl
function in apps/web/instrumentation.ts must stop deriving module paths from
process.cwd(); use a stable startup root or module-relative resolution so the
warmup import consistently targets the sibling module regardless of the launch
directory. Apply the corresponding resolution update in
apps/web/instrumentation.node.mjs at lines 8-10; both sites must use the same
CWD-independent behavior.
Comment on lines
164
to
+167
| - **Internal — socket → web authorization** (`api/internal/socket/*`): | ||
| `authorize-identity`, `authorize-conversation-access`, `authorize-message-action`; plus | ||
| `api/internal/auth/step-up-status`. These are how the stateless socket server makes trust decisions. | ||
| `authorize-identity`, `authorize-conversation-access`, `authorize-message-action`, | ||
| `presence-peers`. These are how the stateless socket server makes trust decisions (no | ||
| step-up-status bridge). |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the sentence on Line 165.
“These are how the stateless socket server makes trust decisions” is ungrammatical. Use “These endpoints let the stateless socket server make trust decisions.”
🧰 Tools
🪛 LanguageTool
[grammar] ~165-~165: Ensure spelling is correct
Context: ...on-access, authorize-message-action, presence-peers`. These are how the stateless socket ser...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 `@docs/ARCHITECTURE.md` around lines 164 - 167, In the “Internal — socket → web
authorization” section, update the sentence beginning “These are how” to “These
endpoints let the stateless socket server make trust decisions,” leaving the
endpoint list and surrounding documentation unchanged.
Source: Linters/SAST tools
Comment on lines
+157
to
175
| ``` | ||
| access token expires / missing | ||
| → valid refresh token | ||
| → verifySession + user/tokenVersion checks | ||
| → rotate refresh token hash (state → active) | ||
| → issue new access + refresh tokens | ||
| → continue | ||
| ``` | ||
|
|
||
| **Invalid / revoked / expired refresh** | ||
|
|
||
| ``` | ||
| invalid or revoked refresh token | ||
| → authentication failure (401) | ||
| → client clears session / redirects to login | ||
| → login recovery | ||
| ``` | ||
|
|
||
| ``` |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to the new Markdown fences.
markdownlint-cli2 reports MD040 on Lines 157, 168, and 175. Use text for the flow diagrams so the document passes the configured Markdown checks.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 157-157: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 168-168: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 175-175: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/architecture/authentication-and-session-model.md` around lines 157 -
175, Update the new Markdown code fences around the access-token and
invalid-refresh flows to include the text language identifier, ensuring both
opening fences use ```text and the document passes MD040.
Source: Linters/SAST tools
| user-facing route handlers in `apps/web/app/api/auth/*` add cookie | ||
| handling, CSRF, and rate limiting (rate limiting was not | ||
| exhaustively traced). | ||
| handling and rate limiting (rate limiting was not exhaustively traced). |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
fd -t f -e ts -e tsx . apps/web/app/api -0 |
xargs -0 rg -n -C 3 \
'export async function (POST|PUT|PATCH|DELETE)|cookies\(|csrf|CSRF|Origin|Referer|sameSite'Repository: harshlocham/semantask
Length of output: 28089
🏁 Script executed:
#!/usr/bin/env bash1
set -euo pipefail
echo "== markdown line =="
sed -n '440,462p' docs/architecture/authentication-and-session-model.md | cat -n -v
echo
echo "== route-level auth/origin checks near relevant APIs =="
for f in \
apps/web/app/api/tasks/route.ts \
apps/web/app/api/tasks/[id]/route.ts \
apps/web/app/api/tasks/[id]/request-execution/route.ts \
apps/web/app/api/tasks/[id]/cancel/route.ts \
apps/web/app/api/messages/route.ts \
apps/web/app/api/messages/[id]/edit/route.ts \
apps/web/app/api/messages/[id]/delete/route.ts \
apps/web/app/api/messages/[id]/delivered/route.ts \
apps/web/app/api/messages/[id]/semantic/route.ts \
apps/web/app/api/conversations/route.ts \
apps/web/app/api/organizations/route.ts \
apps/web/app/api/organizations/[id]/route.ts \
apps/web/app/api/organizations/[id]/members/route.ts \
apps/web/app/api/organizations/[id]/policy/route.ts \
apps/web/app/api/organizations/[id]/quota/route.ts \
apps/web/app/api/work-suggestions/[id]/accept/route.ts \
apps/web/app/api/work-suggestions/[id]/assign/route.ts \
apps/web/app/api/work-suggestions/[id]/dismiss/route.ts \
apps/web/app/api/auth/logout/route.ts \
apps/web/app/api/auth/revoke-all-tokens/route.ts \
apps/web/app/api/admin/tool-grants/route.ts \
apps/web/app/api/admin/tool-grants/[id]/route.ts \
apps/web/app/api/admin/toggleban/route.ts \
apps/web/app/api/admin/changeRoal/route.ts \
apps/web/app/api/admin/assignments/route.ts \
apps/web/app/api/auth/refresh/route.ts \
apps/web/app/api/updateImage/route.ts
do
[ -f "$f" ] || continue
echo "--- $f ---"
rg -n -C 3 \
'export async function (POST|PUT|PATCH|DELETE|GET|HEAD|OPTIONS|POST|PUT)$\
export async function GET\
export async function HEAD\
export async function OPTIONS\
csrf|CSRF\
Origin|Origin:"\s*\
OriginHeader\
Origin |Referer|sameSite|cookies\(|requireAuthUser|requireAdminUser|AuthorizationError' \
"$f" || true
done
echo
echo "== auth guard and origin helpers =="
rg -n -C 4 \
'export (async function|function) (requireAuthUser|requireAdminUser|isSameSite|checkOrigin|verifyCsrf|Origin|origin|csrf|CSRF|sameSite)' \
apps/web/lib apps/web/models apps/web/app/api || trueRepository: harshlocham/semantask
Length of output: 9392
🏁 Script executed:
#!/usr/bin/env bash
set -edu pipefail
echo "== changed files/stat =="
git status --short
git diff --stat || true
echo
echo "== docs line context =="
sed -n '448,462p' docs/architecture/authentication-and-session-model.md | cat -n -v
echo
echo "== changed docs lines =="
git diff HEAD -- docs/architecture/authentication-and-session-model.md | sed -n '/authentication-and-session-model.md/{p}; /Uncertain/{p;N;p}' || true
echo
echo "== require guards/source =="
for f in \
apps/web/lib/utils/auth/requireAuthUser.ts \
apps/web/lib/utils/auth/requireAdminUser.ts \
apps/web/lib/utils/auth/getAuthUser.ts \
apps/web/lib/utils/auth/validateAuthUser.ts
do
[ -f "$f" ] || continue
echo "--- $f ---"
cat -n -v "$f"
done
echo
echo "== API handler auth usage, excluding internal secret routes and google callback state handling =="
python3 - <<'PY'
import os, re
root="apps/web/app/api"
files=[]
for dirpath, dirnames, filenames in os.walk(root):
for fn in filenames:
if fn == "route.ts":
files.append(os.path.join(dirpath, fn))
for path in files:
text=open(path, "r", encoding='utf-8', errors='replace').read()
if re.search(r'export async function (POST|PUT|PATCH|DELETE)', text):
# include relevant tokens, safe context
for token in ['requireAuthUser', 'requireAdminUser', 'AuthError', 'AuthorizationError', 'cookies(', 'get(' , 'csrf', 'CSRF', 'Origin', 'Referer', 'sameSite']:
if token in text:
print(f"--- {path} has {token!r} ---")
for i,line in enumerate(text.splitlines(), 1):
if token in line:
lo=max(1,i-4); hi=min(len(text.splitlines()), i+4)
for j in range(lo, hi+1):
print(f"{j}: {text.splitlines()[j-1]}")
break
PYRepository: harshlocham/semantask
Length of output: 38957
Keep the route-level CSRF caveat.
The removed item still matters because the API cookie-authenticated POST/PUT/PATCH/DELETE routes do not show route-level CSRF token validation or equivalent origin checks in the handler implementations.
🤖 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 `@docs/architecture/authentication-and-session-model.md` at line 456, Restore
the route-level CSRF caveat in the authentication and session model
documentation, explicitly noting that cookie-authenticated API
POST/PUT/PATCH/DELETE handlers lack demonstrated CSRF token validation or
equivalent origin checks.
harshlocham
force-pushed
the
chore/remove-step-up-from-session-flow
branch
from
August 10, 2026 09:52
f8776b0 to
cdd46de
Compare
harshlocham
force-pushed
the
chore/remove-step-up-from-session-flow
branch
2 times, most recently
from
August 10, 2026 11:25
0c3d916 to
749f03c
Compare
harshlocham
force-pushed
the
chore/remove-step-up-from-session-flow
branch
from
August 10, 2026 12:14
749f03c to
22bc766
Compare
harshlocham
force-pushed
the
chore/remove-step-up-from-session-flow
branch
from
August 10, 2026 12:23
22bc766 to
d9fd9b0
Compare
harshlocham
force-pushed
the
chore/remove-step-up-from-session-flow
branch
from
August 10, 2026 12:31
d9fd9b0 to
d410220
Compare
harshlocham
force-pushed
the
chore/remove-step-up-from-session-flow
branch
2 times, most recently
from
August 10, 2026 12:50
6db5541 to
fc6bf87
Compare
harshlocham
force-pushed
the
chore/remove-step-up-from-session-flow
branch
from
August 10, 2026 12:58
fc6bf87 to
071fe5e
Compare
- Eliminated automatic step-up OTP challenges during session refresh and bootstrap processes. - Removed related UI/API components and the StepUpChallenge model as they are no longer needed. - Updated tests to ensure that the socket authorization bridge does not target step-up challenge endpoints. - Adjusted middleware and API routes to reflect the removal of step-up logic, enhancing overall code clarity and maintainability.
- Eliminated automatic step-up OTP challenges from session refresh and bootstrap processes, streamlining the authentication flow. - Removed the StepUpChallenge model and related UI/API components as they are no longer necessary. - Updated documentation to reflect the new MVP session contract, clarifying that session maintenance does not involve step-up challenges. - Introduced a Node-only Mongo warmup script to optimize database connection handling during application startup. - Adjusted various components and services to ensure compatibility with the updated authentication logic, enhancing overall code clarity and maintainability.
harshlocham
force-pushed
the
chore/remove-step-up-from-session-flow
branch
from
August 10, 2026 13:08
071fe5e to
3e23dda
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several Phase 2 features and hardening improvements, with a focus on Work Inbox, execution approval flows, and improved handling of suggest-only execution modes. It also removes the automatic step-up OTP challenge from session refresh, cleans up related dead code, and updates documentation. The changes include new APIs, UI surfaces, policy gate helpers, and robust test coverage.
Session and Authorization Changes:
Work Inbox & Approvals:
/inboxWork Inbox UI, listing authorized WorkSuggestions with server-side pagination./inbox/approvalsUI, reusing the existing task-approvals API in the Work Inbox.Execution Approval and Suggest-Only Mode:
Backend and Observability Hardening:
Work Suggestion Mutations:
Task.suggestionId.These changes collectively improve the product’s security, auditability, and user experience, especially around work suggestions and execution approval flows.
Summary by CodeRabbit