Skip to content

Feat/fix cookie store for cross origin#87

Merged
rongquan1 merged 19 commits into
masterfrom
feat/fix-cookie-store-for-cross-origin
May 21, 2026
Merged

Feat/fix cookie store for cross origin#87
rongquan1 merged 19 commits into
masterfrom
feat/fix-cookie-store-for-cross-origin

Conversation

@manishdex25

@manishdex25 manishdex25 commented May 15, 2026

Copy link
Copy Markdown
Contributor

Summary

What is the background of this pull request?

Changes

  • What are the changes made in this pull request?
  • Change this and that, etc...

Issues

What are the related issues or stories?

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Fixed CSRF token response caching behavior to prevent stale token delivery.
    • Updated CORS configuration to properly handle cross-origin requests.
  • Chores

    • Updated core library dependency to the latest stable version.
  • Tests

    • Added regression test for CSRF token delivery with CORS handling.
    • Updated integration tests with latest document specifications.

Review Change Stack

manishdex25 and others added 15 commits May 6, 2026 13:47
…rigin

- Removed Access-Control-Allow-Origin from netlify.toml.
- Added middleware to vary the Origin header in storage and verify functions.
- Included a new integration test to validate CORS response for TradeTrust production website.
- Upgraded @trustvc/trustvc to version 2.12.2.
- Updated @digitalbazaar/di-sd-primitives to version 3.3.0 and its dependencies.
- Changed JSON schema version in document-sepolia.json to 3.0 and updated its structure to align with the new schema.
- Updated @types/node to version 22.19.15.
- Added undici-types version 6.20.0 to the package-lock.json.
- Updated multiple dependencies to their latest versions, including @aws-sdk/client-kms and related packages.
- Added resolved URLs and integrity hashes for several packages to ensure package integrity.
- Modified the npm install command in the GitHub Actions workflow to use the --omit=optional flag, improving installation efficiency by excluding optional dependencies.
- Downgraded @trustvc/trustvc to version 2.0.7 and @types/node to version 22.0.0 in package-lock.json.
- Removed resolved URLs and integrity hashes for several packages to streamline the lock file.
- Updated the npm install command in the GitHub Actions workflow to include all dependencies.
- Changed the npm install command from 'npm ci' to 'npm i' in both manual_forked.yml and manual.yml workflows to streamline dependency installation.
- Removed optional dependencies from package-lock.json to streamline the file.
- Added a new dependency for @netlify/blobs version 6.5.0 in package-lock.json.
- Updated npm install command in manual_forked.yml and manual.yml workflows to include all dependencies.
…npm ci

- Changed the npm install command from 'npm i' to 'npm ci' in both manual_forked.yml and manual.yml workflows for improved consistency and reliability in dependency installation.
- Changed the schema version in document-sepolia.json from 3.0 to 2.0.
- Updated the structure of the document to align with the new schema, including changes to the data fields and template references.
- Adjusted import paths in integration tests to reference the updated document-sepolia.json file.
- Removed import of document-sepolia version 2 and replaced it with version 3 in both document-storage.test.ts and document-verify.test.ts.
- Updated postDataSepoliaV2 to use documentSepoliaV3 for consistency across tests.
- Changed the test description to explicitly mention the origin URL as https://ref.tradetrust.io for clarity and accuracy in the integration tests.
- Removed the "if-none-match" header from the request to prevent 304 responses that omit access-control-allow-origin.
- Added a test to ensure the server returns a 200 status with the correct CORS headers even when the cached ETag matches.
@netlify

netlify Bot commented May 15, 2026

Copy link
Copy Markdown

Deploy Preview for tradetrust-functions ready!

Name Link
🔨 Latest commit 1d26b8c
🔍 Latest deploy log https://app.netlify.com/projects/tradetrust-functions/deploys/6a06c6a80233b300076ea53a
😎 Deploy Preview https://deploy-preview-87--tradetrust-functions.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@manishdex25 manishdex25 requested a review from rongquan1 May 15, 2026 06:35
@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@manishdex25 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 26 minutes and 22 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 114d3668-f4a1-4198-b2db-d7c98193b8d3

📥 Commits

Reviewing files that changed from the base of the PR and between 61558d1 and 1d26b8c.

📒 Files selected for processing (4)
  • netlify.toml
  • netlify/functions/storage/index.ts
  • netlify/functions/storage/router.ts
  • tests/integration/headers.test.ts
📝 Walkthrough

Walkthrough

PR removes public CORS wildcard origin from Netlify configuration, adds Origin-aware Vary headers to serverless handlers, prevents CSRF-token conditional request caching via If-None-Match removal and Cache-Control headers, upgrades the @trustvc/trustvc dependency, and validates changes through test fixture migrations and new CORS regression coverage.

Changes

CORS and CSRF-Token Handling

Layer / File(s) Summary
CORS Configuration and Middleware
netlify.toml, netlify/functions/storage/index.ts, netlify/functions/verify/index.ts
Removes wildcard Access-Control-Allow-Origin header from static config and adds Express res.vary("Origin") middleware to storage and verify handlers to properly signal origin-dependent responses under serverless-http.
CSRF-Token Response Caching
netlify/functions/storage/router.ts
Removes If-None-Match header from incoming requests before CSRF processing and sets Cache-Control: no-store on the response to prevent ETag-based conditional caching.
TrustVC Dependency Upgrade
package.json
Bumps @trustvc/trustvc from ^2.0.7 to ^2.12.2.
Integration Test Fixtures and Regression Coverage
tests/integration/document-storage.test.ts, tests/integration/document-verify.test.ts, tests/integration/headers.test.ts
Migrates document-storage and document-verify tests from v2 to v3 Sepolia fixtures; adds CSRF-token 304 CORS regression test that verifies conditional requests with matching ETags return 200 with proper CORS headers rather than 304; adds CORS allowlist test case for https://ref.tradetrust.io.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • rongquan1
  • RishabhS7

Poem

🐰 A hop toward origin-aware responses,
No wildcard CORS, just careful choices,
CSRF tokens now uncacheable and free,
Vary by Origin for all to see! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description contains only empty template sections with no actual content filled in (Summary, Changes, and Issues sections are placeholders). Complete all description sections: add background context, list specific changes made (CORS config, middleware, test additions), and reference related issues or stories.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Feat/fix cookie store for cross origin' is partially related to the changeset, referring to cross-origin functionality, but inadequately summarizes the main changes (CORS header removal, Origin middleware addition, CSRF token caching fixes).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/fix-cookie-store-for-cross-origin

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 and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@tests/integration/document-storage.test.ts`:
- Line 22: The test fixture alias postDataSepoliaV2 is incorrectly assigned
documentSepoliaV3; update the test to restore a true v2 payload or rename the
alias and related test descriptions to v3 to match the fixture. Specifically,
replace the value of postDataSepoliaV2 to reference the correct v2 fixture
(e.g., documentSepoliaV2) or rename postDataSepoliaV2 to postDataSepoliaV3 and
update any test titles or expectations that reference "v2 sepolia" so they
consistently reflect v3 behavior; search for usages of postDataSepoliaV2 and
documentSepoliaV3 in document-storage.test.ts and update them together to keep
intent and coverage accurate.

In `@tests/integration/document-verify.test.ts`:
- Line 22: The test fixture alias postDataSepoliaV2 is incorrectly assigned
documentSepoliaV3; update the test to either use the correct v2 fixture (e.g.,
replace documentSepoliaV3 with documentSepoliaV2) or rename the alias and any
test labels to v3 for clarity; locate occurrences of postDataSepoliaV2 and
documentSepoliaV3 in the test file and make the alias and test descriptions
consistent (use documentSepoliaV2 for v2 tests or rename to postDataSepoliaV3
and adjust test names).
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: c8032855-172f-427a-8501-9e0cc495a5bc

📥 Commits

Reviewing files that changed from the base of the PR and between a7d041e and 61558d1.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (8)
  • netlify.toml
  • netlify/functions/storage/index.ts
  • netlify/functions/storage/router.ts
  • netlify/functions/verify/index.ts
  • package.json
  • tests/integration/document-storage.test.ts
  • tests/integration/document-verify.test.ts
  • tests/integration/headers.test.ts
💤 Files with no reviewable changes (1)
  • netlify.toml

Comment thread tests/integration/document-storage.test.ts
Comment thread tests/integration/document-verify.test.ts
- Disabled ETag generation in the storage function to prevent 304 responses that omit CORS headers.
- Updated the CSRF token handling in the router to ensure consistent responses with appropriate CORS headers.
- Updated imports and postData for document-sepolia version 2 in both document-storage.test.ts and document-verify.test.ts to use the correct file.
- Ensured consistency in the test data by referencing the appropriate document-sepolia version 2 instead of version 3.
@rongquan1 rongquan1 merged commit 0b98d49 into master May 21, 2026
10 checks passed
@rongquan1 rongquan1 deleted the feat/fix-cookie-store-for-cross-origin branch May 21, 2026 08:50
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.

2 participants