ReviewOS
Reload

also looking at this

loghqorg/loghq

A read-scoped project key, so a Stacks dashboard can read logs without a person's credentials

#15
Merged glennmichael123 wants to merge feat/read-scoped-project-key into main
9 files +537 -17

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

app/Models/Project.tsmodified+6-0
@@ -27,6 +27,12 @@ export default defineModel({
2727 // Public ingest key: the SDK sends it as `X-LogHQ-Key` so reports can only
2828 // be written to a project that presents its key (revocable, not a secret).
2929 ingest_key: { fillable: true, validation: { rule: schema.string().optional() }, factory: () => globalThis.crypto.randomUUID().replace(/-/g, '') },
30 // Secret read key: a machine presents it as `Authorization: Bearer` to read
31 // this project without a person's credentials (#14). Not fillable and with
32 // no factory — it is issued on demand from project settings, and revoking
33 // it is setting it back to null. See app/Support/read-key.ts.
34 read_key: { fillable: false, validation: { rule: schema.string().optional() } },
35 read_key_last_used_at: { fillable: false, validation: { rule: schema.string().optional() } },
3036 owner_id: { fillable: true, validation: { rule: schema.number().optional() } },
3137 is_active: { fillable: true, validation: { rule: schema.boolean().optional() }, factory: () => true },
3238 },
app/Support/read-key.tsadded+104-0
@@ -0,0 +1,104 @@
1/**
2 * A project's read key: the credential a machine presents to read that
3 * project's logs.
4 *
5 * The ingest key next to it is deliberately public — it ships inside client
6 * bundles, it only permits writes to one project, and `app/Errors/ingest.ts`
7 * compares it with `===` because nothing is lost by leaking it. A read key is
8 * the opposite on both counts: it returns data, and it is meant to sit in a
9 * server's configuration and nowhere else. So it is longer, it is compared in
10 * constant time, and it is never accepted on an ingest route.
11 *
12 * It authorises exactly one project. There is no account behind it, so it
13 * outlives the person who created it and carries none of their scope — which
14 * is the whole reason it exists (loghqorg/loghq#14): a Stacks dashboard
15 * reading logs server-to-server should not be holding somebody's session.
16 */
17
18/**
19 * `lhq_read_` then 256 bits of hex.
20 *
21 * The prefix is doing real work. It tells a read key apart from a user's
22 * bearer token before either is verified, so a read key is never handed to
23 * `Auth.getUserFromToken`; and it gives secret scanners something to match, so
24 * a key pasted into a repository can be found and revoked.
25 */
26const PREFIX = 'lhq_read_'
27const BODY = /^[0-9a-f]{64}$/
28
29export function newReadKey(): string {
30 const bytes = globalThis.crypto.getRandomValues(new Uint8Array(32))
31 return PREFIX + Array.from(bytes, byte => byte.toString(16).padStart(2, '0')).join('')
32}
33
34/** Whether a presented credential is shaped like a read key at all. */
35export function looksLikeReadKey(value: unknown): value is string {
36 return typeof value === 'string' && value.startsWith(PREFIX) && BODY.test(value.slice(PREFIX.length))
37}
38
39/**
40 * Equal, in time that does not depend on how much of the key is right.
41 *
42 * `===` on a string short-circuits at the first differing byte, which leaks
43 * the length of a correct prefix to anyone who can measure the response. That
44 * is a real attack on a secret an attacker can guess at repeatedly, and not
45 * one on the public ingest key, which is why only this side does it.
46 */
47export function sameKey(presented: string, stored: string): boolean {
48 if (presented.length !== stored.length)
49 return false
50 let diff = 0
51 for (let i = 0; i < presented.length; i++)
52 diff |= presented.charCodeAt(i) ^ stored.charCodeAt(i)
53 return diff === 0
54}
55
56export interface ReadKeyProject {
57 id: string
58 read_key?: string | null
59 is_active?: boolean | number | null
60}
61
62export type ReadAuthorization =
63 | { ok: true }
64 | { ok: false, status: 401 | 403 | 404, error: string }
65
66/**
67 * Whether this key may read this project.
68 *
69 * A project nobody can name is a 404 rather than a 401: answering "that
70 * project exists, your key is wrong" tells an unauthenticated caller which
71 * project ids are real.
72 */
73export function authorizeRead(project: ReadKeyProject | null | undefined, presented: unknown): ReadAuthorization {
74 if (!project)
75 return { ok: false, status: 404, error: 'unknown project' }
76 if (project.is_active === false || project.is_active === 0)
77 return { ok: false, status: 403, error: 'project inactive' }
78 // No key issued is not the same as "any key will do": a project without one
79 // must refuse every presented key, including an empty one.
80 if (!looksLikeReadKey(project.read_key))
81 return { ok: false, status: 403, error: 'project has no read key' }
82 if (!looksLikeReadKey(presented) || !sameKey(presented, project.read_key))
83 return { ok: false, status: 401, error: 'invalid read key' }
84 return { ok: true }
85}
86
87/**
88 * How often a key's last-used stamp is written.
89 *
90 * The stamp is there so somebody can see whether a key is still in use before
91 * revoking it, which a minute's resolution answers. Writing on every request
92 * would put an UPDATE in front of every read of a log stream that a dashboard
93 * polls.
94 */
95export const LAST_USED_RESOLUTION_MS = 60_000
96
97export function shouldRecordUse(lastUsedAt: string | Date | null | undefined, now: Date): boolean {
98 if (!lastUsedAt)
99 return true
100 const previous = lastUsedAt instanceof Date ? lastUsedAt.getTime() : Date.parse(String(lastUsedAt))
101 if (Number.isNaN(previous))
102 return true
103 return now.getTime() - previous >= LAST_USED_RESOLUTION_MS
104}
app/Support/request-auth.tsmodified+73-0
@@ -14,6 +14,8 @@
1414 */
1515import { Auth } from '@stacksjs/auth'
1616import { config } from '@stacksjs/config'
17import { db } from '@stacksjs/database'
18import { authorizeRead, looksLikeReadKey, shouldRecordUse } from './read-key'
1719
1820export async function userFromRequest(request: any): Promise<any | null> {
1921 const authHeader = request.headers?.get?.('authorization') ?? ''
@@ -37,3 +39,74 @@ export async function userFromRequest(request: any): Promise<any | null> {
3739 return null
3840 }
3941}
42
43/**
44 * The bearer a request presents, whatever carries it.
45 *
46 * Split out of `userFromRequest` so a read key can be recognised before it is
47 * sent anywhere near `Auth.getUserFromToken`: the two credentials arrive in
48 * the same header and only their shape tells them apart.
49 */
50function bearerFrom(request: any): string {
51 const authHeader = request.headers?.get?.('authorization') ?? ''
52 const header = request.bearerToken?.() ?? authHeader.replace(/^Bearer\s+/i, '')
53 if (header)
54 return header
55 const cookie = request.headers?.get?.('cookie') ?? ''
56 const name = (config.auth?.defaultTokenName || 'auth-token').replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
57 const m = cookie.match(new RegExp(`(?:^|;)\\s*${name}=([^;]+)`))
58 return m ? decodeURIComponent(m[1]) : ''
59}
60
61export type ProjectReadAccess =
62 | { ok: true, as: 'user', user: any }
63 | { ok: true, as: 'key' }
64 | { ok: false, status: 401 | 403 | 404, error: string }
65
66/**
67 * Who may read this project: the person signed in, or a key issued for it.
68 *
69 * Order matters. A read key is checked first when one is presented, because a
70 * key is scoped to a single project and answering it does not need a user
71 * lookup at all. Anything that is not shaped like a read key falls through to
72 * the existing user path unchanged, so the dashboard's cookie and a person's
73 * bearer token behave exactly as they did.
74 *
75 * A key for a different project answers 404 rather than 403, matching what
76 * `ownsProject` already does: a caller who cannot read a project should not be
77 * able to learn that it exists.
78 */
79export async function projectReadAccess(request: any, projectId: string, ownsProject: (user: any, id: string) => Promise<boolean>): Promise<ProjectReadAccess> {
80 const presented = bearerFrom(request)
81
82 if (looksLikeReadKey(presented)) {
83 const row = (await db.unsafe(
84 'SELECT id, read_key, read_key_last_used_at, is_active FROM projects WHERE id = $1 LIMIT 1',
85 [String(projectId)],
86 ))?.[0] ?? null
87 const verdict = authorizeRead(row, presented)
88 // A key that is valid for some other project must not reveal that this one
89 // exists, so its refusal is the same 404 an unknown id gets.
90 if (!verdict.ok)
91 return verdict.status === 401 ? { ok: false, status: 404, error: 'not found' } : verdict
92 await recordReadKeyUse(row)
93 return { ok: true, as: 'key' }
94 }
95
96 const user = await userFromRequest(request)
97 if (!user)
98 return { ok: false, status: 401, error: 'unauthorized' }
99 if (!(await ownsProject(user, projectId)))
100 return { ok: false, status: 404, error: 'not found' }
101 return { ok: true, as: 'user', user }
102}
103
104/** Stamp the key as used, at most once a minute — see LAST_USED_RESOLUTION_MS. */
105async function recordReadKeyUse(project: { id: string, read_key_last_used_at?: string | null }): Promise<void> {
106 const now = new Date()
107 if (!shouldRecordUse(project.read_key_last_used_at, now))
108 return
109 // Never let the stamp fail a read: it is an aid to revoking a key, not part
110 // of authorising one.
111 await db.unsafe('UPDATE projects SET read_key_last_used_at = $1 WHERE id = $2', [now.toISOString(), String(project.id)]).catch(() => {})
112}
database/migrations/0000000024-alter-projects-read-key.sqladded+25-0
@@ -0,0 +1,25 @@
1-- SQLITE DIALECT, WRITTEN BY HAND. See stacksjs/stacks#2346.
2--
3-- A read-scoped project key, so a machine can read this project's logs without
4-- holding a person's credentials (loghqorg/loghq#14).
5--
6-- `ingest_key` beside it is public and write-only: it ships in client bundles
7-- and only permits writing to one project. This one returns data, so it is a
8-- secret, it is compared in constant time (app/Support/read-key.ts), and the
9-- ingest route never accepts it.
10--
11-- Both columns are nullable because a key is issued on demand rather than at
12-- creation. A project that never needs one never has one, and revoking is
13-- setting the column back to NULL — which is also why the uniqueness below is
14-- a partial index: every revoked project would otherwise collide on NULL.
15--
16-- `read_key_last_used_at` exists so somebody can tell whether a key is still
17-- in use before revoking it. It is written at most once a minute per key
18-- (LAST_USED_RESOLUTION_MS), not on every request: the dashboard polls the log
19-- stream, and an UPDATE in front of every poll would cost more than the stamp
20-- is worth.
21--
22-- Numbered 24, continuing past 0000000023.
23ALTER TABLE "projects" ADD COLUMN "read_key" TEXT;
24ALTER TABLE "projects" ADD COLUMN "read_key_last_used_at" TEXT;
25CREATE UNIQUE INDEX IF NOT EXISTS "projects_read_key_unique" ON "projects" ("read_key") WHERE "read_key" IS NOT NULL;
routes/logs.tsmodified+15-12
@@ -15,9 +15,9 @@ import { rateLimit } from '../app/Errors/limits'
1515import { extractEntries, LEVELS, MAX_BATCH, MAX_CORRELATION, normalizeBatch } from '../app/Logs/normalize'
1616// The owner/member predicate lives in one file so these routes and the pages
1717// that render the same data cannot drift on it. See app/Support/access.ts.
18import { ownsLog, ownsProject } from '../app/Support/access'
18import { ownsProject } from '../app/Support/access'
1919import { healthResponse } from '../app/Support/health'
20import { userFromRequest } from '../app/Support/request-auth'
20import { projectReadAccess } from '../app/Support/request-auth'
2121
2222// Ingest abuse bounds. The public key gate is not enough on its own — a script
2323// with the key (readable from any bundle) could flood the ingest. Per-entry
@@ -137,11 +137,10 @@ const STREAM_COLS = 'id, level, message, channel, environment, release, framewor
137137
138138route.get('/api/projects/{projectId}/logs', async (request: any) => {
139139 const projectId = request.params.projectId
140 const user = await userFromRequest(request)
141 if (!user)
142 return json({ error: 'unauthorized' }, 401)
143 if (!(await ownsProject(user, projectId)))
144 return json({ error: 'not found' }, 404)
140 // A person who owns the project, or a read key issued for it (#14).
141 const access = await projectReadAccess(request, projectId, ownsProject)
142 if (!access.ok)
143 return json({ error: access.error }, access.status)
145144
146145 const q = request.query ?? {}
147146 const where: string[] = ['project_id = $1']
@@ -207,12 +206,16 @@ route.get('/api/projects/{projectId}/logs', async (request: any) => {
207206
208207route.get('/api/logs/{logId}', async (request: any) => {
209208 const logId = request.params.logId
210 const user = await userFromRequest(request)
211 if (!user)
212 return json({ error: 'unauthorized' }, 401)
213 if (!(await ownsLog(user, logId)))
214 return json({ error: 'not found' }, 404)
209 // The entry is read before anyone is authorised, and returned only after.
210 // Authorising first would mean asking "may you read project X?" without
211 // knowing which project this entry is in; answering 404 for a missing entry
212 // before checking the caller would tell an unauthenticated one which log ids
213 // exist. So: fetch, authorise against the entry's project (an empty id when
214 // there is no entry, which no key and no user can read), then answer.
215215 const entry = (await db.unsafe('SELECT * FROM log_entries WHERE id = $1 LIMIT 1', [logId]))?.[0]
216 const access = await projectReadAccess(request, entry ? String(entry.project_id) : '', ownsProject)
217 if (!access.ok)
218 return json({ error: access.error }, access.status)
216219 if (!entry)
217220 return json({ error: 'not found' }, 404)
218221 return json({ entry })
routes/projects.tsmodified+42-0
@@ -17,6 +17,7 @@ import { log } from '@stacksjs/logging'
1717// Shared with the page action in resources/views/projects/new.stx, so the two
1818// callers cannot drift on how an id or an ingest key is minted.
1919import { newIngestKey, newProjectId } from '../app/Support/projects'
20import { newReadKey } from '../app/Support/read-key'
2021import { utcNow } from '../app/Support/time'
2122import { GitHubRequestError, verifyAccess } from '../app/Fix/github'
2223import {
@@ -114,6 +115,9 @@ route.get('/api/projects', async (request: any) => {
114115 return json({ error: 'unauthorized' }, 401)
115116 const rows = await db.unsafe(
116117 `SELECT DISTINCT p.id, p.name, p.platform, p.ingest_key, p.is_active, p.created_at,
118 -- Whether a read key exists and when it was last used, never the
119 -- key: it is a secret, and it is shown once when it is issued.
120 (p.read_key IS NOT NULL) AS has_read_key, p.read_key_last_used_at,
117121 (p.owner_id = $1) AS is_owner
118122 FROM projects p
119123 LEFT JOIN project_members m ON m.project_id = p.id AND lower(m.email) = $2
@@ -183,6 +187,44 @@ route.post('/api/projects/{projectId}/rotate-key', async (request: any) => {
183187 return json({ ingest_key: key })
184188}).skipCsrf()
185189
190// Issue (or replace) a project's read key. Owner-only.
191//
192// This is the credential a server presents to read the project's logs without
193// a person's account behind it (#14). It is returned here and nowhere else:
194// unlike the public ingest key, which the listing carries so it can be copied
195// into client code, this one is a secret, so it is shown once and the listing
196// afterwards says only that it exists. Losing it means issuing another, which
197// is the same call — and the previous key stops working the moment it lands.
198route.post('/api/projects/{projectId}/read-key', async (request: any) => {
199 const user = await currentUser(request)
200 if (!user)
201 return json({ error: 'unauthorized' }, 401)
202 const projectId = request.params.projectId
203 if (!(await ownsProject(user, projectId)))
204 return json({ error: 'not found' }, 404)
205
206 const key = newReadKey()
207 // The last-used stamp belongs to the key, so a new key starts with none:
208 // leaving the old one would read as "this key was used before it existed".
209 await db.unsafe('UPDATE projects SET read_key = $1, read_key_last_used_at = NULL, updated_at = $3 WHERE id = $2', [key, projectId, utcNow()])
210 return json({ read_key: key })
211}).skipCsrf()
212
213// Revoke a project's read key. Owner-only. Anything still presenting it gets a
214// 404 from the read API on the next request, the same answer an unknown
215// project gets.
216route.delete('/api/projects/{projectId}/read-key', async (request: any) => {
217 const user = await currentUser(request)
218 if (!user)
219 return json({ error: 'unauthorized' }, 401)
220 const projectId = request.params.projectId
221 if (!(await ownsProject(user, projectId)))
222 return json({ error: 'not found' }, 404)
223
224 await db.unsafe('UPDATE projects SET read_key = NULL, read_key_last_used_at = NULL, updated_at = $2 WHERE id = $1', [projectId, utcNow()])
225 return json({ revoked: true })
226}).skipCsrf()
227
186228// Delete a project and everything under it. Owner-only. Removes children first
187229// to satisfy the foreign keys (log entries/channels/members/invites -> project),
188230// then the project row.
tests/unit/read-key.test.tsadded+120-0
@@ -0,0 +1,120 @@
1import { describe, expect, test } from 'bun:test'
2import { authorizeRead, LAST_USED_RESOLUTION_MS, looksLikeReadKey, newReadKey, sameKey, shouldRecordUse } from '../../app/Support/read-key'
3
4const KEY = newReadKey()
5const project = { id: 'demo', read_key: KEY, is_active: true }
6
7describe('newReadKey', () => {
8 test('is prefixed and carries 256 bits of hex', () => {
9 expect(KEY.startsWith('lhq_read_')).toBe(true)
10 expect(KEY.slice('lhq_read_'.length)).toMatch(/^[0-9a-f]{64}$/)
11 })
12
13 test('does not repeat', () => {
14 const keys = new Set(Array.from({ length: 200 }, () => newReadKey()))
15 expect(keys.size).toBe(200)
16 })
17})
18
19describe('looksLikeReadKey', () => {
20 test('accepts a key it minted', () => {
21 expect(looksLikeReadKey(KEY)).toBe(true)
22 })
23
24 test('rejects anything else a bearer header might carry', () => {
25 // The prefix is what keeps a read key out of Auth.getUserFromToken, so a
26 // user's token and an ingest key must never pass for one.
27 for (const value of [
28 '',
29 null,
30 undefined,
31 'loghq_5f3a19b7c4d2e8091ab12cd34ef56ab7',
32 'eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxIn0.sig',
33 `${'lhq_read_'}${'z'.repeat(64)}`,
34 `${'lhq_read_'}${'a'.repeat(63)}`,
35 `${'lhq_read_'}${'a'.repeat(65)}`,
36 KEY.toUpperCase(),
37 ])
38 expect(looksLikeReadKey(value)).toBe(false)
39 })
40})
41
42describe('sameKey', () => {
43 test('matches an identical key and nothing else', () => {
44 expect(sameKey(KEY, KEY)).toBe(true)
45 expect(sameKey(KEY, newReadKey())).toBe(false)
46 })
47
48 test('is false on a shared prefix, not just a differing first byte', () => {
49 const other = `${KEY.slice(0, -1)}${KEY.endsWith('a') ? 'b' : 'a'}`
50 expect(sameKey(other, KEY)).toBe(false)
51 })
52
53 test('does not throw on a length mismatch', () => {
54 expect(sameKey('short', KEY)).toBe(false)
55 expect(sameKey(KEY, 'short')).toBe(false)
56 })
57})
58
59describe('authorizeRead', () => {
60 test('allows the project its own key', () => {
61 expect(authorizeRead(project, KEY)).toEqual({ ok: true })
62 })
63
64 test('refuses an unknown project with 404', () => {
65 expect(authorizeRead(null, KEY)).toEqual({ ok: false, status: 404, error: 'unknown project' })
66 expect(authorizeRead(undefined, KEY)).toEqual({ ok: false, status: 404, error: 'unknown project' })
67 })
68
69 test('refuses an inactive project with 403, however is_active is stored', () => {
70 // sqlite hands booleans back as 0/1.
71 expect(authorizeRead({ ...project, is_active: false }, KEY).ok).toBe(false)
72 expect(authorizeRead({ ...project, is_active: 0 }, KEY)).toEqual({ ok: false, status: 403, error: 'project inactive' })
73 })
74
75 test('refuses a project with no key issued, whatever is presented', () => {
76 // Guards `null === null`: no key configured must never authorise, not even
77 // against a caller presenting nothing.
78 for (const presented of [KEY, null, undefined, ''])
79 expect(authorizeRead({ ...project, read_key: null }, presented).ok).toBe(false)
80 expect(authorizeRead({ ...project, read_key: null }, KEY).status).toBe(403)
81 })
82
83 test('refuses another project\'s key with 401', () => {
84 expect(authorizeRead(project, newReadKey())).toEqual({ ok: false, status: 401, error: 'invalid read key' })
85 })
86
87 test('refuses an ingest key, which grants writes and must not grant reads', () => {
88 expect(authorizeRead({ ...project, read_key: KEY }, 'loghq_5f3a19b7c4d2e8091ab12cd34ef56ab7').ok).toBe(false)
89 })
90
91 test('refuses a stored value that is not a well-formed key', () => {
92 // A column hand-edited to something short must not become a weak key.
93 expect(authorizeRead({ ...project, read_key: 'x' }, 'x').ok).toBe(false)
94 })
95})
96
97describe('shouldRecordUse', () => {
98 const now = new Date('2026-10-19T12:00:00.000Z')
99
100 test('records the first use', () => {
101 expect(shouldRecordUse(null, now)).toBe(true)
102 expect(shouldRecordUse(undefined, now)).toBe(true)
103 })
104
105 test('records again once the resolution has passed, and not before', () => {
106 const justBefore = new Date(now.getTime() - LAST_USED_RESOLUTION_MS + 1).toISOString()
107 const atResolution = new Date(now.getTime() - LAST_USED_RESOLUTION_MS).toISOString()
108 expect(shouldRecordUse(justBefore, now)).toBe(false)
109 expect(shouldRecordUse(atResolution, now)).toBe(true)
110 })
111
112 test('records when the stored stamp cannot be read', () => {
113 // Better an extra write than a key that never looks used.
114 expect(shouldRecordUse('not a date', now)).toBe(true)
115 })
116
117 test('accepts a Date as well as a string', () => {
118 expect(shouldRecordUse(new Date(now.getTime() - 1000), now)).toBe(false)
119 })
120})