mirror of
https://github.com/HeyPuter/puter.git
synced 2026-08-24 15:07:17 +00:00
fix(fs): expire signed URLs over entries the signer doesn't own
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.
This commit is contained in:
@@ -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 }
|
||||
: (() => {
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<typeof signFile>[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<typeof signFile>[0], config, {
|
||||
...(ttlSeconds === undefined ? {} : { ttlSeconds }),
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user