Skip to content

Commit 9f54034

Browse files
authored
fix(openscience): stamp queued usage with its account so a flush can't bill the wrong one (#130)
Queued usage rows carried no account identity, so flushPendingUsage sent every row under whatever session was active at startup. If usage was queued under account A (or while the session was transiently unreadable) and account B later logged in, the flush billed B for A's usage. dropUsageQueue only runs on explicit logout, not a silent account swap. Stamp each queued row with a stable account tag (user_id, else a short hash of the api_key — never the raw key). On flush, a row tagged for a DIFFERENT account is kept in the queue rather than sent (it flushes when that account is active); legacy/untagged rows stay best-effort. Pure shouldFlushForAccount helper is unit-tested. (#14)
1 parent 4c53c5d commit 9f54034

2 files changed

Lines changed: 46 additions & 5 deletions

File tree

backend/cli/src/openscience/index.ts

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import fs from "fs/promises"
44
import { existsSync, readFileSync, writeFileSync, chmodSync } from "fs"
55
import { createRequire } from "module"
66
import { fileURLToPath } from "url"
7-
import { randomUUID } from "crypto"
7+
import { randomUUID, createHash } from "crypto"
88
import { Global } from "../global"
99
import { Log } from "../util/log"
1010
import { Lock } from "../util/lock"
@@ -1235,12 +1235,29 @@ export namespace OpenScience {
12351235

12361236
const pendingQueuePath = path.join(Global.Path.data, "usage-queue.jsonl")
12371237

1238-
async function persistToQueue(params: UsageParams) {
1238+
/** Stable per-account tag stored with a queued usage row so a later flush can't
1239+
* bill a DIFFERENT account for it. user_id when known, else a short hash of the
1240+
* api_key (never the raw key, which must not sit in the queue file). */
1241+
function accountTag(session: OpenScienceSession): string {
1242+
if (session.user_id) return session.user_id
1243+
return "k:" + createHash("sha256").update(session.api_key).digest("hex").slice(0, 16)
1244+
}
1245+
1246+
/** Whether a queued row tagged `rowAccount` may be flushed under `currentAccount`.
1247+
* A row with no tag is legacy/accountless → best-effort send under the current
1248+
* account; a row tagged for a DIFFERENT account is never sent (kept until that
1249+
* account is active). Pure + exported for tests. */
1250+
export function shouldFlushForAccount(rowAccount: string | undefined, currentAccount: string): boolean {
1251+
return !rowAccount || rowAccount === currentAccount
1252+
}
1253+
1254+
async function persistToQueue(params: UsageParams, account?: string) {
12391255
try {
12401256
// Serialize against flushPendingUsage so an append can't land between
12411257
// the flusher's read and its final rewrite (which would delete it).
12421258
using _ = await Lock.write(pendingQueuePath)
1243-
await fs.appendFile(pendingQueuePath, JSON.stringify(params) + "\n")
1259+
const row = account ? { ...params, __account: account } : params
1260+
await fs.appendFile(pendingQueuePath, JSON.stringify(row) + "\n")
12441261
log.info("usage queued for retry", { service: params.service })
12451262
} catch (e) {
12461263
log.warn("failed to persist usage to queue", { error: e instanceof Error ? e.message : String(e) })
@@ -1330,7 +1347,7 @@ export namespace OpenScience {
13301347
return { recorded: false, modelBlocked: true }
13311348
}
13321349
if (!result.permanent) {
1333-
await persistToQueue(params)
1350+
await persistToQueue(params, accountTag(session))
13341351
}
13351352
return null
13361353
}
@@ -1349,11 +1366,18 @@ export namespace OpenScience {
13491366

13501367
const session = await getSession()
13511368
if (!session) return
1369+
const currentAccount = accountTag(session)
13521370

13531371
const retry: string[] = []
13541372
for (const line of lines) {
13551373
try {
1356-
const params: UsageParams = JSON.parse(line)
1374+
const { __account, ...params } = JSON.parse(line) as UsageParams & { __account?: string }
1375+
// Never bill the active account for usage another account generated —
1376+
// keep it queued until that account is the one flushing.
1377+
if (!shouldFlushForAccount(__account, currentAccount)) {
1378+
retry.push(line)
1379+
continue
1380+
}
13571381
const result = await sendReport(params, session)
13581382
if (!result.ok && !result.permanent) {
13591383
retry.push(line)
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { describe, expect, test } from "bun:test"
2+
import { OpenScience } from "../../src/openscience"
3+
4+
describe("OpenScience.shouldFlushForAccount", () => {
5+
test("flushes a row tagged for the current account", () => {
6+
expect(OpenScience.shouldFlushForAccount("user-1", "user-1")).toBe(true)
7+
})
8+
9+
test("refuses a row tagged for a DIFFERENT account (never bill the wrong one)", () => {
10+
expect(OpenScience.shouldFlushForAccount("user-1", "user-2")).toBe(false)
11+
})
12+
13+
test("flushes a legacy/accountless row under the current account (best-effort)", () => {
14+
expect(OpenScience.shouldFlushForAccount(undefined, "user-1")).toBe(true)
15+
expect(OpenScience.shouldFlushForAccount("", "user-1")).toBe(true)
16+
})
17+
})

0 commit comments

Comments
 (0)