hearthforge

harden low-severity review findings across git, CI, and auth

- raw: also serve SVG as text/plain (same XSS class as HTML)
- git: log unexpected failures in content/list ops (not existence checks)
- git: use mkdtemp for patch/edit temp files instead of predictable /tmp paths
- git: preserve existing file mode (exec bit, symlink) on edit/move
- ci: allocate repo_run_id from an atomic per-repo counter table so run
  numbers never collide or repeat after history pruning
- ci: purge cache volumes on repo delete/rename
- git: check tar/zstd exit codes in archiveRepo; drop partial .tar.zst
- auth: pin WebAuthn origin/RP-ID to BASE_URL, not the client Origin header
- test runner: retry only on the futex stall, never on real failures
- schema.sql: add repo_run_id + ci_run_counters, drop dead git_name/git_email

AuthorKonata <konata@posteo.jp>

Date2026-07-10 06:36:07 UTC

Commit56123391517390e7bb70ecdac394c54ffc488a66

Parent4524187

7 files changed, 211 insertions(+), 86 deletions(-)

▾Mscripts/test.ts

+12-5@ 5612339 @ master

@@ -10,16 +10,21 @@ const files = readdirSync(testsDir)

const STALL_TIMEOUT = 20_000; // kill if no output for 20s

const MAX_RETRIES = 3;

// retry logic needed because tests get randomly get stuck on startup with bun

// strace shows bun completely spinning in futex and not doing anything else

function runTest(filePath: string): Promise<boolean> {

// strace shows bun completely spinning in futex and not doing anything else.

// Only a stall-kill is retried — a genuine assertion failure returns

// {ok: false, stalled: false} and must NOT be retried, or a real product bug

// that fails intermittently would be laundered into a pass.

function runTest(filePath: string): Promise<{ ok: boolean; stalled: boolean }> {

return new Promise((resolve) => {

const child = spawn('bun', ['test', '--bail=1', '--timeout', '30000', filePath], {

stdio: ['ignore', 'pipe', 'pipe'],

});

let stalled = false;

let timer = setTimeout(onStall, STALL_TIMEOUT);

function onStall() {

stalled = true;

console.error(`\n[test-runner] stall detected, killing ${path.basename(filePath)} (no output for ${STALL_TIMEOUT / 1000}s)`);

child.kill('SIGKILL');

}

@@ -40,7 +45,7 @@ function runTest(filePath: string): Promise<boolean> {

child.on('close', (code) => {

clearTimeout(timer);

resolve(code === 0);

resolve({ ok: code === 0, stalled });

});

}

@@ -56,8 +61,10 @@ for (const file of files) {

if (attempt > 1) {

console.log(`[test-runner] retrying ${file} (attempt ${attempt}/${MAX_RETRIES})`);

}

ok = await runTest(filePath);

if (ok) break;

const result = await runTest(filePath);

ok = result.ok;

// Retry only the futex stall — a genuine failure is final.

if (ok || !result.stalled) break;

}

if (ok) passed++;

▾Msrc/db/index.ts

+13@ 5612339 @ master

@@ -198,6 +198,14 @@ interface CiSecretTable {

created_at: Generated<string>;

}

// Per-repo monotonic counter for the human-facing run number (#1, #2, …).

// Incremented atomically on each trigger so numbers never collide or repeat

// after history pruning — unlike deriving the number from a live row count.

interface CiRunCounterTable {

repo_id: number;

last_run_id: number;

}

export interface Database {

users: UserTable;

passkeys: PasskeyTable;

@@ -219,6 +227,7 @@ export interface Database {

ci_steps: CiStepTable;

ci_artifacts: CiArtifactTable;

ci_secrets: CiSecretTable;

ci_run_counters: CiRunCounterTable;

}

// Selectable row types (id is plain number, as returned by queries)

@@ -345,6 +354,10 @@ sqlite.run(`CREATE TABLE IF NOT EXISTS ci_secrets (

created_at TEXT NOT NULL DEFAULT (datetime('now')),

UNIQUE(repo_id, name)

)`);

sqlite.run(`CREATE TABLE IF NOT EXISTS ci_run_counters (

repo_id INTEGER PRIMARY KEY REFERENCES repositories(id) ON DELETE CASCADE,

last_run_id INTEGER NOT NULL DEFAULT 0

)`);

// Run column-level migrations now that the CI tables are guaranteed to exist

// (see the note above runMigrations).

▾Msrc/db/schema.sql

+7-3@ 5612339 @ master

@@ -4,8 +4,6 @@ CREATE TABLE IF NOT EXISTS users (

password_hash TEXT,

created_at TEXT NOT NULL,

avatar_version INTEGER NOT NULL DEFAULT 1,

git_name TEXT,

git_email TEXT,

is_pending INTEGER NOT NULL DEFAULT 0,

register_application TEXT

);

@@ -170,7 +168,8 @@ CREATE TABLE IF NOT EXISTS ci_runs (

variable_overrides TEXT,

started_at TEXT,

finished_at TEXT,

created_at TEXT NOT NULL DEFAULT (datetime('now'))

created_at TEXT NOT NULL DEFAULT (datetime('now')),

repo_run_id INTEGER

);

CREATE TABLE IF NOT EXISTS ci_steps (

@@ -201,3 +200,8 @@ CREATE TABLE IF NOT EXISTS ci_secrets (

created_at TEXT NOT NULL DEFAULT (datetime('now')),

UNIQUE(repo_id, name)

);

CREATE TABLE IF NOT EXISTS ci_run_counters (

repo_id INTEGER PRIMARY KEY REFERENCES repositories(id) ON DELETE CASCADE,

last_run_id INTEGER NOT NULL DEFAULT 0

);

▾Msrc/routes/auth.tsx

+8-4@ 5612339 @ master

@@ -29,10 +29,14 @@ import { Login } from "../views/auth/Login.tsx";

import { Register } from "../views/auth/Register.tsx";

import { html } from "../views/render.tsx";

function rpFromRequest(request: Request): { origin: string; rpId: string } {

const origin = request.headers.get("origin");

if (!origin) throw new Error("Missing Origin header");

return { origin, rpId: new URL(origin).hostname };

// WebAuthn's expected origin and RP ID are pinned to the configured public

// origin (BASE_URL), never derived from the client's Origin header — otherwise

// the server-side origin check in verification validates the value against

// itself and becomes a no-op. The browser must be on this origin for passkeys

// to work, which is the intended production posture (set BASE_URL).

const PUBLIC_RP_ID = new URL(config.PUBLIC_ORIGIN).hostname;

function rpFromRequest(_request: Request): { origin: string; rpId: string } {

return { origin: config.PUBLIC_ORIGIN, rpId: PUBLIC_RP_ID };

}

function randomHex(bytes: number): string {

▾Msrc/routes/repos.tsx

+24-10@ 5612339 @ master

@@ -20,6 +20,7 @@ import { db } from "../db/index.ts";

import { contentDisposition } from "../lib/contentDisposition.ts";

import { redirect } from "../lib/redirect.ts";

import { requireAdmin, resolveSession } from "../middleware/session.ts";

import { purgeRepoCaches } from "../services/ci.ts";

import {

git,

invalidateRefCache,

@@ -165,18 +166,24 @@ async function mimeForContent(

}

// A repo file opened directly via /raw is served from the forge's own origin.

// HTML would render as a document there and, despite our CSP, could load a

// same-origin `<script src>` pointing at another raw file — a stored-XSS path

// through the normal patch-merge flow. Serve HTML as plain text so it can't

// execute; every other type keeps its real MIME so media previews and

// downloads still work. Paired with `X-Content-Type-Options: nosniff` (set

// globally) so a text/plain body can't be sniffed back into HTML.

// HTML and SVG would render as active documents there and, despite our CSP,

// could load a same-origin `<script src>` pointing at another raw file — a

// stored-XSS path through the normal patch-merge flow. Serve those as plain

// text so they can't execute; every other type keeps its real MIME so media

// previews and downloads still work. (SVG is never shown via <img> in the

// blob view — it renders as highlighted source — so this costs no preview.)

// Paired with `X-Content-Type-Options: nosniff` (set globally) so a

// text/plain body can't be sniffed back into HTML.

const RAW_INERT_TYPES = new Set([

"text/html",

"application/xhtml+xml",

"image/svg+xml",

]);

function rawServeContentType(contentType: string): string {

const base = contentType.split(";")[0]!.trim().toLowerCase();

if (base === "text/html" || base === "application/xhtml+xml") {

return "text/plain; charset=utf-8";

}

return contentType;

return RAW_INERT_TYPES.has(base)

? "text/plain; charset=utf-8"

: contentType;

}

const README_NAMES = ["README.md", "readme.md", "README", "readme"];

@@ -1075,6 +1082,9 @@ export const repoRoutes = new Elysia()

// we abort before touching the DB so the repo remains accessible.

rmSync(repoPath(repo.name), { recursive: true, force: true });

await db.deleteFrom("repositories").where("id", "=", repo.id).execute();

// Reclaim the repo's CI cache volumes (labeled by repo name), which the

// DB cascade doesn't touch. Best-effort — don't block the redirect.

purgeRepoCaches(repo.name).catch(() => {});

return new Response(null, { status: 302, headers: { Location: "/" } });

})

@@ -1144,6 +1154,10 @@ export const repoRoutes = new Elysia()

}

invalidateRefCache(oldName);

// CI cache volumes are labeled with the old repo name and would

// otherwise detach (a run under the new name can't find them).

// Purge them so caches rebuild cleanly under the new name.

purgeRepoCaches(oldName).catch(() => {});

return redirect(

`/${newName}/settings?success=${encodeURIComponent("Repository renamed.")}`,

);

▾Msrc/services/ci.ts

+14-5@ 5612339 @ master

@@ -1069,14 +1069,23 @@ export async function triggerRun(

.returning("id")

.executeTakeFirstOrThrow();

const countRow = await db

.selectFrom("ci_runs")

.select(db.fn.countAll<number>().as("c"))

.where("repo_id", "=", repo.id)

// Allocate the human-facing run number atomically from a per-repo counter.

// A single upsert-and-increment can't collide under concurrent triggers and

// never reuses a number after pruneHistory shrinks the run table — both of

// which a COUNT(*)-based scheme suffered from.

const counter = await db

.insertInto("ci_run_counters")

.values({ repo_id: repo.id, last_run_id: 1 })

.onConflict((oc) =>

oc.column("repo_id").doUpdateSet((eb) => ({

last_run_id: eb("ci_run_counters.last_run_id", "+", 1),

})),

)

.returning("last_run_id")

.executeTakeFirstOrThrow();

await db

.updateTable("ci_runs")

.set({ repo_run_id: Number(countRow.c) })

.set({ repo_run_id: counter.last_run_id })

.where("id", "=", runId.id)

.execute();

▾Msrc/services/git.ts

+133-59@ 5612339 @ master

@@ -1,3 +1,5 @@

import { mkdtempSync, rmSync } from "node:fs";

import os from "node:os";

import path from "node:path";

import { $ as _$ } from "bun";

@@ -59,6 +61,49 @@ export function repoPath(name: string): string {

return path.join(paths.REPOS_DIR, `${name}.git`);

}

// Log an unexpected git failure. Many git calls legitimately fail for benign

// reasons (a ref that doesn't exist yet, an empty repo), so callers still

// swallow the error and return an empty result — but we surface it here so a

// corrupted repo, permission problem, or missing binary isn't completely

// invisible.

function logGitError(op: string, name: string, err: unknown): void {

console.error(`[git] ${op} failed for ${name}:`, err);

}

// Create a private, uniquely-named temp directory (mode 0700, created

// atomically by the OS) and remove it afterward. Replaces predictable

// /tmp/hf-*-<time>-<rand> paths, which on a shared host were open to a

// pre-planted symlink redirecting our writes.

async function withTempDir<T>(

prefix: string,

fn: (dir: string) => Promise<T>,

): Promise<T> {

const dir = mkdtempSync(path.join(os.tmpdir(), `hf-${prefix}-`));

try {

return await fn(dir);

} finally {

rmSync(dir, { recursive: true, force: true });

}

// The 6-digit octal mode of a path at a given ref (e.g. "100644", "100755",

// "120000"), or null if it doesn't exist there. Used to preserve the

// executable bit / symlink type across UI edits instead of forcing 100644.

async function treeFileMode(

p: string,

ref: string,

filePath: string,

): Promise<string | null> {

try {

const out =

await $`git -C ${p} ls-tree --end-of-options ${ref} -- ${filePath}`.text();

const mode = out.split(/\s+/)[0];

return mode && /^\d{6}$/.test(mode) ? mode : null;

} catch {

return null;

}

export async function validateCommit(

repoName: string,

hash: string,

@@ -114,6 +159,11 @@ export async function archiveRepo(

if ((await tgz.exited) !== 0)

throw new Error("git archive (tar.gz) failed");

// The .tar.zst is a best-effort bonus format: if zstd is missing the spawn

// throws and we skip it. But if zstd IS present and fails (disk full,

// refusing to overwrite, …) we must not leave a truncated artifact behind

// silently — log it and remove the partial file.

const zstPath = path.join(outDir, `${base}.tar.zst`);

try {

const tar = Bun.spawn(

[

@@ -127,13 +177,21 @@ export async function archiveRepo(

{ signal, env: gitEnv, stdout: "pipe" },

);

const zst = Bun.spawn(

["zstd", "-o", path.join(outDir, `${base}.tar.zst`)],

{ signal, env: gitEnv, stdin: tar.stdout },

);

await Promise.all([tar.exited, zst.exited]);

const zst = Bun.spawn(["zstd", "-f", "-o", zstPath], {

signal,

env: gitEnv,

stdin: tar.stdout,

});

const [tarCode, zstCode] = await Promise.all([tar.exited, zst.exited]);

if (tarCode !== 0 || zstCode !== 0) {

console.error(

`[git] archive (tar.zst) failed for ${repoName}@${ref}: tar=${tarCode} zstd=${zstCode}`,

);

await $`rm -f ${zstPath}`.quiet().nothrow();

}

} catch {

// zstd not available — skip silently

await $`rm -f ${zstPath}`.quiet().nothrow();

}

@@ -317,7 +375,8 @@ export const git = {

const out =

await $`git ${sigArgs} -C ${p} log --format=%H%x1f%s%x1f%an%x1f%ai%x1f%G? --max-count=${limit} --skip=${skip} --end-of-options ${ref}`.text();

return parseLog(out);

} catch {

} catch (e) {

logGitError(`log(${ref})`, name, e);

return [];

}

@@ -363,7 +422,8 @@ export const git = {

}));

}

return entries;

} catch {

} catch (e) {

logGitError(`lsTree(${ref})`, name, e);

return [];

}

@@ -378,7 +438,8 @@ export const git = {

const buf =

await $`git -C ${p} show --end-of-options ${`${ref}:${filePath}`}`.arrayBuffer();

return Buffer.from(buf);

} catch {

} catch (e) {

logGitError(`show(${ref}:${filePath})`, name, e);

return null;

}

@@ -387,7 +448,8 @@ export const git = {

const p = repoPath(name);

try {

return await $`git -C ${p} diff-tree --no-commit-id -r -p -M --root --end-of-options ${sha}`.text();

} catch {

} catch (e) {

logGitError(`diff(${sha})`, name, e);

return "";

}

@@ -419,7 +481,8 @@ export const git = {

branchCache.delete(branchCache.keys().next().value!);

}

return value;

} catch {

} catch (e) {

logGitError("branches", name, e);

return [];

}

@@ -439,7 +502,8 @@ export const git = {

tagCache.delete(tagCache.keys().next().value!);

}

return value;

} catch {

} catch (e) {

logGitError("tags", name, e);

return [];

}

@@ -469,7 +533,8 @@ export const git = {

date: parts[4] ?? "",

};

});

} catch {

} catch (e) {

logGitError("branchesWithInfo", name, e);

return [];

}

@@ -501,7 +566,8 @@ export const git = {

isAnnotated,

};

});

} catch {

} catch (e) {

logGitError("tagsWithInfo", name, e);

return [];

}

@@ -559,31 +625,35 @@ export const git = {

patchContent: string,

): Promise<{ clean: boolean; output: string }> {

const p = repoPath(name);

const tmpFile = `/tmp/hf-patch-${Date.now()}-${Math.random().toString(36).slice(2)}.patch`;

// Use a throwaway index (GIT_INDEX_FILE) so this read-only preview never

// mutates — nor races a concurrent applyPatch/editFile on — the repo's

// shared index. Without it, this GET-triggered check could reset the

// index mid-merge and silently drop the patch being written.

const tmpIndex = `/tmp/hf-index-${Date.now()}-${Math.random().toString(36).slice(2)}`;

const idxEnv = { ...gitEnv, GIT_INDEX_FILE: tmpIndex };

try {

await Bun.write(tmpFile, patchContent);

// Bare repos have no working tree; populate the index from HEAD so we can

// check against git objects (--cached) rather than the filesystem.

await $`git -C ${p} read-tree HEAD`.env(idxEnv).quiet();

const result =

await $`git -C ${p} apply --check --cached ${tmpFile}`

.env(idxEnv)

.quiet()

.nothrow();

return {

clean: result.exitCode === 0,

output: result.stderr.toString(),

};

return await withTempDir("patch", async (dir) => {

const tmpFile = path.join(dir, "change.patch");

// Use a throwaway index (GIT_INDEX_FILE) so this read-only

// preview never mutates — nor races a concurrent

// applyPatch/editFile on — the repo's shared index. Without it,

// this GET-triggered check could reset the index mid-merge and

// silently drop the patch being written.

const idxEnv = {

...gitEnv,

GIT_INDEX_FILE: path.join(dir, "index"),

};

await Bun.write(tmpFile, patchContent);

// Bare repos have no working tree; populate the index from HEAD

// so we can check against git objects (--cached) rather than the

// filesystem.

await $`git -C ${p} read-tree HEAD`.env(idxEnv).quiet();

const result =

await $`git -C ${p} apply --check --cached ${tmpFile}`

.env(idxEnv)

.quiet()

.nothrow();

return {

clean: result.exitCode === 0,

output: result.stderr.toString(),

};

});

} catch (e) {

return { clean: false, output: String(e) };

} finally {

await $`rm -f ${tmpFile} ${tmpIndex}`.quiet().nothrow();

}

@@ -597,8 +667,8 @@ export const git = {

): Promise<void> {

return withRepoLock(name, async () => {

const p = repoPath(name);

const tmpFile = `/tmp/hf-patch-${Date.now()}-${Math.random().toString(36).slice(2)}.patch`;

try {

await withTempDir("patch", async (dir) => {

const tmpFile = path.join(dir, "change.patch");

await Bun.write(tmpFile, patchContent);

// Populate index, apply to index, then create a real commit in the bare repo.

await $`git -C ${p} read-tree HEAD`;

@@ -629,9 +699,7 @@ export const git = {

await $`git -C ${p} symbolic-ref HEAD`.text()

).trim();

await $`git -C ${p} update-ref ${ref} ${commit}`;

} finally {

await $`rm -f ${tmpFile}`.quiet().nothrow();

}

});

@@ -650,8 +718,13 @@ export const git = {

newPath && newPath !== filePath ? newPath : filePath;

const isMove = targetPath !== filePath;

const p = repoPath(name);

const tmpFile = `/tmp/hf-edit-${Date.now()}-${Math.random().toString(36).slice(2)}`;

try {

// Preserve the file's existing mode (executable bit / symlink)

// rather than forcing every edit back to a plain 100644 file.

const mode =

(await treeFileMode(p, `refs/heads/${branch}`, filePath)) ??

"100644";

return await withTempDir("edit", async (dir) => {

const tmpFile = path.join(dir, "blob");

await Bun.write(tmpFile, content);

if (isMove) {

await $`git --work-tree=/tmp -C ${p} read-tree refs/heads/${branch}`;

@@ -664,7 +737,8 @@ export const git = {

if (isMove) {

await $`git --work-tree=/tmp -C ${p} update-index --remove ${filePath}`;

}

await $`git -C ${p} update-index --add --cacheinfo 100644,${blobHash},${targetPath}`;

const cacheInfo = `${mode},${blobHash},${targetPath}`;

await $`git -C ${p} update-index --add --cacheinfo ${cacheInfo}`;

const tree = isMove

? (

await $`git --work-tree=/tmp -C ${p} write-tree`.text()

@@ -692,9 +766,7 @@ export const git = {

).trim();

await $`git -C ${p} update-ref refs/heads/${branch} ${commit}`;

return commit;

} finally {

await $`rm -f ${tmpFile}`.quiet().nothrow();

}

});

@@ -709,8 +781,8 @@ export const git = {

): Promise<string> {

return withRepoLock(name, async () => {

const p = repoPath(name);

const tmpFile = `/tmp/hf-new-${Date.now()}-${Math.random().toString(36).slice(2)}`;

try {

return await withTempDir("new", async (dir) => {

const tmpFile = path.join(dir, "blob");

await Bun.write(tmpFile, content);

const parentSha = await git.resolveRef(

name,

@@ -750,9 +822,7 @@ export const git = {

).trim();

await $`git -C ${p} update-ref refs/heads/${branch} ${commit}`;

return commit;

} finally {

await $`rm -f ${tmpFile}`.quiet().nothrow();

}

});

@@ -809,10 +879,14 @@ export const git = {

): Promise<string> {

return withRepoLock(name, async () => {

const p = repoPath(name);

const tmpFile = `/tmp/hf-move-${Date.now()}-${Math.random().toString(36).slice(2)}`;

try {

// Preserve the moved file's mode (executable bit / symlink).

const mode =

(await treeFileMode(p, `refs/heads/${branch}`, oldPath)) ??

"100644";

return await withTempDir("move", async (dir) => {

const tmpFile = path.join(dir, "blob");

const contentBuf =

await $`git -C ${p} show ${`${branch}:${oldPath}`}`.arrayBuffer();

await $`git -C ${p} show --end-of-options ${`${branch}:${oldPath}`}`.arrayBuffer();

await Bun.write(tmpFile, contentBuf);

// --work-tree=/tmp is needed because bare repos have no work tree and

// `update-index --remove` requires one (even though it only touches the index).

@@ -821,7 +895,8 @@ export const git = {

await $`git -C ${p} hash-object -w ${tmpFile}`.text()

).trim();

await $`git --work-tree=/tmp -C ${p} update-index --remove ${oldPath}`;

await $`git -C ${p} update-index --add --cacheinfo 100644,${blobHash},${newPath}`;

const cacheInfo = `${mode},${blobHash},${newPath}`;

await $`git -C ${p} update-index --add --cacheinfo ${cacheInfo}`;

const tree = (

await $`git --work-tree=/tmp -C ${p} write-tree`.text()

).trim();

@@ -847,9 +922,7 @@ export const git = {

).trim();

await $`git -C ${p} update-ref refs/heads/${branch} ${commit}`;

return commit;

} finally {

await $`rm -f ${tmpFile}`.quiet().nothrow();

}

});

@@ -1049,7 +1122,8 @@ export const git = {

parents: (parts[7] ?? "").trim().split(/\s+/).filter(Boolean),

sigStatus: parseSigStatus(parts[8] ?? ""),

};

} catch {

} catch (e) {

logGitError(`commitMeta(${sha})`, name, e);

return null;

}