Skip to content

Test/session cookie security regressions#114

Open
ojuotimi932 wants to merge 12 commits into
karagozemin:mainfrom
ojuotimi932:test/session-cookie-security-regressions
Open

Test/session cookie security regressions#114
ojuotimi932 wants to merge 12 commits into
karagozemin:mainfrom
ojuotimi932:test/session-cookie-security-regressions

Conversation

@ojuotimi932

Copy link
Copy Markdown
Contributor

Closes #75

@vercel

vercel Bot commented Jun 29, 2026

Copy link
Copy Markdown

@ojuotimi932 is attempting to deploy a commit to the karagoz's projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Jun 29, 2026

Copy link
Copy Markdown

@ojuotimi932 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@karagozemin

Copy link
Copy Markdown
Owner

Thanks for the session cookie regression PR. I cannot merge it yet because the new test file is not picked up by the repo's Vitest include pattern.

Command run:

npm ci
npm test -- src/modules/auth/__tests__/session-cookie.spec.ts

Result:

No test files found
include: src/**/*.test.ts

The new file is named session-cookie.spec.ts, but the project currently includes src/**/*.test.ts. Please either rename the file to session-cookie.test.ts or update the test configuration intentionally, then confirm:

npm test -- src/modules/auth/__tests__/session-cookie.test.ts
npm run build

@karagozemin

Copy link
Copy Markdown
Owner

Thanks for the rename/update. I rechecked the current branch and the test file is now picked up by Vitest, but it still cannot merge because it fails before running any tests:

npm test -- src/modules/auth/__tests__/session-cookie.test.ts

fails with:

Error: Cannot find package 'express' imported from src/modules/auth/__tests__/session-cookie.test.ts

The test currently depends on express, supertest, and cookie-parser, which are not project dependencies. It also builds a mock Express app rather than exercising Fortexa's existing auth/session code path, so it would not catch regressions in the real implementation.

Please rewrite this against the existing Fortexa auth/session helpers or route handlers already in the app, without adding unrelated server dependencies. Then rerun the focused test and typecheck.

@karagozemin

Copy link
Copy Markdown
Owner

Thanks for the update. I rechecked this under the karagozemin account.

The previous Express dependency direction appears to have been changed, but the PR is currently blocked before tests because whitespace validation fails:

git diff --check origin/main...HEAD
src/lib/auth/session.ts:3: trailing whitespace.
src/lib/auth/session.ts:4: trailing whitespace.
src/lib/auth/session.ts:5: trailing whitespace.
src/lib/auth/session.ts:6: trailing whitespace.
src/lib/auth/session.ts:7: trailing whitespace.
src/lib/auth/session.ts:41: trailing whitespace.
src/lib/auth/session.ts:61: trailing whitespace.
src/modules/auth/__tests__/session-cookie.test.ts:3: trailing whitespace.
src/modules/auth/__tests__/session-cookie.test.ts:4: trailing whitespace.
src/modules/auth/__tests__/session-cookie.test.ts:5: trailing whitespace.
src/modules/auth/__tests__/session-cookie.test.ts:6: trailing whitespace.
src/modules/auth/__tests__/session-cookie.test.ts:7: trailing whitespace.
...

Please remove the trailing whitespace, then rerun:

  • git diff --check origin/main...HEAD
  • npm test -- --run src/modules/auth/__tests__/session-cookie.test.ts
  • npx tsc --noEmit

Once diff check is clean I can continue the functional review.

@ojuotimi932

Copy link
Copy Markdown
Contributor Author

all checks passed. kindly review and merge repo

@karagozemin

Copy link
Copy Markdown
Owner

Thanks for the PR! The build passes locally ✅, but after other PRs landed on main this branch now has merge conflicts and can't be merged as-is. Could you rebase onto the latest main and resolve the conflicts? Once it's mergeable I'll merge right away. 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Session cookie security regression tests

2 participants