馃攼 fix: Gate Shared Startup Config By Link Access - #13897
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
馃挕 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23cf9a3e82
鈩癸笍 About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 馃憤.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (allowSharedLinks) { | ||
| router.get('/:shareId/config', optionalJwtAuth, canAccessSharedLink, async (_req, res) => { | ||
| try { | ||
| const payload = await getShareStartupPayload(); |
There was a problem hiding this comment.
When a shared link has expired but its ACL entry has not yet been TTL-cleaned, canAccessSharedLink can still call next() because it looks up the raw SharedLink and checks ACL only; the existing message/file paths then re-check activeExpirationFilter in getSharedMessages/getSharedLinkFile, but this new config path immediately returns the payload. That lets /api/share/:shareId/config disclose the gated footer/analytics/bundler config for a link that /api/share/:shareId would 404, so please re-read the active share or add the expiration check before building the payload.
Useful? React with 馃憤聽/ 馃憥.
There was a problem hiding this comment.
Fixed in f46c4bd. The shared-link access middleware now reads only active, non-expired shares with activeExpirationFilter before checking ACL/public access, so /api/share/:shareId/config cannot return config for expired links waiting on TTL cleanup. Added a regression test covering an expired shared link with a still-valid PUBLIC ACL. Verified with: cd packages/api && npx jest src/shared-links/access.test.ts --runInBand --coverage=false; cd api && npx jest server/routes/tests/share.spec.js --runInBand; npx tsc --noEmit -p packages/api/tsconfig.json; eslint/sort-imports/prettier checks for touched files.
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: 鈩癸笍 About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 馃憤. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
* fix: gate shared startup config by link access * fix: satisfy shared config CI checks * fix: align shared config client types * fix: reject expired shared link access
* fix: gate shared startup config by link access * fix: satisfy shared config CI checks * fix: align shared config client types * fix: reject expired shared link access
* fix: gate shared startup config by link access * fix: satisfy shared config CI checks * fix: align shared config client types * fix: reject expired shared link access
1 participant
Summary
Fixes #13881.
I routed shared-conversation startup config through the same shared-link access boundary as shared messages, so private/auth-required shares no longer rely on a loose
context=sharestartup config path.GET /api/share/:shareId/configbehindoptionalJwtAuthandcanAccessSharedLink, returningprivate, no-storeresponses.ShareViewand shared artifact previews to read startup config from the share-scoped endpoint, including Sandpack options forbundlerURLandstaticBundlerURL.GET /api/config?context=share.Change Type
Testing
npm run smart-reinstallcd packages/api && npx jest src/shared-links/config.test.ts --runInBand --coverage=falsecd api && npx jest server/routes/__tests__/config.spec.js server/routes/__tests__/share.spec.js --runInBandcd api && npx jest server/routes/__tests__/share.spec.js --runInBandTest Configuration:
npm cifromnpm run smart-reinstallChecklist