From d717c313d0bc2aa4a886d5e99e8eb87ff3311d3d Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Mon, 10 Aug 2026 20:57:41 -0700 Subject: [PATCH] fix: metering cache hits + whoami gates --- extensions/whoami.test.ts | 59 +++++++++++++ extensions/whoami.ts | 13 ++- .../metering/MeteringBufferStore.test.ts | 33 +++++++ .../stores/metering/MeteringBufferStore.ts | 42 +++++++-- src/backend/util/secureHttp.test.ts | 85 +++++++++++++++++++ src/backend/util/secureHttp.ts | 36 +++++++- src/docs/src/Objects/user.md | 2 +- src/puter-js/types/modules/auth.d.ts | 6 +- 8 files changed, 261 insertions(+), 15 deletions(-) diff --git a/extensions/whoami.test.ts b/extensions/whoami.test.ts index 0e34b1b14..7a8b158d5 100644 --- a/extensions/whoami.test.ts +++ b/extensions/whoami.test.ts @@ -61,6 +61,7 @@ const seedUser = async () => { uuid: uuidv4(), password: 'hashedpw', email: `${slug}@example.com`, + last_activity_ts: new Date().toISOString(), }); }; @@ -163,6 +164,64 @@ describe('whoami extension — handleWhoami', () => { expect(body.directories).toBeUndefined(); }); + describe('last_activity_ts', () => { + const whoamiFor = async (actor: Record) => { + const { res, captured } = makeRes(); + await runWithContext({ actor }, () => handleWhoami(makeReq(), res)); + return captured.body as Record; + }; + + it('is sent to a browser/root session', async () => { + const user = await seedUser(); + const body = await whoamiFor({ + user: { uuid: user.uuid, id: user.id as number }, + session: { uid: 'sess-web', kind: 'web' }, + }); + + expect(typeof body.last_activity_ts).toBe('number'); + }); + + it('is sent to a full-access personal access token', async () => { + const user = await seedUser(); + const body = await whoamiFor({ + user: { uuid: user.uuid, id: user.id as number }, + accessToken: { uid: 'tok-full', fullAccess: true }, + }); + + expect(typeof body.last_activity_ts).toBe('number'); + }); + + it('is withheld from a scoped access token', async () => { + const user = await seedUser(); + const body = await whoamiFor({ + user: { uuid: user.uuid, id: user.id as number }, + accessToken: { uid: 'tok-scoped' }, + }); + + expect(body.last_activity_ts).toBeUndefined(); + }); + + it('is withheld from app actors', async () => { + const user = await seedUser(); + const body = await whoamiFor({ + user: { uuid: user.uuid, id: user.id as number }, + app: { uid: 'app-test-actor' }, + }); + + expect(body.last_activity_ts).toBeUndefined(); + }); + + it('is withheld from worker tokens', async () => { + const user = await seedUser(); + const body = await whoamiFor({ + user: { uuid: user.uuid, id: user.id as number }, + session: { uid: 'sess-worker', kind: 'worker' }, + }); + + expect(body.last_activity_ts).toBeUndefined(); + }); + }); + it('redacts tmp_password from metadata for user actors', async () => { const user = await seedUser(); await server.stores.user.updateMetadata(user.id as number, { diff --git a/extensions/whoami.ts b/extensions/whoami.ts index 2d44f8042..7850e18e4 100644 --- a/extensions/whoami.ts +++ b/extensions/whoami.ts @@ -150,9 +150,15 @@ export const handleWhoami = async ( details.directories = directories; } - // Last activity + // Last activity — when the account was last online. Only the account's + // own credentials see it: a browser/root session, or a full-access + // personal access token. Delegated credentials (apps, workers, scoped + // access tokens) are told who the user is, not when they were around. + const isOwnCredential = actor.accessToken + ? actor.accessToken.fullAccess === true + : !actor.app && actor.session?.kind !== 'worker'; const lastActivityTs = toUnixSeconds(user.last_activity_ts); - if (lastActivityTs !== undefined) { + if (isOwnCredential && lastActivityTs !== undefined) { details.last_activity_ts = lastActivityTs; } @@ -189,7 +195,8 @@ export const handleWhoami = async ( } const subscription = details.subscription as - { offering?: Record } | undefined; + | { offering?: Record } + | undefined; if (subscription?.offering) { delete subscription.offering.group; delete subscription.offering.benefits; diff --git a/src/backend/stores/metering/MeteringBufferStore.test.ts b/src/backend/stores/metering/MeteringBufferStore.test.ts index b9dc0dc22..46880f2be 100644 --- a/src/backend/stores/metering/MeteringBufferStore.test.ts +++ b/src/backend/stores/metering/MeteringBufferStore.test.ts @@ -531,6 +531,39 @@ describe('MeteringBufferStore', () => { ).toEqual({}); }); + it('holds an abandoned claim for a later cycle when not sweeping', async () => { + const tag = bucketTag(key); + const nonce = 'unsweptnonce'; + const entry = `${Date.now() - 60_000}:${key}`; + + await server.clients.redis.hset( + `meter:p:{${tag}}:${nonce}`, + 'total', + '25', + ); + await server.clients.redis.hset( + `meter:pending:{${tag}}`, + nonce, + entry, + ); + + await target.flushCycle(false); + + // Untouched, not lost: the amount has gone nowhere and the claim is + // still indexed for whichever cycle sweeps next. + expect(await storedTotal(key)).toBe(0); + expect( + await server.clients.redis.hgetall(`meter:pending:{${tag}}`), + ).toEqual({ [nonce]: entry }); + + await target.flushCycle(); + + expect(await storedTotal(key)).toBe(25); + expect( + await server.clients.redis.hgetall(`meter:pending:{${tag}}`), + ).toEqual({}); + }); + it('re-drives an abandoned claim through exactly one of two concurrent flushes', async () => { const tag = bucketTag(key); const nonce = 'abandonednonce'; diff --git a/src/backend/stores/metering/MeteringBufferStore.ts b/src/backend/stores/metering/MeteringBufferStore.ts index 30597adcb..bb097710f 100644 --- a/src/backend/stores/metering/MeteringBufferStore.ts +++ b/src/backend/stores/metering/MeteringBufferStore.ts @@ -62,9 +62,25 @@ const FLUSH_JITTER_MS = 500; /** Per bucket, per cycle. Anything above this flushes on the next cycle. */ const CLAIMS_PER_BUCKET = 100; -/** A claim left unfinished this long is assumed abandoned and re-driven. */ +/** + * A claim left unfinished this long is assumed abandoned and re-driven. The + * sweep that finds it runs on its own cadence, so a claim can sit up to one + * sweep interval past this before anything picks it up. + */ const ORPHAN_AGE_MS = 30_000; +/** + * Flush cycles between orphan sweeps. Nothing is eligible to be re-driven until + * it is `ORPHAN_AGE_MS` old, so reading the pending index every cycle turns up + * nothing the cycles before it have not already passed over — and a read of an + * absent index still costs a lookup on every bucket. Sweeping on its own + * cadence trades a little detection latency for the reads in between. + */ +const CYCLES_PER_ORPHAN_SWEEP = Math.max( + 1, + Math.round(ORPHAN_AGE_MS / FLUSH_INTERVAL_MS), +); + /** Long enough that a counter for the current month can never expire. */ const BUFFER_TTL_MS = 40 * 24 * 60 * 60 * 1000; @@ -303,6 +319,7 @@ export class MeteringBufferStore extends PuterStore { #absorbedCount = 0; #flushedCount = 0; #cyclesSinceReport = 0; + #cyclesSinceSweep = 0; // -- Lifecycle ---------------------------------------------------- @@ -528,7 +545,13 @@ export class MeteringBufferStore extends PuterStore { const delay = FLUSH_INTERVAL_MS + (Math.random() * 2 - 1) * FLUSH_JITTER_MS; this.#flushTimer = setTimeout(() => { - this.flushCycle() + // Only the timed cycle throttles the sweep. A cycle asked for + // directly — a drain, a test — is expected to do the whole job. + const sweepOrphans = this.#cyclesSinceSweep === 0; + this.#cyclesSinceSweep = + (this.#cyclesSinceSweep + 1) % CYCLES_PER_ORPHAN_SWEEP; + + this.flushCycle(sweepOrphans) .catch((e) => { console.error('[metering] flush cycle failed', e); }) @@ -541,14 +564,18 @@ export class MeteringBufferStore extends PuterStore { * Write one cycle's worth of buffered counters onward. Driven by the flush * timer; returns how many counters it handled so a drain loop knows when * there is nothing left. + * + * `sweepOrphans` decides whether this cycle also looks for claims an + * earlier flush abandoned. Skipping it leaves them for a later cycle; + * nothing is dropped either way. */ - async flushCycle(): Promise { + async flushCycle(sweepOrphans = true): Promise { const work: Array<() => Promise> = []; let truncated = 0; const buckets = await Promise.all( Array.from({ length: BUCKET_COUNT }, (_, bucket) => - this.#drainBucket(`m${bucket}`), + this.#drainBucket(`m${bucket}`, sweepOrphans), ), ); @@ -605,7 +632,10 @@ export class MeteringBufferStore extends PuterStore { ); } - async #drainBucket(tag: string): Promise<{ + async #drainBucket( + tag: string, + sweepOrphans: boolean, + ): Promise<{ tag: string; keys: string[]; orphans: Array<{ nonce: string; key: string }>; @@ -614,7 +644,7 @@ export class MeteringBufferStore extends PuterStore { const redis = this.clients.redis; const [popped, pending] = await Promise.all([ redis.spop(dirtyKey(tag), CLAIMS_PER_BUCKET), - redis.hgetall(pendingIndexKey(tag)), + sweepOrphans ? redis.hgetall(pendingIndexKey(tag)) : null, ]); // An empty pop can come back as nothing at all rather than an empty diff --git a/src/backend/util/secureHttp.test.ts b/src/backend/util/secureHttp.test.ts index 8395bcef1..2909af34e 100644 --- a/src/backend/util/secureHttp.test.ts +++ b/src/backend/util/secureHttp.test.ts @@ -21,6 +21,7 @@ import { Agent as UndiciAgent } from 'undici'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { configContainer } from '../exports.js'; import { + guardedLookup, isPublicResolvedAddress, secureFetch, validateUrlNoIP, @@ -32,6 +33,23 @@ vi.mock('node:dns', async (importOriginal) => { 'resolves-private.test': '10.0.0.1', 'resolves-metadata.test': '169.254.169.254', }; + // Multi-address fixtures, in the order a resolver really hands them back + // for these shapes — AAAA ahead of A for a dual-stack name. + const MULTI_HOSTS: Record = { + 'dual-stack.test': [ + { address: '2606:4700:20::ac43:47b8', family: 6 }, + { address: '104.26.14.31', family: 4 }, + ], + 'v6-only.test': [{ address: '2606:4700:20::ac43:47b8', family: 6 }], + 'private-v4-public-v6.test': [ + { address: '10.0.0.1', family: 4 }, + { address: '2606:4700:20::ac43:47b8', family: 6 }, + ], + 'public-v4-private-v6.test': [ + { address: '104.26.14.31', family: 4 }, + { address: '::1', family: 6 }, + ], + }; return { ...actual, lookup: ((hostname: string, options: unknown, callback: unknown) => { @@ -39,6 +57,11 @@ vi.mock('node:dns', async (importOriginal) => { err: Error | null, addresses?: { address: string; family: number }[], ) => void; + const multi = MULTI_HOSTS[hostname]; + if (multi) { + cb(null, multi); + return; + } const addr = PRIVATE_HOSTS[hostname]; if (addr) { cb(null, [{ address: addr, family: 4 }]); @@ -180,6 +203,68 @@ describe('secureHttp resolved address validation', () => { } }); + // -- Address family selection ------------------------------------ + // + // A single-address caller gets one shot, so handing back an address on a + // family this host can't route means the connection hangs to the connect + // timeout instead of failing over. Resolvers put AAAA first for dual-stack + // names, which is where that bites. + + /** Drive `guardedLookup` against one of the DNS fixtures above. */ + const lookupHost = ( + hostname: string, + options: Record = {}, + ) => + new Promise<{ err: Error | null; result: unknown; family?: number }>( + (resolve) => { + guardedLookup( + hostname, + options as never, + ((err: Error | null, result: unknown, family?: number) => + resolve({ err, result, family })) as never, + ); + }, + ); + + it('hands a single-address caller a routable family, not just the first answer', async () => { + const { err, result, family } = await lookupHost('dual-stack.test'); + expect(err).toBeNull(); + expect(result).toBe('104.26.14.31'); + expect(family).toBe(4); + }); + + it('still returns the only family available when a host is v6-only', async () => { + const { err, result, family } = await lookupHost('v6-only.test'); + expect(err).toBeNull(); + expect(result).toBe('2606:4700:20::ac43:47b8'); + expect(family).toBe(6); + }); + + it('hands the full list to a caller that asked for every address', async () => { + const { err, result } = await lookupHost('dual-stack.test', { + all: true, + }); + expect(err).toBeNull(); + expect(result).toEqual([ + { address: '2606:4700:20::ac43:47b8', family: 6 }, + { address: '104.26.14.31', family: 4 }, + ]); + }); + + // The selection above must never become a way to reach a blocked address: + // one private answer rejects the whole resolution, whichever family it is + // on and whichever family selection would have preferred. + it('rejects the whole set when any resolved address is private', async () => { + for (const host of [ + 'private-v4-public-v6.test', + 'public-v4-private-v6.test', + ]) { + const { err, result } = await lookupHost(host); + expect(err).toMatchObject({ code: 'ERR_SSRF_BLOCKED' }); + expect(result).toBe(''); + } + }); + it('does not apply the connect-time guard when routed through the CORS proxy', async () => { // The proxy is admin-trusted config and may legitimately sit on a // private address; the user-supplied host is resolved by the proxy diff --git a/src/backend/util/secureHttp.ts b/src/backend/util/secureHttp.ts index 704e595f5..2e14e892e 100644 --- a/src/backend/util/secureHttp.ts +++ b/src/backend/util/secureHttp.ts @@ -110,7 +110,11 @@ export function isPublicResolvedAddress(address: string): boolean { // public, and the check runs on the resolution the socket actually uses — // a validate-then-fetch design re-resolves at connect time, so checking // here (rather than before fetch) is what defeats DNS rebinding. -const guardedLookup: net.LookupFunction = (hostname, options, callback) => { +export const guardedLookup: net.LookupFunction = ( + hostname, + options, + callback, +) => { // `net`'s lookup hook can be handed `options` as either an object or a // bare address-family number. Normalize so we can both resolve all // addresses and report errors back in the arity the caller expects: @@ -147,14 +151,38 @@ const guardedLookup: net.LookupFunction = (hostname, options, callback) => { } if (wantAll) { callback(null, list); - } else { - callback(null, list[0].address, list[0].family); + return; } + // Single-address callers get no second try, so the one we hand back + // has to be on a family this host can actually route. Resolvers order + // AAAA first for dual-stack names, and a deployment without IPv6 + // egress then has nowhere to fall back to — the connection just hangs + // until the connect timeout, which reads as the destination being + // down rather than as an unusable family. + // + // Only reached when the caller opted out of address selection (see + // `autoSelectFamily` below); either way the address still comes from + // the list validated above, so re-resolving can't slip an unvalidated + // one in. + const preferred = list.find((a) => a.family === 4) ?? list[0]; + callback(null, preferred.address, preferred.family); }); }; const ssrfGuardDispatcher = new UndiciAgent({ - connect: { lookup: guardedLookup }, + connect: { + lookup: guardedLookup, + // Try each resolved address rather than betting the request on the + // first one. Without this, whether a dead address family costs 250ms + // or the full connect timeout depends on the runtime's default, which + // is not something this module should be at the mercy of. + // + // Safe against the rebinding this guard exists to stop: the addresses + // raced here are exactly the ones `guardedLookup` validated, so every + // candidate has already been checked. + autoSelectFamily: true, + autoSelectFamilyAttemptTimeout: 250, + }, }); // Proxied requests skip the SSRF lookup guard (the only locally-resolved diff --git a/src/docs/src/Objects/user.md b/src/docs/src/Objects/user.md index 2a171c946..b14d72c33 100644 --- a/src/docs/src/Objects/user.md +++ b/src/docs/src/Objects/user.md @@ -37,7 +37,7 @@ A boolean value indicating whether the user's account is temporary. #### `last_activity_ts` (Number) -A number value indicating the user's last active timestamp. +A number value indicating the user's last active timestamp. Only returned to the account's own credentials — a signed-in session or a full-access API token; apps acting on a user's behalf, workers, and scoped tokens do not receive it. #### `paid_storage` (Number) diff --git a/src/puter-js/types/modules/auth.d.ts b/src/puter-js/types/modules/auth.d.ts index 6a29582a8..b3ab9140b 100644 --- a/src/puter-js/types/modules/auth.d.ts +++ b/src/puter-js/types/modules/auth.d.ts @@ -21,7 +21,11 @@ export interface User { hasDevAccountAccess?: boolean; /** Whether the user's account is temporary. */ is_temp?: boolean; - /** The user's last active timestamp. */ + /** + * The user's last active timestamp. Only returned to the account's own + * credentials — a signed-in session or a full-access API token; apps, + * workers and scoped tokens do not receive it. + */ last_activity_ts?: number; otp?: boolean; /** The amount of paid storage. */