fix(core,content): harden public-viewer resolver + agent/desktop/mail polish#526
Merged
fix(core,content): harden public-viewer resolver + agent/desktop/mail polish#526
Conversation
…ent toggle icon + desktop webview chunk reload + mail layout polish
✅ Deploy Preview for agent-native-design ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for agent-native-voice ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for agent-native-meeting-notes ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…lips share-dialog/regenerate-title + calendar root tweaks
This comment has been minimized.
This comment has been minimized.
✅ Deploy Preview for agent-native-starter canceled.
|
✅ Deploy Preview for agent-native-scheduling ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
✅ Deploy Preview for nutritrack-daily-calories canceled.
|
✅ Deploy Preview for agent-native-dispatch canceled.
|
✅ Deploy Preview for agent-native-issues canceled.
|
✅ Deploy Preview for agent-native-forms canceled.
|
✅ Deploy Preview for agent-native-slides canceled.
|
…active and rows lack org_id
Contributor
There was a problem hiding this comment.
Builder reviewed your changes — looks good ✅
Review Details
Code Review Summary — Blocking Security Issues Remain
Incremental Review Result: I've verified the latest commit and confirmed that all three critical security issues from the previous review remain unfixed. These are authentication/authorization vulnerabilities in the public-viewer resolver.
Risk Assessment: HIGH — Blocking merge until security issues are resolved.
🔴 THREE CRITICAL ISSUES (Still Unfixed)
All in templates/content/server/lib/public-documents.ts:
1. Origin validation fails open when host header is missing (Line 28)
- Current:
if (appOrigin && url.origin !== appOrigin) return null; - Problem: When host header is missing/stripped,
appOriginbecomes null and validation is completely skipped - Attack: Attacker removes host header → origin check fails open → unauthorized document access
- Fix: Change to
if (!appOrigin || url.origin !== appOrigin) return null;
2. Path validation uses permissive endsWith() (Lines 75-76)
- Current: Uses both
===ANDendsWith("/_agent-native/builder/callback") - Problem:
endsWith()accepts paths like/evil/_agent-native/builder/callback - Attack: Attacker crafts path with arbitrary prefix → bypasses exact path check → reuses OAuth callback
- Fix: Remove
endsWith(), keep only exact equality:pathOnly === "/_agent-native/builder/callback"
3. UUID validation regex accepts malformed formats (Line 77)
- Current:
/^[0-9a-f-]{36}$/imatches any 36 chars of hex/dashes - Problem: Accepts invalid UUIDs like
----ffff----...instead of strict RFC 4122 format - Risk: Cookie validation is weakened; malformed IDs bypass identity checks
- Fix: Use strict format:
/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i
✅ NEW WORK IN THIS COMMIT (Good)
- Database scoping fix (
scoping.ts,exec.ts): Correctly implements(org_id = X OR org_id IS NULL)to preserve legacy personal rows when org context is active. ✓ Secure. - Builder token TTL (
useBuilderStatus.ts): Defensive improvement that discards tokens >9 minutes old. ✓ Sound. - Route chunk recovery tests: Comprehensive coverage of desktop suppression logic. ✓ Correct.
Summary
The core security hardening for public-viewer is incomplete. All three validation issues must be fixed in public-documents.ts before this PR can be approved. The new database scoping and TTL tracking work is good, but cannot unblock this PR while the authentication vulnerabilities remain.
🧪 Browser testing: Skipped — backend security changes only.
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.
Summary
Changesets
🤖 Generated with Claude Code