From aa24b145a4fb337e2ef54611bb4669f340cf32ee Mon Sep 17 00:00:00 2001 From: Santosh Yadav Date: Wed, 4 Nov 2026 06:46:29 +0100 Subject: [PATCH] fix: trust only own entries in the session token store devframe's verifyAuthToken trusts a token when `trusted[token]` is truthy. The store is an ordinary object, so inherited keys such as `constructor` or `toString` pass as tokens, and any client could become trusted through the auth handshake or the `devframe_auth_token` connection parameter without the one-time code. devframe 1.3.2 has the same check. At startup the devframe's setup now rebuilds `trusted` as a null-prototype object holding only own entries that are token records. devframe's storage keeps the prototype on later copies, so exchanges, revokes and pruning leave it prototype-free. This lives in the published package, so apps that install it are covered without a dependency patch. --- apps/docs/src/content/security.md | 2 +- .../devtools/src/__tests__/auth-guard.test.ts | 74 ++++++++++++++++--- packages/devtools/src/auth-store.ts | 34 ++++++++- 3 files changed, 99 insertions(+), 11 deletions(-) diff --git a/apps/docs/src/content/security.md b/apps/docs/src/content/security.md index d0853bd0..9413088a 100644 --- a/apps/docs/src/content/security.md +++ b/apps/docs/src/content/security.md @@ -94,7 +94,7 @@ A refused code is never compared with the real one. devframe gives you a new cod #### Trusted browsers -A browser that exchanges the code gets a session token, so it doesn't need the code again after a reload. The server keeps those tokens in `~/.pangular-inspector/auth//auth.json`, one folder per project (named after a hash of its path), so a token from one project doesn't open another project's server. The folder is readable by you only (mode `0700`), and the file too (`0600`). A token expires 7 days after the code was exchanged. To sign out every browser, delete the project's folder, or all of `~/.pangular-inspector/auth`, and restart the server. +A browser that exchanges the code gets a session token, so it doesn't need the code again after a reload. The server keeps those tokens in `~/.pangular-inspector/auth//auth.json`, one folder per project (named after a hash of its path), so a token from one project doesn't open another project's server. The folder is readable by you only (mode `0700`), and the file too (`0600`). A token expires 7 days after the code was exchanged. At startup the server drops entries in the file that aren't token records, and it only trusts a token that has its own entry, so a name such as `constructor` or `toString` is never a token. To sign out every browser, delete the project's folder, or all of `~/.pangular-inspector/auth`, and restart the server. Version 0.0.7 and earlier kept the tokens in `~/.devframes/devframe/auth.json` (the Express hub and the Vite plugin) or `~/.pangular/devframe/auth.json` (the CLI), readable by other users and without expiry. Newer versions don't read those files. Delete them if no other devframe tool uses them. diff --git a/packages/devtools/src/__tests__/auth-guard.test.ts b/packages/devtools/src/__tests__/auth-guard.test.ts index 2dc7beb0..14180b68 100644 --- a/packages/devtools/src/__tests__/auth-guard.test.ts +++ b/packages/devtools/src/__tests__/auth-guard.test.ts @@ -15,7 +15,12 @@ import { connectDevframe } from 'devframe/client'; import { getTempAuthCode } from 'devframe/node/auth'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { CodeGuard, guardedExchange } from '../auth-guard.ts'; -import { authStoreDir, pruneExpiredTokens, AUTH_TOKEN_TTL_MS } from '../auth-store.ts'; +import { + authStoreDir, + hardenTokenStore, + pruneExpiredTokens, + AUTH_TOKEN_TTL_MS, +} from '../auth-store.ts'; import { initPangularHub } from '../hub.ts'; describe('CodeGuard', () => { @@ -140,6 +145,27 @@ describe('pruneExpiredTokens', () => { expect(Object.keys(state.trusted)).toEqual(['fresh']); }); + it('rebuilds the store without a prototype and drops entries that are not token records', () => { + const state = { + trusted: { + good: { authToken: 'good', timestamp: 1 }, + legacy: { timestamp: 1 }, + swapped: { authToken: 'other', timestamp: 1 }, + bare: true, + } as Record, + }; + const storage = { + value: () => state as never, + mutate: (fn: (s: never) => void) => fn(state as never), + }; + expect(hardenTokenStore(storage)).toBe(2); + expect(Object.getPrototypeOf(state.trusted)).toBeNull(); + expect(Object.keys(state.trusted)).toEqual(['good', 'legacy']); + expect(state.trusted['constructor']).toBeUndefined(); + expect(state.trusted['toString']).toBeUndefined(); + expect(hardenTokenStore(storage)).toBe(0); + }); + it('names the store after a hash of the project, not its path', () => { const dir = authStoreDir('/work/secret-project', '/home/me'); expect(dir).toMatch(/^\/home\/me\/\.pangular-inspector\/auth\/[0-9a-f]{16}$/); @@ -151,6 +177,15 @@ describe('initPangularHub one-time code', () => { const cleanups: (() => Promise | void)[] = []; afterEach(async () => { for (const cleanup of cleanups.splice(0).reverse()) await cleanup(); + // The devframe client keeps its last connection and token on globalThis. + const stored = globalThis as Record; + for (const key of [ + '__DEVFRAME_CONNECTION__', + '__DEVFRAME_CONNECTION_META__', + '__DEVFRAME_CONNECTION_AUTH_TOKEN__', + ]) { + delete stored[key]; + } vi.unstubAllGlobals(); vi.unstubAllEnvs(); }); @@ -173,16 +208,37 @@ describe('initPangularHub one-time code', () => { cleanups.push(() => hub.close()); vi.stubGlobal('location', new URL(`http://127.0.0.1:${port}/`)); await hub.context; - const rpc = await connectDevframe({ - baseURL: `http://127.0.0.1:${port}/__devframes/pangular/`, - simpleAuth: false, - otpParam: false, - webmcp: false, - }); - cleanups.push(() => rpc.close?.()); - return { rpc, home }; + const baseURL = `http://127.0.0.1:${port}/__devframes/pangular/`; + const connect = async (authToken?: string) => { + const client = await connectDevframe({ + baseURL, + ...(authToken ? { authToken } : {}), + simpleAuth: false, + otpParam: false, + webmcp: false, + }); + cleanups.push(() => client.close?.()); + return client; + }; + const rpc = await connect(); + return { rpc, home, connect }; } + it('trusts no token named after an Object.prototype property', async () => { + const { rpc, connect, home } = await serve(); + expect(await rpc.requestTrustWithCode(getTempAuthCode())).toBeTruthy(); + for (const token of ['constructor', '__proto__', 'toString', 'hasOwnProperty']) { + const client = await connect(token); + expect(await client.ensureTrusted(500).catch(() => false), token).toBe(false); + } + const file = join(authStoreDir(process.cwd(), home), 'auth.json'); + const issued = await vi.waitFor( + () => Object.keys(JSON.parse(readFileSync(file, 'utf8')).trusted)[0] as string, + { timeout: 3000 }, + ); + expect(await (await connect(issued)).ensureTrusted(3000)).toBe(true); + }); + it('slows down a client that guesses, and stores its token privately per project', async () => { const { rpc, home } = await serve(); const wrong = (code: string) => (code === '000000' ? '000001' : '000000'); diff --git a/packages/devtools/src/auth-store.ts b/packages/devtools/src/auth-store.ts index 2944aa0f..bbf4cc7c 100644 --- a/packages/devtools/src/auth-store.ts +++ b/packages/devtools/src/auth-store.ts @@ -126,6 +126,36 @@ interface AuthStorage { mutate(fn: (state: { trusted: Record }) => void): void; } +function isTokenRecord(token: string, record: unknown): boolean { + if (!record || typeof record !== 'object') return false; + const { authToken, timestamp } = record as TrustedRecord & { authToken?: unknown }; + return typeof timestamp === 'number' && (authToken === undefined || authToken === token); +} + +/** + * devframe trusts a token when `trusted[token]` is truthy. With an ordinary + * object that includes inherited keys, so `constructor` or `toString` would + * pass as a token. This rebuilds `trusted` with no prototype, keeping only + * own entries that hold a record for their own token. devframe's storage + * copies objects with their prototype, so the store stays prototype-free + * through later exchanges and revokes. Returns how many entries it dropped. + */ +export function hardenTokenStore(storage: AuthStorage): number { + const trusted = storage.value().trusted; + const entries = trusted && typeof trusted === 'object' ? Object.entries(trusted) : []; + const kept = entries.filter(([token, record]) => isTokenRecord(token, record)); + if (trusted && Object.getPrototypeOf(trusted) === null && kept.length === entries.length) { + return 0; + } + storage.mutate((state) => { + state.trusted = Object.assign( + Object.create(null) as Record, + Object.fromEntries(kept), + ); + }); + return entries.length - kept.length; +} + /** Drops tokens older than `ttl`. Returns how many it dropped. */ export function pruneExpiredTokens( storage: AuthStorage, @@ -146,7 +176,8 @@ export function pruneExpiredTokens( const pruning = new WeakSet(); /** - * Prunes the context's token store now and every hour. Safe to call more + * Hardens the context's token store (`hardenTokenStore`), then prunes it now + * and every hour. Safe to call more * than once per context. Skipped for a static build, which has no tokens. */ export function expireAuthTokens( @@ -164,6 +195,7 @@ export function expireAuthTokens( return; } // devframe rewrites the file after a change, with the default mode. + hardenTokenStore(storage); const prune = () => { if (pruneExpiredTokens(storage, Date.now(), ttl)) onPruned(); };