Repository navigation
Conversation
A signed URL for a workspace PDF carried a folder-scoped claim, so the same token also served other preview files in the PDF's folder and its subfolders until it expired. Only HTML previews keep the folder claim, since a page loads its relative styles, scripts and images. PDFs and every other workspace preview now get an exact-file claim. Refs pingdotgg#9228 Co-authored-by: 404khai <160283456+404khai@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 tasks done
Contributor
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This focused fix changes the access scope of signed workspace-file URLs, narrowing new PDF and non-HTML links from folder-wide access to the linked file while preserving HTML relative assets. Because this is an access-control boundary change with security implications, it requires human review. Notes:
You can add or adjust custom eligibility rules. Learn more. |
This branch has not been deployed
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
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.
Problem
Signed asset links for workspace file previews in
apps/server/src/assets/AssetAccess.tsgave PDFs the folder-wide scope meant for HTML previews. A link opened or shared for one PDF was not limited to that file. As reported in #9228, its scope included other preview files beside it and in nested folders. A PDF link should grant access to the linked file and nothing else.Change
The minting rule now gives folder scope only to HTML previews. PDFs and other supported non-HTML previews receive exact-file claims. HTML keeps folder scope because pages need their relative styles, scripts and images. The existing image-dimension inspection still runs before claim creation.
New tests in
apps/server/src/assets/AssetAccess.test.tscheck that a PDF link opens only its own file. They also check that an HTML page ending in.html,.htm,.HTMLor.HTMstill loads its relative styles, scripts and images.Links minted before this change keep their old scope until they expire; new non-HTML links get the exact-file scope.
Scope and approval
Fixes #9228. This rebuilds closed #9260 on current main, following @shivamhwp's direction to preserve image-dimension inspection and keep folder scope only for HTML. Credit to @404khai for the same approach in #9260.
This qualifies for the small, focused obvious-bug exception. It corrects the scope of an existing file-preview grant with one claim-selection change and regression tests. PDF links still open their own file, and HTML and image previews behave as before.
Verification
vp test run apps/server/src/assets/AssetAccess.test.ts: 43 tests passed. With the old source and new tests, 2 PDF tests failed and 41 passed; restoring the fix made all 43 pass..htmand matching extensions case-sensitively each make 2 of the 4 HTML cases fail.vp test run apps/server/src/assets apps/server/src/http.test.ts apps/server/src/auth/RpcAuthorization.test.ts: 110 tests passed across 6 files.vp run typecheck: exit 0, no errors.An independent reviewer ran the server asset, HTTP and auth suites plus related web, mobile, client-runtime and shared suites: 29 test files and 509 tests passed. That run used the same
AssetAccess.ts, before the HTML test was split into four cases. Manual browser and mobile checks were not run.The screenshot shows the rule before and after the fix and the red/green test results.
Made with Claude Opus 5.5 (code, tests and PR text review, Claude Code harness) and GPT-6.1 Sol (code reviews, PR text and screenshot, Codex harness), all in T3 Code.