From a2e4b1c1bc5773b770bf07c5d8b89105f478a443 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Wed, 12 Aug 2026 18:41:04 -0400 Subject: [PATCH] fix(fs): expire signed URLs over entries the signer doesn't own MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit signFile defaults to a ~317k-year TTL and verifySignature checks only uid, expires and signature — never the ACL. A recipient who ever signed a shared file therefore held a permanent, revocation-proof URL to its bytes: revoking the share did nothing to it. signEntry now takes the acting user and drops to NON_OWNER_SIGNATURE_TTL_SECONDS (1 hour) when the signer is not the entry's owner. Owners keep the permanent default, so no existing client changes behavior. The signature-authenticated directory listing bounds its children unconditionally: that route has no session actor, and a signature proves possession rather than ownership, so a recipient holding a short-lived directory signature could otherwise mint permanent URLs for every child. A bounded window is not revocation — the durable fix is a per-entry signature epoch folded into the HMAC and bumped on any permission change. --- .../controllers/fs/LegacyFSController.ts | 42 +++++++++++++---- .../controllers/fs/legacyFsHelpers.test.ts | 47 +++++++++++++++++++ src/backend/controllers/fs/legacyFsHelpers.ts | 22 ++++++++- src/backend/util/fileSigning.ts | 10 ++++ 4 files changed, 111 insertions(+), 10 deletions(-) diff --git a/src/backend/controllers/fs/LegacyFSController.ts b/src/backend/controllers/fs/LegacyFSController.ts index 6fbe18c50..d4e1ac734 100644 --- a/src/backend/controllers/fs/LegacyFSController.ts +++ b/src/backend/controllers/fs/LegacyFSController.ts @@ -37,7 +37,10 @@ import type { PuterRouter } from '../../core/http/PuterRouter.js'; import type { ACLService } from '../../services/acl/ACLService.js'; import { assertActorHasCredits } from '../../services/metering/enforcement.js'; import type { SignedFile } from '../../util/fileSigning.js'; -import { verifySignature } from '../../util/fileSigning.js'; +import { + NON_OWNER_SIGNATURE_TTL_SECONDS, + verifySignature, +} from '../../util/fileSigning.js'; import { buildHostedBackingDenial, hostedIndexUrlBackingIsUnavailable, @@ -1353,7 +1356,9 @@ export class LegacyFSController extends PuterController { ); } - const signed = signEntry(entry, signingCfg); + const signed = signEntry(entry, signingCfg, { + actorUserId: actor.user?.id, + }); if (finalAction !== 'write') { const { write_url: _, ...rest } = signed; result.signatures.push({ ...rest, path: entry.path }); @@ -1464,7 +1469,9 @@ export class LegacyFSController extends PuterController { 'outer.gui.item.added', uploadResult.fsEntry, ); - const signed = signEntry(uploadResult.fsEntry, signingCfg); + const signed = signEntry(uploadResult.fsEntry, signingCfg, { + actorUserId: callerActor.user?.id, + }); res.json({ ...signed, path: uploadResult.fsEntry.path }); return; } @@ -1493,7 +1500,12 @@ export class LegacyFSController extends PuterController { dedupeName: true, }); await this.#emitGuiEvent('outer.gui.item.added', entry); - res.json({ ...signEntry(entry, signingCfg), path: entry.path }); + res.json({ + ...signEntry(entry, signingCfg, { + actorUserId: callerActor.user?.id, + }), + path: entry.path, + }); return; } if (operation === 'rename') { @@ -1512,7 +1524,12 @@ export class LegacyFSController extends PuterController { ); const renamed = await this.services.fs.rename(targetEntry, newName); await this.#emitGuiEvent('outer.gui.item.updated', renamed); - res.json({ ...signEntry(renamed, signingCfg), path: renamed.path }); + res.json({ + ...signEntry(renamed, signingCfg, { + actorUserId: callerActor.user?.id, + }), + path: renamed.path, + }); return; } if (operation === 'delete' || operation === 'trash') { @@ -1580,7 +1597,12 @@ export class LegacyFSController extends PuterController { ? { old_path: targetEntry.path } : undefined, ); - res.json({ ...signEntry(result, signingCfg), path: result.path }); + res.json({ + ...signEntry(result, signingCfg, { + actorUserId: callerActor.user?.id, + }), + path: result.path, + }); return; } @@ -1645,7 +1667,9 @@ export class LegacyFSController extends PuterController { if (entry.isDir) { const children = await this.services.fs.listDirectory(entry.uuid); const signedChildren = children.map((child) => { - const { write_url: _, ...rest } = signEntry(child, signingCfg); + const { write_url: _, ...rest } = signEntry(child, signingCfg, { + ttlSeconds: NON_OWNER_SIGNATURE_TTL_SECONDS, + }); return { ...rest, path: child.path }; }); res.json(signedChildren); @@ -1776,7 +1800,9 @@ export class LegacyFSController extends PuterController { } const signingCfg = signingConfigFromAppConfig(this.config); - const signed = signEntry(entry, signingCfg); + const signed = signEntry(entry, signingCfg, { + actorUserId: actor.user?.id, + }); const signature = writeOk ? { ...signed, path: entry.path } : (() => { diff --git a/src/backend/controllers/fs/legacyFsHelpers.test.ts b/src/backend/controllers/fs/legacyFsHelpers.test.ts index f786f0121..51021ec6a 100644 --- a/src/backend/controllers/fs/legacyFsHelpers.test.ts +++ b/src/backend/controllers/fs/legacyFsHelpers.test.ts @@ -24,11 +24,13 @@ import { getBoolean, getString, loadLegacyAssociatedApps, + signEntry, signEntryThumbnail, signingConfigFromAppConfig, toLegacyEntry, } from './legacyFsHelpers.js'; import type { FSEntry } from '../../stores/fs/FSEntry.js'; +import { NON_OWNER_SIGNATURE_TTL_SECONDS } from '../../util/fileSigning.js'; const entryWithApp = (associatedAppId: number): FSEntry => ({ associatedAppId }) as unknown as FSEntry; @@ -444,3 +446,48 @@ describe('loadLegacyAssociatedApps short-circuit', () => { expect(seen).toEqual([[4, 5]]); }); }); + +describe('signEntry', () => { + const cfg = { secret: 'test-secret', apiBaseUrl: 'https://api.test' }; + const entry = { + uuid: 'e-1', + name: 'x.txt', + isDir: false, + size: 1, + accessed: 0, + modified: 0, + created: 0, + userId: 7, + }; + // signFile ceils its own clock read, so allow a second of slack. + const inSeconds = (expires: number) => + expires - Math.ceil(Date.now() / 1000); + + it('leaves an owner’s signature effectively permanent', () => { + const signed = signEntry(entry, cfg, { actorUserId: 7 }); + expect(inSeconds(signed.expires)).toBeGreaterThan( + NON_OWNER_SIGNATURE_TTL_SECONDS * 10, + ); + }); + + it('expires a signature over someone else’s entry', () => { + const signed = signEntry(entry, cfg, { actorUserId: 8 }); + const ttl = inSeconds(signed.expires); + expect(ttl).toBeLessThanOrEqual(NON_OWNER_SIGNATURE_TTL_SECONDS); + expect(ttl).toBeGreaterThan(0); + }); + + it('keeps the old default when no actor is supplied', () => { + expect(inSeconds(signEntry(entry, cfg).expires)).toBeGreaterThan( + NON_OWNER_SIGNATURE_TTL_SECONDS * 10, + ); + }); + + it('honours an explicit ttl over the ownership check', () => { + const signed = signEntry(entry, cfg, { + actorUserId: 7, + ttlSeconds: 30, + }); + expect(inSeconds(signed.expires)).toBeLessThanOrEqual(30); + }); +}); diff --git a/src/backend/controllers/fs/legacyFsHelpers.ts b/src/backend/controllers/fs/legacyFsHelpers.ts index 06066e292..93b0e99d3 100644 --- a/src/backend/controllers/fs/legacyFsHelpers.ts +++ b/src/backend/controllers/fs/legacyFsHelpers.ts @@ -34,6 +34,7 @@ import { expandTildePath, } from '../../services/fs/resolveNode.js'; import { + NON_OWNER_SIGNATURE_TTL_SECONDS, signFile, type SigningConfig, type SignedFile, @@ -520,7 +521,13 @@ export function signingConfigFromAppConfig(config: IConfig): SigningConfig { return { secret, apiBaseUrl }; } -/** Convenience wrapper: turn an FSEntry into a signed-file response object. */ +/** + * Convenience wrapper: turn an FSEntry into a signed-file response object. + * + * Pass `actorUserId` so a signature over someone else's entry — a shared file — + * expires rather than outliving the share (see + * NON_OWNER_SIGNATURE_TTL_SECONDS). + */ export function signEntry( entry: { uuid: string; @@ -530,8 +537,19 @@ export function signEntry( accessed: number | null; modified: number; created: number | null; + userId?: number; }, config: SigningConfig, + opts: { actorUserId?: number; ttlSeconds?: number } = {}, ): SignedFile { - return signFile(entry as Parameters[0], config); + const isForeign = + typeof opts.actorUserId === 'number' && + typeof entry.userId === 'number' && + entry.userId !== opts.actorUserId; + const ttlSeconds = + opts.ttlSeconds ?? + (isForeign ? NON_OWNER_SIGNATURE_TTL_SECONDS : undefined); + return signFile(entry as Parameters[0], config, { + ...(ttlSeconds === undefined ? {} : { ttlSeconds }), + }); } diff --git a/src/backend/util/fileSigning.ts b/src/backend/util/fileSigning.ts index 50ed08658..0fdf22466 100644 --- a/src/backend/util/fileSigning.ts +++ b/src/backend/util/fileSigning.ts @@ -83,6 +83,16 @@ function signaturesEqual(provided: string, expected: string): boolean { return timingSafeEqual(a, b); } +/** + * Lifetime for a signature over an entry the signer doesn't own. + * + * `verifySignature` checks the signature and expiry, never the ACL, so a URL + * handed to a recipient keeps working after their access is revoked. This is + * what bounds that window; the durable fix is a per-entry signature epoch the + * owner can bump. + */ +export const NON_OWNER_SIGNATURE_TTL_SECONDS = 60 * 60; + /** * Produce a signed-URL object. The default `expires` timestamp uses a * ~317k-year TTL (effectively permanent) — existing clients depend on that