Skip to content

Commit 537b1a4

Browse files
committed
fix(web): reject invalid dashboard periods without exiting
1 parent dcdfbc1 commit 537b1a4

7 files changed

Lines changed: 182 additions & 32 deletions

File tree

dash/src/App.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useEffect, useMemo, useState, type ReactNode } from 'react'
2-
import { keepPreviousData, useQuery, useQueryClient } from '@tanstack/react-query'
2+
import { useQuery, useQueryClient } from '@tanstack/react-query'
33

44
import {
55
approvePairing,
@@ -354,7 +354,6 @@ export function App() {
354354
const { data, isError, error, refetch } = useQuery({
355355
queryKey: ['devices', period, provider],
356356
queryFn: () => fetchDevices(period, provider),
357-
placeholderData: keepPreviousData,
358357
// When devices are paired, re-pull periodically so a device that briefly
359358
// dropped (asleep/network blip) reappears on its own instead of staying
360359
// gone until you switch tabs.

src/cli-date.ts

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,35 @@ export const PERIOD_LABELS: Record<Period, string> = {
3131

3232
const VALID_PERIODS: ReadonlyArray<Period> = ['today', 'week', '30days', 'month', 'all']
3333

34-
export function toPeriod(s: string): Period {
34+
export class UsageQueryError extends Error {
35+
constructor(message: string) {
36+
super(message)
37+
this.name = 'UsageQueryError'
38+
}
39+
}
40+
41+
export function parsePeriodOrThrow(s: string): Period {
3542
if ((VALID_PERIODS as readonly string[]).includes(s)) return s as Period
36-
// Fail loudly instead of silently coercing to 'week'. Previously a typo
37-
// like `-p mounth` produced a quiet 7-day report and the user thought
38-
// they were viewing the month.
39-
process.stderr.write(
40-
`codeburn: unknown period "${s}". Valid values: ${VALID_PERIODS.join(', ')}.\n`
41-
)
42-
process.exit(1)
43+
throw new UsageQueryError(`Unknown period "${s}". Valid values: ${VALID_PERIODS.join(', ')}.`)
44+
}
45+
46+
export function toPeriod(s: string): Period {
47+
try {
48+
return parsePeriodOrThrow(s)
49+
} catch {
50+
// Fail loudly instead of silently coercing to 'week'. Previously a typo
51+
// like `-p mounth` produced a quiet 7-day report and the user thought
52+
// they were viewing the month.
53+
process.stderr.write(
54+
`codeburn: unknown period "${s}". Valid values: ${VALID_PERIODS.join(', ')}.\n`
55+
)
56+
process.exit(1)
57+
}
4358
}
4459

4560
function parseLocalDate(s: string): Date {
4661
if (!ISO_DATE_RE.test(s)) {
47-
throw new Error(`Invalid date format "${s}": expected YYYY-MM-DD`)
62+
throw new UsageQueryError(`Invalid date format "${s}": expected YYYY-MM-DD`)
4863
}
4964
const [y, m, d] = s.split('-').map(Number) as [number, number, number]
5065
const date = new Date(y, m - 1, d)
@@ -53,7 +68,7 @@ function parseLocalDate(s: string): Date {
5368
// dated Feb 28 - Mar 2. Reject overflow so the user gets a loud error
5469
// instead of an off-by-N-days date range.
5570
if (date.getFullYear() !== y || date.getMonth() !== m - 1 || date.getDate() !== d) {
56-
throw new Error(`Invalid date "${s}": ${m}/${d}/${y} is not a real calendar date`)
71+
throw new UsageQueryError(`Invalid date "${s}": ${m}/${d}/${y} is not a real calendar date`)
5772
}
5873
return date
5974
}
@@ -118,7 +133,7 @@ export function parseDateRangeFlags(from: string | undefined, to: string | undef
118133
const end = endOfLocalDay(endDate)
119134

120135
if (start > end) {
121-
throw new Error(`--from must not be after --to (got ${from} > ${to})`)
136+
throw new UsageQueryError(`--from must not be after --to (got ${from} > ${to})`)
122137
}
123138
return { start, end }
124139
}
@@ -198,3 +213,15 @@ export function parseDaysFlag(days: string | undefined): { days: Set<string>; ra
198213
export function formatDateRangeLabel(from: string | undefined, to: string | undefined): string {
199214
return `${from ?? 'all'} to ${to ?? 'today'}`
200215
}
216+
217+
/** Resolve a usage query period for HTTP handlers without calling process.exit. */
218+
export function periodInfoFromQuery(
219+
q: { period?: string; from?: string; to?: string },
220+
defaultPeriod: string,
221+
): { range: DateRange; label: string } {
222+
const customRange = parseDateRangeFlags(q.from, q.to)
223+
if (customRange) {
224+
return { range: customRange, label: formatDateRangeLabel(q.from, q.to) }
225+
}
226+
return getDateRange(parsePeriodOrThrow(q.period ?? defaultPeriod))
227+
}

src/sharing/share-run.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { sanitizeForSharing } from './sanitize.js'
99
import { getSharingDir, loadPeers, savePeers } from './store.js'
1010
import { loadPricing } from '../models.js'
1111
import { buildMenubarPayloadForRange } from '../usage-aggregator.js'
12-
import { getDateRange, parseDateRangeFlags, formatDateRangeLabel, toPeriod } from '../cli-date.js'
12+
import { periodInfoFromQuery } from '../cli-date.js'
1313

1414
function lanAddress(): string | null {
1515
for (const list of Object.values(networkInterfaces())) {
@@ -32,10 +32,7 @@ export async function runShareServer(opts: { port: number; pair: boolean; always
3232
const peers = new PeerStore(await loadPeers(dir))
3333

3434
const getUsage = async (q: UsageQuery): Promise<unknown> => {
35-
const customRange = parseDateRangeFlags(q.from, q.to)
36-
const periodInfo = customRange
37-
? { range: customRange, label: formatDateRangeLabel(q.from, q.to) }
38-
: getDateRange(toPeriod(q.period ?? 'month'))
35+
const periodInfo = periodInfoFromQuery(q, 'month')
3936
return sanitizeForSharing(await buildMenubarPayloadForRange(periodInfo, { provider: 'all', optimize: false }))
4037
}
4138

src/sharing/share-server.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { IncomingMessage, ServerResponse } from 'http'
33
import type { TLSSocket } from 'tls'
44
import type { AddressInfo } from 'net'
55

6+
import { UsageQueryError } from '../cli-date.js'
67
import { certFingerprint, pairingCode, PeerStore, PairingWindow } from './pairing.js'
78
import type { Identity } from './identity.js'
89

@@ -82,7 +83,10 @@ export class ShareServer {
8283
} catch (err) {
8384
// Never leave a request hanging (a hung peer makes the caller time out
8485
// and drop this device); always answer, even on an internal error.
85-
if (!res.headersSent) json(500, { error: err instanceof Error ? err.message : String(err) })
86+
if (!res.headersSent) {
87+
const message = err instanceof Error ? err.message : String(err)
88+
json(err instanceof UsageQueryError ? 400 : 500, { error: message })
89+
}
8690
}
8791
}
8892

src/web-dashboard.ts

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { hostname } from 'os'
1010

1111
import { loadPricing } from './models.js'
1212
import { buildMenubarPayloadForRange } from './usage-aggregator.js'
13-
import { getDateRange, parseDateRangeFlags, formatDateRangeLabel, toPeriod } from './cli-date.js'
13+
import { periodInfoFromQuery, UsageQueryError } from './cli-date.js'
1414
import { pullDevices, linkRemote } from './sharing/host.js'
1515
import { browse } from './sharing/discovery.js'
1616
import { loadOrCreateIdentity } from './sharing/identity.js'
@@ -31,6 +31,11 @@ function readBody(req: import('http').IncomingMessage): Promise<string> {
3131
})
3232
}
3333

34+
function writeJsonError(res: import('http').ServerResponse, status: number, error: string): void {
35+
res.writeHead(status, { 'content-type': 'application/json; charset=utf-8' })
36+
res.end(JSON.stringify({ error }))
37+
}
38+
3439
const HERE = dirname(fileURLToPath(import.meta.url))
3540

3641
// Locate the built React dashboard (dist/dash). Works both when running from a
@@ -94,10 +99,7 @@ export async function runWebDashboard(opts: {
9499
// Sharing this device serves the SANITIZED aggregate (no project names/paths
95100
// or per-session detail), unlike the local /api/usage which shows everything.
96101
const shareGetUsage = async (q: { period?: string; from?: string; to?: string }) => {
97-
const customRange = parseDateRangeFlags(q.from, q.to)
98-
const periodInfo = customRange
99-
? { range: customRange, label: formatDateRangeLabel(q.from, q.to) }
100-
: getDateRange(toPeriod(q.period ?? opts.period))
102+
const periodInfo = periodInfoFromQuery(q, opts.period)
101103
return sanitizeForSharing(await buildMenubarPayloadForRange(periodInfo, { provider: 'all', optimize: false }))
102104
}
103105
const share = new ShareController(shareGetUsage)
@@ -126,10 +128,14 @@ export async function runWebDashboard(opts: {
126128
const provider = url.searchParams.get('provider') ?? opts.provider
127129
const from = url.searchParams.get('from') ?? opts.from
128130
const to = url.searchParams.get('to') ?? opts.to
129-
const customRange = parseDateRangeFlags(from, to)
130-
const periodInfo = customRange
131-
? { range: customRange, label: formatDateRangeLabel(from, to) }
132-
: getDateRange(toPeriod(period))
131+
let periodInfo
132+
try {
133+
periodInfo = periodInfoFromQuery({ period, from, to }, opts.period)
134+
} catch (err) {
135+
if (!(err instanceof UsageQueryError)) throw err
136+
writeJsonError(res, 400, err instanceof Error ? err.message : String(err))
137+
return
138+
}
133139
const payload = await buildMenubarPayloadForRange(periodInfo, {
134140
provider,
135141
project: opts.project,
@@ -148,11 +154,15 @@ export async function runWebDashboard(opts: {
148154
const provider = url.searchParams.get('provider') ?? opts.provider
149155
const from = url.searchParams.get('from') ?? opts.from
150156
const to = url.searchParams.get('to') ?? opts.to
157+
try {
158+
periodInfoFromQuery({ period, from, to }, opts.period)
159+
} catch (err) {
160+
if (!(err instanceof UsageQueryError)) throw err
161+
writeJsonError(res, 400, err instanceof Error ? err.message : String(err))
162+
return
163+
}
151164
const localGetUsage = async (q: { period?: string; from?: string; to?: string }) => {
152-
const customRange = parseDateRangeFlags(q.from, q.to)
153-
const periodInfo = customRange
154-
? { range: customRange, label: formatDateRangeLabel(q.from, q.to) }
155-
: getDateRange(toPeriod(q.period ?? period))
165+
const periodInfo = periodInfoFromQuery(q, period)
156166
return buildMenubarPayloadForRange(periodInfo, { provider, project: opts.project, exclude: opts.exclude, optimize: false })
157167
}
158168
const results = await pullDevices(localGetUsage, { period, from, to }, hostname(), {})

tests/cli-date.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import {
33
getDateRange,
44
PERIODS,
55
PERIOD_LABELS,
6+
parsePeriodOrThrow,
7+
periodInfoFromQuery,
68
toPeriod,
79
type Period,
810
} from '../src/cli-date.js'
@@ -104,6 +106,42 @@ describe('PERIODS / PERIOD_LABELS', () => {
104106
})
105107
})
106108

109+
describe('parsePeriodOrThrow', () => {
110+
it('round-trips known periods', () => {
111+
const known: Period[] = ['today', 'week', '30days', 'month', 'all']
112+
for (const p of known) {
113+
expect(parsePeriodOrThrow(p)).toBe(p)
114+
}
115+
})
116+
117+
it('throws on unknown input without calling process.exit', () => {
118+
const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => { throw new Error('exit') })
119+
try {
120+
expect(() => parsePeriodOrThrow('garbage')).toThrow(/Unknown period "garbage"/)
121+
expect(exitSpy).not.toHaveBeenCalled()
122+
} finally {
123+
exitSpy.mockRestore()
124+
}
125+
})
126+
})
127+
128+
describe('periodInfoFromQuery', () => {
129+
it('resolves a named period', () => {
130+
const info = periodInfoFromQuery({ period: 'week' }, 'month')
131+
expect(info.label).toBe('Last 7 Days')
132+
})
133+
134+
it('throws for an invalid period without calling process.exit', () => {
135+
const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => { throw new Error('exit') })
136+
try {
137+
expect(() => periodInfoFromQuery({ period: 'garbage' }, 'month')).toThrow(/Unknown period "garbage"/)
138+
expect(exitSpy).not.toHaveBeenCalled()
139+
} finally {
140+
exitSpy.mockRestore()
141+
}
142+
})
143+
})
144+
107145
describe('toPeriod', () => {
108146
it('round-trips known periods', () => {
109147
const known: Period[] = ['today', 'week', '30days', 'month', 'all']

tests/sharing/transport.test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'
33
import { generateIdentity, type Identity } from '../../src/sharing/identity.js'
44
import { PeerStore } from '../../src/sharing/pairing.js'
55
import { ShareServer } from '../../src/sharing/share-server.js'
6+
import { getDateRange, parsePeriodOrThrow } from '../../src/cli-date.js'
67
import { hello, pair, fetchUsage } from '../../src/sharing/client.js'
78

89
describe('device sharing transport (loopback mutual TLS)', () => {
@@ -52,6 +53,80 @@ describe('device sharing transport (loopback mutual TLS)', () => {
5253
expect((ur.json as { current: { cost: number } }).current.cost).toBe(42)
5354
})
5455

56+
it('returns bad request when getUsage rejects an invalid period', async () => {
57+
const badServer = new ShareServer({
58+
identity: serverId,
59+
peers,
60+
getUsage: async (q) => {
61+
getDateRange(parsePeriodOrThrow(q.period ?? 'month'))
62+
return { current: { cost: 0 } }
63+
},
64+
})
65+
const badPort = await badServer.listen(0, '127.0.0.1')
66+
try {
67+
const pin = badServer.openPairing()
68+
const pr = await pair({ ...ep(), port: badPort }, pin, 'Mac Studio')
69+
const token = (pr.json as { token: string }).token
70+
const ur = await fetchUsage(
71+
{ ...ep(), port: badPort, expectedFingerprint: serverId.fingerprint },
72+
token,
73+
{ period: 'garbage' },
74+
)
75+
expect(ur.status).toBe(400)
76+
expect((ur.json as { error: string }).error).toMatch(/Unknown period "garbage"/)
77+
} finally {
78+
await badServer.close()
79+
}
80+
})
81+
82+
it('keeps unexpected getUsage failures as internal errors', async () => {
83+
const badServer = new ShareServer({
84+
identity: serverId,
85+
peers,
86+
getUsage: async () => {
87+
throw new Error('database temporarily unavailable')
88+
},
89+
})
90+
const badPort = await badServer.listen(0, '127.0.0.1')
91+
try {
92+
const pin = badServer.openPairing()
93+
const pr = await pair({ ...ep(), port: badPort }, pin, 'Mac Studio')
94+
const token = (pr.json as { token: string }).token
95+
const ur = await fetchUsage(
96+
{ ...ep(), port: badPort, expectedFingerprint: serverId.fingerprint },
97+
token,
98+
)
99+
expect(ur.status).toBe(500)
100+
expect((ur.json as { error: string }).error).toMatch(/database temporarily unavailable/)
101+
} finally {
102+
await badServer.close()
103+
}
104+
})
105+
106+
it('does not classify plain string-matched errors as usage validation errors', async () => {
107+
const badServer = new ShareServer({
108+
identity: serverId,
109+
peers,
110+
getUsage: async () => {
111+
throw new Error('Unknown period "garbage". Valid values: today, week, 30days, month, all.')
112+
},
113+
})
114+
const badPort = await badServer.listen(0, '127.0.0.1')
115+
try {
116+
const pin = badServer.openPairing()
117+
const pr = await pair({ ...ep(), port: badPort }, pin, 'Mac Studio')
118+
const token = (pr.json as { token: string }).token
119+
const ur = await fetchUsage(
120+
{ ...ep(), port: badPort, expectedFingerprint: serverId.fingerprint },
121+
token,
122+
)
123+
expect(ur.status).toBe(500)
124+
expect((ur.json as { error: string }).error).toMatch(/Unknown period "garbage"/)
125+
} finally {
126+
await badServer.close()
127+
}
128+
})
129+
55130
it('rejects a wrong PIN', async () => {
56131
server.openPairing()
57132
const pr = await pair(ep(), '000000', 'x')

0 commit comments

Comments
 (0)