Add notification image proxy, expand README, fix compose env-file handling
- Proxy notification image_url server-side (GET /api/v1/notifications/:id/image) instead of loading it directly in the browser, so LAN-only image sources (e.g. Frigate snapshots) still load for clients off that network - Tighten CSP img-src to 'self' now that images are same-origin - Update threat model / architecture docs to match the new image-fetch design - Expand README: Home Assistant quick-start, Pocket ID/OIDC walkthrough, Pangolin note, backup automation, new troubleshooting entries - Fix docker-compose.yml: document that --env-file .env must be passed explicitly (Compose's implicit .env lookup was unreliable with -f from a different directory) - Fix image-proxy tests to build a real session cookie (AUTH_MODE=oidc applies inside the test container too, so requireAuth needs one)
This commit is contained in:
1 parent
a82c29a69d
commit
079013730b
10 files changed
+332
-32
No files matched your search
@@ -43,19 +43,22 @@ export async function buildWebApp(): Promise<FastifyInstance> {
|
||||
const app = Fastify({ logger: loggerOptions, bodyLimit: 1024 * 1024 })
|
||||
|
||||
// CSP/HSTS/frame-ancestors per docs/phase1/00-README.md's security
|
||||
// requirements. crossOriginEmbedderPolicy is off because notification
|
||||
// image_url values are arbitrary third-party https URLs the browser
|
||||
// loads directly (docs/phase1/02-threat-model.md's SSRF mitigation) —
|
||||
// COEP would require CORP headers from origins we don't control. HSTS is
|
||||
// only sent in production so a plain-HTTP dev/LAN setup never gets stuck
|
||||
// with a stale HSTS pin from testing.
|
||||
// requirements. imgSrc is 'self' only — notification images are proxied
|
||||
// through GET /api/v1/notifications/:id/image (modules/notifications/routes.ts)
|
||||
// rather than the browser loading notification.imageUrl directly, so the
|
||||
// browser never needs to reach an arbitrary third-party host itself. That
|
||||
// also closes a minor privacy leak the old direct-load approach had (a
|
||||
// topic's image_url could otherwise act as a read-receipt/tracking pixel
|
||||
// revealing the viewer's IP to whatever host it pointed at). HSTS is only
|
||||
// sent in production so a plain-HTTP dev/LAN setup never gets stuck with
|
||||
// a stale HSTS pin from testing.
|
||||
await app.register(fastifyHelmet, {
|
||||
contentSecurityPolicy: {
|
||||
directives: {
|
||||
defaultSrc: ["'self'"],
|
||||
scriptSrc: ["'self'"],
|
||||
styleSrc: ["'self'", "'unsafe-inline'"],
|
||||
imgSrc: ["'self'", 'https:', 'data:'],
|
||||
imgSrc: ["'self'"],
|
||||
connectSrc: ["'self'"],
|
||||
fontSrc: ["'self'"],
|
||||
objectSrc: ["'none'"],
|
||||
@@ -64,6 +67,10 @@ export async function buildWebApp(): Promise<FastifyInstance> {
|
||||
formAction: ["'self'"],
|
||||
},
|
||||
},
|
||||
// Left off rather than flipped on speculatively — everything the app
|
||||
// loads is same-origin now, so COEP's require-corp default would
|
||||
// likely be safe, but that's an untested behavior change and not what
|
||||
// prompted this edit; revisit deliberately if it's ever worth it.
|
||||
crossOriginEmbedderPolicy: false,
|
||||
hsts: config.isProduction ? { maxAge: 15552000, includeSubDomains: true } : false,
|
||||
})
|
||||
|
||||
@@ -0,0 +1,147 @@
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import http from 'node:http'
|
||||
import type { AddressInfo } from 'node:net'
|
||||
import type { FastifyInstance } from 'fastify'
|
||||
import { eq } from 'drizzle-orm'
|
||||
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
|
||||
import { db, pool } from '../../db/client.js'
|
||||
import { runMigrations } from '../../db/migrate.js'
|
||||
import { notifications, topics, users } from '../../db/schema.js'
|
||||
import { buildWebApp } from '../../listeners/web.js'
|
||||
import { encryptJson } from '../../lib/crypto-session.js'
|
||||
import { getSetting } from '../../db/settings.js'
|
||||
import { createNotification } from './repository.js'
|
||||
|
||||
// Integration test — same requirements/run command as
|
||||
// modules/tokens/repository.integration.test.ts. Runs against a tiny local
|
||||
// HTTP server standing in for a LAN camera/service, so it doesn't depend on
|
||||
// network access or a real Frigate instance being up.
|
||||
//
|
||||
// The image endpoint requires a real session (requireAuth calls
|
||||
// getSessionUser whenever AUTH_MODE=oidc, which is what this test's own
|
||||
// container runs under) — a plain app.inject() with no cookie gets 401, so
|
||||
// each request below carries a hand-built session cookie, encrypted the
|
||||
// same way modules/auth/session.ts does it against the same persisted
|
||||
// session_secret.
|
||||
describe('notification image proxy', () => {
|
||||
let app: FastifyInstance
|
||||
let upstream: http.Server
|
||||
let upstreamPort: number
|
||||
let sessionCookie: string
|
||||
const testSlug = `test-image-proxy-${randomUUID()}`
|
||||
const testAuthSubject = `test-image-proxy-user-${randomUUID()}`
|
||||
let userId: string
|
||||
let topicId: string
|
||||
|
||||
const pngBytes = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a])
|
||||
|
||||
beforeAll(async () => {
|
||||
await runMigrations()
|
||||
app = await buildWebApp()
|
||||
await app.ready()
|
||||
|
||||
upstream = http.createServer((req, res) => {
|
||||
if (req.url === '/image.png') {
|
||||
res.writeHead(200, { 'Content-Type': 'image/png' })
|
||||
res.end(pngBytes)
|
||||
} else if (req.url === '/not-an-image') {
|
||||
res.writeHead(200, { 'Content-Type': 'text/html' })
|
||||
res.end('<html></html>')
|
||||
} else {
|
||||
res.writeHead(404)
|
||||
res.end()
|
||||
}
|
||||
})
|
||||
await new Promise<void>((resolve) => upstream.listen(0, '127.0.0.1', resolve))
|
||||
upstreamPort = (upstream.address() as AddressInfo).port
|
||||
|
||||
const [user] = await db
|
||||
.insert(users)
|
||||
.values({
|
||||
email: `${testAuthSubject}@example.test`,
|
||||
displayName: 'Test User',
|
||||
authSubject: testAuthSubject,
|
||||
role: 'admin',
|
||||
})
|
||||
.returning()
|
||||
userId = user!.id
|
||||
|
||||
const [topic] = await db
|
||||
.insert(topics)
|
||||
.values({ slug: testSlug, displayName: 'Test Topic', createdBy: userId })
|
||||
.returning()
|
||||
topicId = topic!.id
|
||||
|
||||
// Same key derivation as modules/auth/session.ts's registerSession —
|
||||
// that ran as part of buildWebApp() above, so the secret already
|
||||
// exists in system_settings by this point.
|
||||
const secretHex = await getSetting<string>('session_secret')
|
||||
const sessionKey = Buffer.from(secretHex!, 'hex')
|
||||
sessionCookie = encryptJson(sessionKey, { userId })
|
||||
})
|
||||
|
||||
afterAll(async () => {
|
||||
await db.delete(notifications).where(eq(notifications.topicId, topicId))
|
||||
await db.delete(topics).where(eq(topics.id, topicId))
|
||||
await db.delete(users).where(eq(users.id, userId))
|
||||
await new Promise<void>((resolve) => upstream.close(() => resolve()))
|
||||
await app.close()
|
||||
await pool.end()
|
||||
})
|
||||
|
||||
it('proxies an image from the stored imageUrl', async () => {
|
||||
const result = await createNotification({
|
||||
topicSlug: testSlug,
|
||||
source: 'test',
|
||||
title: 'Has image',
|
||||
message: 'test',
|
||||
imageUrl: `http://127.0.0.1:${upstreamPort}/image.png`,
|
||||
})
|
||||
if ('error' in result) throw new Error('unexpected error creating notification')
|
||||
|
||||
const res = await app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/v1/notifications/${result.notification.id}/image`,
|
||||
headers: { cookie: `notizarr_session=${sessionCookie}` },
|
||||
})
|
||||
|
||||
expect(res.statusCode).toBe(200)
|
||||
expect(res.headers['content-type']).toBe('image/png')
|
||||
expect(res.rawPayload.equals(pngBytes)).toBe(true)
|
||||
})
|
||||
|
||||
it('returns 404 for a notification with no imageUrl', async () => {
|
||||
const result = await createNotification({
|
||||
topicSlug: testSlug,
|
||||
source: 'test',
|
||||
title: 'No image',
|
||||
message: 'test',
|
||||
})
|
||||
if ('error' in result) throw new Error('unexpected error creating notification')
|
||||
|
||||
const res = await app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/v1/notifications/${result.notification.id}/image`,
|
||||
headers: { cookie: `notizarr_session=${sessionCookie}` },
|
||||
})
|
||||
expect(res.statusCode).toBe(404)
|
||||
})
|
||||
|
||||
it('returns 502 when the upstream response is not an image', async () => {
|
||||
const result = await createNotification({
|
||||
topicSlug: testSlug,
|
||||
source: 'test',
|
||||
title: 'Bad upstream',
|
||||
message: 'test',
|
||||
imageUrl: `http://127.0.0.1:${upstreamPort}/not-an-image`,
|
||||
})
|
||||
if ('error' in result) throw new Error('unexpected error creating notification')
|
||||
|
||||
const res = await app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/v1/notifications/${result.notification.id}/image`,
|
||||
headers: { cookie: `notizarr_session=${sessionCookie}` },
|
||||
})
|
||||
expect(res.statusCode).toBe(502)
|
||||
})
|
||||
})
|
||||
@@ -1,3 +1,5 @@
|
||||
import { Readable } from 'node:stream'
|
||||
import type { ReadableStream as NodeWebReadableStream } from 'node:stream/web'
|
||||
import type { FastifyInstance } from 'fastify'
|
||||
import { requireAdmin, requireAuth } from '../auth/guards.js'
|
||||
import { writeAuditLog } from '../audit/log.js'
|
||||
@@ -116,6 +118,68 @@ export async function registerNotificationReadRoutes(app: FastifyInstance) {
|
||||
},
|
||||
)
|
||||
|
||||
// Proxies notification.imageUrl through the server instead of having the
|
||||
// client's browser load it directly. Matters because that URL is often a
|
||||
// LAN-only camera/service address (e.g. Frigate) — the server sits on
|
||||
// that LAN persistently, but a phone off-network doesn't. Deliberately
|
||||
// narrower than a general-purpose fetch proxy: there's no client-supplied
|
||||
// URL parameter, only the fixed value already stored on a notification
|
||||
// the caller is authorized to see (publishing one at all requires a
|
||||
// valid, scoped API token), which is what keeps this from being an open
|
||||
// SSRF endpoint.
|
||||
app.get(
|
||||
'/api/v1/notifications/:id/image',
|
||||
{
|
||||
preHandler: requireAuth,
|
||||
schema: {
|
||||
params: { type: 'object', properties: { id: { type: 'string' } }, required: ['id'] },
|
||||
},
|
||||
},
|
||||
async (req, reply) => {
|
||||
const { id } = req.params as { id: string }
|
||||
const notification = await getNotificationForUser(req.currentUser!.id, id)
|
||||
if (!notification?.imageUrl) {
|
||||
reply.code(404)
|
||||
return { error: 'not_found' }
|
||||
}
|
||||
|
||||
const controller = new AbortController()
|
||||
const timeout = setTimeout(() => controller.abort(), 10_000)
|
||||
try {
|
||||
const upstream = await fetch(notification.imageUrl, { signal: controller.signal })
|
||||
if (!upstream.ok || !upstream.body) {
|
||||
reply.code(502)
|
||||
return { error: 'upstream_fetch_failed' }
|
||||
}
|
||||
|
||||
const contentType = upstream.headers.get('content-type') ?? ''
|
||||
if (!contentType.startsWith('image/')) {
|
||||
reply.code(502)
|
||||
return { error: 'not_an_image', message: `upstream returned ${contentType || 'no content-type'}` }
|
||||
}
|
||||
|
||||
const contentLength = Number(upstream.headers.get('content-length') ?? 0)
|
||||
if (contentLength > 20 * 1024 * 1024) {
|
||||
reply.code(502)
|
||||
return { error: 'image_too_large' }
|
||||
}
|
||||
|
||||
reply.header('Content-Type', contentType)
|
||||
reply.header('Cache-Control', 'private, max-age=300')
|
||||
// DOM lib's fetch() types ReadableStream from lib.dom, Readable.fromWeb
|
||||
// expects node:stream/web's — structurally identical at runtime, this
|
||||
// cast just bridges the two type declarations.
|
||||
return Readable.fromWeb(upstream.body as unknown as NodeWebReadableStream<Uint8Array>)
|
||||
} catch (err) {
|
||||
req.log.warn(err, 'notification image proxy fetch failed')
|
||||
reply.code(502)
|
||||
return { error: 'upstream_fetch_failed' }
|
||||
} finally {
|
||||
clearTimeout(timeout)
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
app.patch(
|
||||
'/api/v1/notifications/:id',
|
||||
{
|
||||
|
||||
Reference in new issue
Block a user