Skip to content

Commit 0455b02

Browse files
committed
fix(account-deletion): keep a public object a stored URL with a fragment still names
A relative /api/files/serve/<key>#frag kept the fragment in its candidate key, so a live reference could be missed and the object purged. The guard now also reads the path without its fragment, on both the decoded and fallback paths.
1 parent 2cba4fa commit 0455b02

2 files changed

Lines changed: 28 additions & 4 deletions

File tree

‎apps/sim/lib/users/storage-reference-guard.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ const SERVE_PATH = '/api/files/serve/'
4040
/**
4141
* The storage keys a stored URL can name: a `/api/files/serve/...` path (relative or absolute,
4242
* with or without a provider segment), or a direct bucket URL in virtual-host or path style.
43+
* A relative path is also read without its `#fragment`, which a key may hold literally.
4344
* Over-matching only keeps an object. Never throws: a value whose percent-encoding is malformed
4445
* yields its raw path and that path with each valid escape decoded (a legacy key can hold a
4546
* literal `%`), so one bad stored string cannot disable every purge.
@@ -72,10 +73,13 @@ function keysNamedByUrl(value: string): string[] {
7273
const serveAt = raw.indexOf(SERVE_PATH)
7374
path = serveAt >= 0 ? raw.slice(serveAt + SERVE_PATH.length) : raw
7475
}
75-
path = path.replace(/^\/+/, '')
76-
add(path)
77-
const slash = path.indexOf('/')
78-
if (slash > 0) add(path.slice(slash + 1))
76+
const fragmentAt = path.indexOf('#')
77+
for (const candidate of fragmentAt >= 0 ? [path, path.slice(0, fragmentAt)] : [path]) {
78+
const key = candidate.replace(/^\/+/, '')
79+
add(key)
80+
const slash = key.indexOf('/')
81+
if (slash > 0) add(key.slice(slash + 1))
82+
}
7983
return [...keys]
8084
}
8185

‎apps/sim/lib/users/storage-sweep.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,26 @@ describe('account storage object cleanup', () => {
524524
expect(await exists('workspace-logos/wsforeign9/icon.png')).toBe(true)
525525
})
526526

527+
it.each([
528+
{
529+
key: 'workspace-logos/wsforeign9/icon.png',
530+
url: '/api/files/serve/workspace-logos%2Fwsforeign9%2Ficon.png#v2',
531+
},
532+
{
533+
key: 'workspace-logos/wsforeign9/brand logo%.png',
534+
url: '/api/files/serve/workspace-logos/wsforeign9/brand%20logo%.png#v2',
535+
},
536+
])('keeps a public key a relative URL with a fragment names: $url', async ({ key, url }) => {
537+
const storedAs = key.replace(/[ %]/g, '_')
538+
await put(storedAs)
539+
queueTableRows(schemaMock.user, [])
540+
queueTableRows(schemaMock.user, [{ value: url }])
541+
542+
await cleanup(payload([{ key, context: 'workspace-logos' }]), outboxContext())
543+
544+
expect(await exists(storedAs)).toBe(true)
545+
})
546+
527547
it('keeps a public key without an owner id to narrow on, reading no URL column', async () => {
528548
await put('og-images/logo.png')
529549

0 commit comments

Comments
 (0)