From abbbb87c991bed18804b133e375a1dbfbf8275ee Mon Sep 17 00:00:00 2001 From: Muzaffar Omer Date: Sun, 30 Aug 2026 19:46:00 +0200 Subject: [PATCH 1/8] fix: don't stage or discard unsafe paths --- index.ts | 10 +++ src/jj/stageDirectory.ts | 24 ++++++- src/patch/paths.ts | 54 ++++++++++++++ src/review/check.ts | 51 ++++++++++--- src/ui/messages.ts | 3 + test/hostile.test.ts | 150 +++++++++++++++++++++++++++++++++++++++ test/paths.test.ts | 40 +++++++++++ 7 files changed, 321 insertions(+), 11 deletions(-) create mode 100644 src/patch/paths.ts create mode 100644 test/hostile.test.ts create mode 100644 test/paths.test.ts diff --git a/index.ts b/index.ts index 2fb27e2..a19135e 100644 --- a/index.ts +++ b/index.ts @@ -520,6 +520,11 @@ async function report( return; } + if (outcome.kind === "unsafe-path") { + ctx.notify(messages.unsafePath(outcome.path, outcome.detail), "error"); + return; + } + if (outcome.kind === "nothing-staged") { ctx.notify(messages.nothingToStage, "warning"); return; @@ -597,6 +602,11 @@ function reportDiscard( return; } + if (outcome.kind === "unsafe-path") { + ctx.notify(messages.unsafePath(outcome.path, outcome.detail), "error"); + return; + } + if (outcome.kind === "unsupported") { ctx.notify(messages.cannotDiscard(outcome.path, outcome.detail), "error"); return; diff --git a/src/jj/stageDirectory.ts b/src/jj/stageDirectory.ts index 49643ee..3cdf99f 100644 --- a/src/jj/stageDirectory.ts +++ b/src/jj/stageDirectory.ts @@ -2,6 +2,7 @@ import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; import { tmpdir } from "node:os"; import type { StageOperation } from "./operations"; +import { unsafePathReason } from "../patch/paths"; import { CONTENT_DIRECTORY, DELETE_MANIFEST, @@ -20,8 +21,11 @@ export interface StageDirectory { } export class UnstageablePathError extends Error { - constructor(readonly path: string) { - super(`Cannot stage ${JSON.stringify(path)}: paths containing newlines are not supported.`); + constructor( + readonly path: string, + reason: string, + ) { + super(`Cannot stage ${JSON.stringify(path)}: ${reason}.`); this.name = "UnstageablePathError"; } } @@ -43,7 +47,21 @@ export async function createStageDirectory( ): Promise { for (const operation of operations) { if (!isExpressiblePath(operation.path)) { - throw new UnstageablePathError(operation.path); + throw new UnstageablePathError( + operation.path, + "paths containing newlines are not supported", + ); + } + + // `checkReviewedFile` has already refused a path that leaves the + // workspace, so this can only fire on a bug. It is here anyway because of + // what sits downstream: the helper script runs `rm -f` and a redirect + // against whatever these manifests name, and its quoting — which is + // correct — stops word-splitting without saying anything about `..`. The + // boundary in front of a shell should be able to refuse on its own. + const unsafe = unsafePathReason(operation.path); + if (unsafe !== null) { + throw new UnstageablePathError(operation.path, unsafe); } } diff --git a/src/patch/paths.ts b/src/patch/paths.ts new file mode 100644 index 0000000..a3910d0 --- /dev/null +++ b/src/patch/paths.ts @@ -0,0 +1,54 @@ +/** + * Whether a path recovered from a patch may be acted on. + * + * Every path this extension reads, writes, or deletes is parsed out of patch + * text, and the places it lands — `join(workspace.root, path)` when + * discarding, and the staging directory jj's diff editor reads back — all + * normalise `..` rather than confining it. `join("/repo", "../../etc/x")` is + * `/etc/x`, not an error. So containment has to be established before the + * join, never by it. + * + * Refusing costs nothing, because no working copy can describe what this + * rejects: git and jj both emit repo-root-relative paths from any directory, + * and git's one cwd-relative mode (`--relative`) omits files outside the cwd + * rather than reaching them with `..`. + */ + +/** + * Read a path the way both POSIX and Windows would. + * + * Patches carry `/`, but a patch can be authored anywhere, and this runs on + * either platform — so a backslash is treated as a separator rather than + * assumed to be part of a filename. That is stricter than POSIX, where a + * backslash is a legal character, and deliberately so: the paths it costs are + * ones no diff of a working copy produces. + */ +function segments(path: string): readonly string[] { + return path.split(/[/\\]/); +} + +/** + * Why this path must not be touched, or null when it is safe. + * + * A reason rather than a boolean, so the refusal can say which rule the path + * broke — the reviewer sees a hostile-looking path and deserves to know what + * was wrong with it. + */ +export function unsafePathReason(path: string): string | null { + if (path === "") { + return "the patch does not name a file"; + } + + // Checked by hand rather than with `isAbsolute`, which answers for the + // platform this happens to run on: `C:\x` is not absolute to a Mac, and a + // patch does not stop being a Windows patch when it is read on one. + if (path.startsWith("/") || path.startsWith("\\") || /^[A-Za-z]:/.test(path)) { + return "it is an absolute path"; + } + + if (segments(path).includes("..")) { + return "it points outside the workspace with a `..` segment"; + } + + return null; +} diff --git a/src/review/check.ts b/src/review/check.ts index 382daa9..f38c400 100644 --- a/src/review/check.ts +++ b/src/review/check.ts @@ -1,6 +1,7 @@ import { parseDocument, type Document } from "../patch/document"; import { findDisagreement, type HostHunk } from "../patch/agreement"; import { parseFilePatch, type FilePatch } from "../patch/parse"; +import { unsafePathReason } from "../patch/paths"; import { findStaleHunk } from "../patch/select"; import { disposeFile, requiresWorkingCopyCheck, type FileDisposition, type FileMark } from "../staging/plan"; @@ -15,13 +16,17 @@ export interface ReviewedFile { /** * Why an operation stopped without touching anything. * - * Both refusals mean the same thing to a reviewer — what you are looking at is - * not what is on disk — but they are found in different ways, so they are - * reported separately. + * `stale` and `disagreement` both mean the same thing to a reviewer — what you + * are looking at is not what is on disk — but they are found in different + * ways, so they are reported separately. `unsafe-path` is a different kind of + * answer: not "this is out of date" but "this patch names a file no diff of a + * working copy could name", which is a bug here or a patch from somewhere it + * should not have come from. */ export type ReviewRefusal = | { readonly kind: "stale"; readonly path: string; readonly detail: string } - | { readonly kind: "disagreement"; readonly path: string; readonly detail: string }; + | { readonly kind: "disagreement"; readonly path: string; readonly detail: string } + | { readonly kind: "unsafe-path"; readonly path: string; readonly detail: string }; /** One reviewed file, once every check has passed and its fate is known. */ export interface CheckedFile { @@ -34,11 +39,30 @@ export interface CheckedFile { /** * Check one file and decide what the marks say about it. * - * Shared by staging and discarding, which ask the same two questions before - * acting — does this extension read the patch the way Hunk does, and does the - * patch still describe what is on disk — and differ only in what they do with - * the answer. + * Shared by staging and discarding, which ask the same questions before + * acting — is this a path we may touch at all, does this extension read the + * patch the way Hunk does, and does the patch still describe what is on disk — + * and differ only in what they do with the answer. */ +/** + * The first path in a patch that must not be acted on. + * + * A rename carries two, and both are used: the new path is written, and the + * old one is restored from `$left` or rewritten when the rename is undone. + */ +function findUnsafePath(patch: FilePatch): ReviewRefusal | null { + const paths = patch.previousPath === undefined ? [patch.path] : [patch.path, patch.previousPath]; + + for (const path of paths) { + const reason = unsafePathReason(path); + if (reason !== null) { + return { kind: "unsafe-path", path, detail: reason }; + } + } + + return null; +} + export async function checkReviewedFile( file: ReviewedFile, mark: FileMark | undefined, @@ -46,6 +70,17 @@ export async function checkReviewedFile( ): Promise { const patch = parseFilePatch(file.patchText); + // Before anything else, because everything else acts on these paths: this + // function reads `patch.path` below, discarding writes and deletes at it, + // and the jj staging directory is built from it. Checking here rather than + // at those three sites means none of them can be reached with a path that + // was never checked — including for files nobody marked, which still become + // `delete` and `restore` instructions in a jj revision. + const unsafe = findUnsafePath(patch); + if (unsafe) { + return unsafe; + } + const disagreement = findDisagreement(patch, file.hostHunks); if (disagreement) { return { kind: "disagreement", path: file.path, detail: disagreement }; diff --git a/src/ui/messages.ts b/src/ui/messages.ts index fe7a45d..9dca744 100644 --- a/src/ui/messages.ts +++ b/src/ui/messages.ts @@ -101,6 +101,9 @@ export const messages = { disagreement: (path: string, detail: string) => `Refusing to stage ${path}: ${detail}. This is a bug — please report it.`, + unsafePath: (path: string, detail: string) => + `Refusing to touch ${JSON.stringify(path)}: ${detail}. Nothing was changed — no ordinary diff names a file this way, so this is a bug or a patch worth being suspicious of. Please report it.`, + /** Whatever the backend reported, whichever backend it was. */ failed: (detail: string) => `Could not finish: ${detail}`, diff --git a/test/hostile.test.ts b/test/hostile.test.ts new file mode 100644 index 0000000..4bef8d1 --- /dev/null +++ b/test/hostile.test.ts @@ -0,0 +1,150 @@ +import { describe, expect, test } from "bun:test"; +import { discardMarkedHunks } from "../src/discard/discard"; +import { createStageDirectory } from "../src/jj/stageDirectory"; +import type { FileMark } from "../src/staging/plan"; +import { stageMarkedHunks } from "../src/staging/stage"; +import { reviewFromPatch } from "./support/review"; + +/** + * Patches that name a file no diff of a working copy could name. + * + * git will not emit these for a tracked file, which is exactly why they are + * worth testing: nothing else in the suite would notice if the checks that + * refuse them were removed, and the paths reach `writeFile` and `rm` directly. + */ + +/** A patch whose only file sits at `path`. */ +function patchNaming(path: string): string { + return [ + `diff --git a/${path} b/${path}`, + `--- a/${path}`, + `+++ b/${path}`, + "@@ -1,1 +1,1 @@", + "-before", + "+after", + "", + ].join("\n"); +} + +/** A patch that renames `from` to a perfectly ordinary path. */ +function patchRenamingFrom(from: string): string { + return [ + "diff --git a/safe.txt b/safe.txt", + "similarity index 90%", + `rename from ${from}`, + "rename to safe.txt", + `--- a/${from}`, + "+++ b/safe.txt", + "@@ -1,1 +1,1 @@", + "-before", + "+after", + "", + ].join("\n"); +} + +const ESCAPING = "../../outside.txt"; +const WHOLE: FileMark = { kind: "whole" }; + +/** Stage one patch, recording anything the operation tried to touch. */ +async function stage(patchText: string) { + const touched: string[] = []; + const files = reviewFromPatch(patchText); + + const outcome = await stageMarkedHunks( + { files, marks: new Map(files.map((file) => [file.id, WHOLE])) }, + { + backend: { + destination: "the index", + stage: async () => { + touched.push("staged"); + }, + }, + readWorkingCopyFile: async (path) => { + touched.push(`read ${path}`); + return "after\n"; + }, + }, + ); + + return { outcome, touched }; +} + +/** Discard one patch, recording anything the operation tried to touch. */ +async function discard(patchText: string) { + const touched: string[] = []; + const files = reviewFromPatch(patchText); + + const outcome = await discardMarkedHunks( + { files, marks: new Map(files.map((file) => [file.id, WHOLE])) }, + { + readWorkingCopyFile: async (path) => { + touched.push(`read ${path}`); + return "after\n"; + }, + writeWorkingCopyFile: async (path) => { + touched.push(`write ${path}`); + }, + removeWorkingCopyFile: async (path) => { + touched.push(`remove ${path}`); + }, + }, + ); + + return { outcome, touched }; +} + +describe("a patch naming a path outside the workspace", () => { + test.each([ + ["a `..` path", patchNaming(ESCAPING)], + ["an absolute path", patchNaming("/etc/cron.d/x")], + ["a rename away from a `..` path", patchRenamingFrom(ESCAPING)], + ])("is refused before staging touches anything: %s", async (_name, patchText) => { + const { outcome, touched } = await stage(patchText); + + expect(outcome.kind).toBe("unsafe-path"); + expect(touched).toEqual([]); + }); + + test.each([ + ["a `..` path", patchNaming(ESCAPING)], + ["an absolute path", patchNaming("/etc/cron.d/x")], + ["a rename away from a `..` path", patchRenamingFrom(ESCAPING)], + ])("is refused before discarding touches anything: %s", async (_name, patchText) => { + const { outcome, touched } = await discard(patchText); + + expect(outcome.kind).toBe("unsafe-path"); + expect(touched).toEqual([]); + }); + + test("names the offending path, not the file it was disguised as", async () => { + const { outcome } = await discard(patchRenamingFrom(ESCAPING)); + + expect(outcome).toMatchObject({ kind: "unsafe-path", path: ESCAPING }); + }); + + test("is refused a second time at the boundary in front of the shell", async () => { + // The chokepoint above already refuses this, so reaching the staging + // directory with such a path takes a bug. It still must not be written. + const attempt = createStageDirectory([ + { kind: "write", path: ESCAPING, content: "after\n" }, + ]); + + await expect(attempt).rejects.toThrow(/outside the workspace/); + }); +}); + +describe("an ordinary patch", () => { + test("still stages", async () => { + const { outcome, touched } = await stage(patchNaming("src/f.txt")); + + expect(outcome.kind).toBe("staged"); + expect(touched).toContain("staged"); + }); + + test("still discards", async () => { + const { outcome, touched } = await discard(patchNaming("src/f.txt")); + + expect(outcome.kind).toBe("discarded"); + expect(touched).toContain("write src/f.txt"); + }); +}); diff --git a/test/paths.test.ts b/test/paths.test.ts new file mode 100644 index 0000000..84a5139 --- /dev/null +++ b/test/paths.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, test } from "bun:test"; +import { unsafePathReason } from "../src/patch/paths"; + +describe("unsafePathReason", () => { + test.each([ + "src/patch/paths.ts", + "f.txt", + "a/b/c/deep.ts", + // `..` only counts as a whole segment; these are ordinary filenames. + "src/..hidden", + "src/weird..name.ts", + "...", + ])("allows %j", (path) => { + expect(unsafePathReason(path)).toBeNull(); + }); + + test.each([ + ["../outside.ts", "`..` segment"], + ["a/../../outside.ts", "`..` segment"], + ["src/../../../etc/cron.d/x", "`..` segment"], + ["a/..", "`..` segment"], + ["..", "`..` segment"], + // A backslash is a separator here even on POSIX, so a Windows-shaped + // escape cannot slip past the split. + ["a\\..\\..\\outside.ts", "`..` segment"], + ])("rejects %j as escaping", (path) => { + expect(unsafePathReason(path)).toContain("outside the workspace"); + }); + + test.each(["/etc/passwd", "\\\\server\\share\\x", "C:\\Windows\\x", "c:/Windows/x"])( + "rejects %j as absolute", + (path) => { + expect(unsafePathReason(path)).toContain("absolute"); + }, + ); + + test("rejects the empty path", () => { + expect(unsafePathReason("")).toContain("does not name a file"); + }); +}); From 405b23ca56f513535c628b7fc01f86f0e64b4be9 Mon Sep 17 00:00:00 2001 From: Muzaffar Mohammed Date: Sun, 30 Aug 2026 19:54:17 +0200 Subject: [PATCH 2/8] bd init: initialize beads issue tracking --- .agents/skills/beads/SKILL.md | 80 +++++++++++++++++++++++++++++++++ .beads/.gitignore | 83 +++++++++++++++++++++++++++++++++++ .beads/README.md | 81 ++++++++++++++++++++++++++++++++++ .beads/config.yaml | 70 +++++++++++++++++++++++++++++ .beads/metadata.json | 9 ++++ .gitignore | 6 +++ CLAUDE.md | 77 ++++++++++++++++++++++++++++++++ 7 files changed, 406 insertions(+) create mode 100644 .agents/skills/beads/SKILL.md create mode 100644 .beads/.gitignore create mode 100644 .beads/README.md create mode 100644 .beads/config.yaml create mode 100644 .beads/metadata.json create mode 100644 CLAUDE.md diff --git a/.agents/skills/beads/SKILL.md b/.agents/skills/beads/SKILL.md new file mode 100644 index 0000000..a5a3344 --- /dev/null +++ b/.agents/skills/beads/SKILL.md @@ -0,0 +1,80 @@ +--- +name: beads +description: Use when working in a repository that uses bd or Beads for durable project task tracking, issue dependencies, blocker management, multi-session handoff, or shared work memory. Trigger when the user asks to find ready work, claim or close tasks, create follow-up work, inspect blockers, recover project context, or choose between local planning and persistent project tracking. +--- + +# Beads + +Use Beads as the shared project task system. Local plans, scratch files, and personal memories are useful, but they are not the durable source of truth for project work. + +## First Step + +Run: + +```bash +bd prime +``` + +If that prints nothing, check whether the repository has an active Beads workspace: + +```bash +bd where +``` + +## Preferred Route + +Use the `bd` CLI when shell access is available. It is the most compact and direct Beads interface. + +## Core CLI Workflow + +1. Find work: + +```bash +bd ready +bd list --status=open +bd list --status=in_progress +``` + +2. Inspect before editing: + +```bash +bd show +``` + +3. Claim work atomically: + +```bash +bd update --claim +``` + +4. Create durable follow-up work when implementation reveals new tasks: + +```bash +bd create "Short title" --description="Why this exists and what needs to be done" --type=task --priority=2 +``` + +5. Close completed work: + +```bash +bd close --reason="Completed" +``` + +## What Belongs In Beads + +Use Beads for: + +- shared project tasks +- blockers and dependencies +- discovered follow-up work +- work that must survive thread reset, compaction, or handoff +- status that another person or agent should be able to resume + +Use agent-local planning tools only for the current turn's execution checklist. Do not treat them as shared project state. + +## Rules + +- Do not create markdown TODO files as the source of truth when Beads is available. +- Do not use `bd edit`; it opens an interactive editor. Use `bd update` flags instead. +- Prefer `--json` when parsing `bd` output programmatically. +- If hooks are installed, `bd prime` may already be injected. Run it manually when context is missing. +- Do not auto-close or mutate tasks unless the work is actually complete. diff --git a/.beads/.gitignore b/.beads/.gitignore new file mode 100644 index 0000000..e6af2b9 --- /dev/null +++ b/.beads/.gitignore @@ -0,0 +1,83 @@ +# Dolt database (managed by Dolt, not git) +dolt/ +embeddeddolt/ +proxieddb/ + +# Runtime files +bd.sock +bd.sock.startlock +sync-state.json +last-touched +.exclusive-lock + +# Daemon runtime (lock, log, pid) +daemon.* + +# Push state (runtime, per-machine) +push-state.json + +# Lock files (various runtime locks) +*.lock + +# Credential key (encryption key for federation peer auth — never commit) +.beads-credential-key + +# Local version tracking (prevents upgrade notification spam after git ops) +.local_version + +proxied_server_client_info.json + +# Worktree redirect file (contains relative path to main repo's .beads/) +# Must not be committed as paths would be wrong in other clones +redirect + +# Sync state (local-only, per-machine) +# These files are machine-specific and should not be shared across clones +.sync.lock +export-state/ +export-state.json +last_pull + +# Ephemeral store (SQLite - wisps/molecules, intentionally not versioned) +ephemeral.sqlite3 +ephemeral.sqlite3-journal +ephemeral.sqlite3-wal +ephemeral.sqlite3-shm + +# Dolt server management (auto-started by bd) +dolt-server.pid +dolt-server.log +dolt-server.lock +dolt-server.port +dolt-server.activity + +# Debug-mode pprof artifacts (written when dolt.debug: true in config.yaml) +dolt-pprof/ + +# Corrupt backup directories (created by bd doctor --fix recovery) +*.corrupt.backup/ + +# Backup data (auto-exported JSONL, local-only) +backup/ + +# Per-project environment file (Dolt connection config, GH#2520) +.env + +# Legacy files (from pre-Dolt versions) +*.db +*.db?* +*.db-journal +*.db-wal +*.db-shm +db.sqlite +bd.db +# NOTE: Do NOT add negation patterns here. +# They would override fork protection in .git/info/exclude. +# Config files (metadata.json, config.yaml) are tracked by git by default +# since no pattern above ignores them. +# Shared project state — created by `bd init`, same for every clone. +!.gitignore +!README.md +!config.yaml +!metadata.json +!interactions.jsonl diff --git a/.beads/README.md b/.beads/README.md new file mode 100644 index 0000000..63e8f4c --- /dev/null +++ b/.beads/README.md @@ -0,0 +1,81 @@ +# Beads - AI-Native Issue Tracking + +Welcome to Beads! This repository uses **Beads** for issue tracking - a modern, AI-native tool designed to live directly in your codebase alongside your code. + +## What is Beads? + +Beads is issue tracking that lives in your repo, making it perfect for AI coding agents and developers who want their issues close to their code. No web UI required - everything works through the CLI and integrates seamlessly with git. + +**Learn more:** [github.com/steveyegge/beads](https://github.com/steveyegge/beads) + +## Quick Start + +### Essential Commands + +```bash +# Create new issues +bd create "Add user authentication" + +# View all issues +bd list + +# View issue details +bd show + +# Update issue status +bd update --claim +bd update --status done + +# Sync with Dolt remote +bd dolt push +``` + +### Working with Issues + +Issues in Beads are: +- **Git-native**: Stored in Dolt database with version control and branching +- **AI-friendly**: CLI-first design works perfectly with AI coding agents +- **Branch-aware**: Issues can follow your branch workflow +- **Sync-ready**: Uses Dolt remotes for backup and team sharing + +## Why Beads? + +✨ **AI-Native Design** +- Built specifically for AI-assisted development workflows +- CLI-first interface works seamlessly with AI coding agents +- No context switching to web UIs + +🚀 **Developer Focused** +- Issues live in your repo, right next to your code +- Works offline, syncs when you push +- Fast, lightweight, and stays out of your way + +🔧 **Git Integration** +- Dolt-native sync via bd dolt push / bd dolt pull +- Branch-aware issue tracking +- Dolt-native three-way merge resolution + +## Get Started with Beads + +Try Beads in your own projects: + +```bash +# Install Beads +curl -sSL https://raw.githubusercontent.com/steveyegge/beads/main/scripts/install.sh | bash + +# Initialize in your repo +bd init + +# Create your first issue +bd create "Try out Beads" +``` + +## Learn More + +- **Documentation**: [github.com/steveyegge/beads/docs](https://github.com/steveyegge/beads/tree/main/docs) +- **Quick Start Guide**: Run `bd quickstart` +- **Examples**: [github.com/steveyegge/beads/examples](https://github.com/steveyegge/beads/tree/main/examples) + +--- + +*Beads: Issue tracking that moves at the speed of thought* ⚡ diff --git a/.beads/config.yaml b/.beads/config.yaml new file mode 100644 index 0000000..047c909 --- /dev/null +++ b/.beads/config.yaml @@ -0,0 +1,70 @@ +# Beads Configuration File +# This file configures default behavior for all bd commands in this repository +# All settings can also be set via environment variables (BD_* prefix) +# or overridden with command-line flags + +# Issue prefix for this repository (used by bd init) +# If not set, bd init will auto-detect from directory name +# Example: issue-prefix: "myproject" creates issues like "myproject-1", "myproject-2", etc. +# issue-prefix: "" + +# Use no-db mode: JSONL-only, no Dolt database +# When true, .beads/issues.jsonl is the only local store +# no-db: false + +# Enable JSON output by default +# json: false + +# Feedback title formatting for mutating commands (create/update/close/dep/edit) +# 0 = hide titles, N > 0 = truncate to N characters +# output: +# title-length: 255 + +# Default actor for audit trails (overridden by BEADS_ACTOR or --actor) +# actor: "" + +# Export events (audit trail) to .beads/events.jsonl on each flush/sync +# When enabled, new events are appended incrementally using a high-water mark. +# Use 'bd export --events' to trigger manually regardless of this setting. +# events-export: false + +# Multi-repo configuration (experimental - bd-307) +# Allows hydrating from multiple repositories and routing writes to the correct database +# repos: +# primary: "." # Primary repo (where this database lives) +# additional: # Additional repos to hydrate from (read-only) +# - ~/beads-planning # Personal planning repo +# - ~/work-planning # Work planning repo + +# Dolt-native backup (periodic backup for off-machine recovery) +# This is full database backup only. Cross-machine sync uses Dolt remotes. +# backup: +# enabled: false # Disable auto-backup entirely +# interval: 15m # Minimum time between auto-backups +# git-push: false # Disable git push (backup locally only) +# git-repo: "" # Separate git repo for backups (default: project repo) + +# Optional JSONL auto-export for viewers, interchange, and issue-level migration. +# Disabled by default; enable only when an integration needs fresh .beads/issues.jsonl. +# Use relative paths under .beads/ for JSONL import/export filenames. +# export: +# auto: false +# path: issues.jsonl +# interval: 60s +# git-add: false +# import: +# path: issues.jsonl + +# Integration settings (access with 'bd config get/set') +# Non-secret keys (stored in the database): +# - jira.url, jira.project +# - linear.team_id +# - github.org, github.repo +# +# Secret keys (stored in this file but prefer env vars to avoid git exposure): +# - linear.api_key → use LINEAR_API_KEY env var instead +# - github.token → use GITHUB_TOKEN env var instead + +sync.remote: "muzomer/hunk-commit" +dolt: + shared-server: true \ No newline at end of file diff --git a/.beads/metadata.json b/.beads/metadata.json new file mode 100644 index 0000000..18a0a1e --- /dev/null +++ b/.beads/metadata.json @@ -0,0 +1,9 @@ +{ + "database": "dolt", + "backend": "dolt", + "dolt_mode": "server", + "dolt_database": "hunk_commit", + "project_id": "73b410ae-8d9f-44ea-86d3-056356c2676e", + "global_dolt_database": "beads_global", + "global_project_id": "00000000-0000-0000-0000-000000000000" +} \ No newline at end of file diff --git a/.gitignore b/.gitignore index c2658d7..3350359 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,7 @@ node_modules/ + +# Beads / Dolt files (added by bd init) +.dolt/ +*.db +.beads-credential-key +.beads/proxieddb/ diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..db14bd1 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,77 @@ +# Project Instructions for AI Agents + +This file provides instructions and context for AI coding agents working on this project. + + +## Beads Issue Tracker + +This project uses **bd (beads)** for issue tracking. Run `bd prime` to see full workflow context and commands. + +### Quick Reference + +```bash +bd ready # Find available work +bd show # View issue details +bd update --claim # Claim work +bd close # Complete work +``` + +### Rules + +- Use `bd` for ALL task tracking — do NOT use TodoWrite, TaskCreate, or markdown TODO lists +- Run `bd prime` for detailed command reference and session close protocol +- Use `bd remember` for persistent knowledge — do NOT use MEMORY.md files + +**Architecture in one line:** issues live in a local Dolt DB; sync uses `refs/dolt/data` on your git remote; `.beads/issues.jsonl` is a passive export. See https://github.com/gastownhall/beads/blob/main/docs/SYNC_CONCEPTS.md for details and anti-patterns. + +## Agent Context Profiles + +The managed Beads block is task-tracking guidance, not permission to override repository, user, or orchestrator instructions. + +- **Conservative (default)**: Use `bd` for task tracking. Do not run git commits, git pushes, or Dolt remote sync unless explicitly asked. At handoff, report changed files, validation, and suggested next commands. +- **Minimal**: Keep tool instruction files as pointers to `bd prime`; use the same conservative git policy unless active instructions say otherwise. +- **Team-maintainer**: Only when the repository explicitly opts in, agents may close beads, run quality gates, commit, and push as part of session close. A current "do not commit" or "do not push" instruction still wins. + +## Session Completion + +This protocol applies when ending a Beads implementation workflow. It is subordinate to explicit user, repository, and orchestrator instructions. + +1. **File issues for remaining work** - Create beads for anything that needs follow-up +2. **Run quality gates** (if code changed) - Tests, linters, builds +3. **Update issue status** - Close finished work, update in-progress items +4. **Handle git/sync by active profile**: + ```bash + # Conservative/minimal/default: report status and proposed commands; wait for approval. + git status + + # Team-maintainer opt-in only, unless current instructions forbid it: + git pull --rebase + git push + git status + ``` +5. **Hand off** - Summarize changes, validation, issue status, and any blocked sync/commit/push step + +**Critical rules:** +- Explicit user or orchestrator instructions override this Beads block. +- Do not commit or push without clear authority from the active profile or the current user request. +- If a required sync or push is blocked, stop and report the exact command and error. + + + +## Build & Test + +_Add your build and test commands here_ + +```bash +# Example: +# npm install +# npm test +``` + +## Architecture Overview + +_Add a brief overview of your project architecture_ + +## Conventions & Patterns + +_Add your project-specific conventions here_ From 277c27d4fee7b483ab56222689f27964eb83cd60 Mon Sep 17 00:00:00 2001 From: Muzaffar Omer Date: Sun, 30 Aug 2026 20:28:35 +0200 Subject: [PATCH 3/8] chore: ignore beads issues file as issues are tracked in dolthub --- .beads/.gitignore | 7 ++----- .beads/issues.jsonl | 5 +++++ 2 files changed, 7 insertions(+), 5 deletions(-) create mode 100644 .beads/issues.jsonl diff --git a/.beads/.gitignore b/.beads/.gitignore index e6af2b9..e50ad2e 100644 --- a/.beads/.gitignore +++ b/.beads/.gitignore @@ -71,11 +71,8 @@ backup/ *.db-shm db.sqlite bd.db -# NOTE: Do NOT add negation patterns here. -# They would override fork protection in .git/info/exclude. -# Config files (metadata.json, config.yaml) are tracked by git by default -# since no pattern above ignores them. -# Shared project state — created by `bd init`, same for every clone. +issues.jsonl + !.gitignore !README.md !config.yaml diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl new file mode 100644 index 0000000..74d793f --- /dev/null +++ b/.beads/issues.jsonl @@ -0,0 +1,5 @@ +{"_type":"issue","id":"hunk-commit-6af","title":"Discarding writes through a symlink, escaping the workspace","description":"`writeWorkingCopyFile` in index.ts (the discard environment, ~line 573) calls\n`writeFile`, which follows a symlink at the destination. The path containment\ncheck added for concern-8 does not cover this: the path is entirely legitimate,\nonly the destination's type is not.\n\nTrigger: a repository that tracks a symlink — git stores them as ordinary files\n— pointing outside the repo, e.g. `link -\u003e ~/.ssh/authorized_keys`, plus an edit\nto `link` in the review. Discarding that hunk writes *through* the link, outside\nthe workspace, with content the patch controls.\n\nThe same applies to the jj helper script (src/jj/script.ts): the `cat \"$stage/\nfiles/$path\" \u003e \"$right/$path\"` redirect follows a symlink in `$right` the same\nway.\n\n`removeWorkingCopyFile` is fine as it stands — `rm` does not follow symlinks.\n","status":"open","priority":1,"issue_type":"bug","owner":"muzaffar.mohammed@pix4d.com","created_at":"2026-08-30T18:24:03Z","created_by":"Muzaffar Mohammed","updated_at":"2026-08-30T18:24:03Z","labels":["discard","paths","security"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"hunk-commit-ys8","title":"Git-quoted paths are never decoded, so the wrong file is read and written","description":"`stripPathPrefix` in src/patch/parse.ts:68-74 does not unquote. git quotes any\npath containing non-ASCII or special characters, writing\n\n +++ \"b/caf\\303\\251\"\n\nrather than `+++ b/café`. The leading `\"` makes both `startsWith(\"a/\")` and\n`startsWith(\"b/\")` fail, so the prefix is not even stripped: the quotes and the\nC-style escapes survive into `patch.path` and are used as a literal filename.\nStaging or discarding then reads and writes a file that does not exist under\nthat name.\n\nA repository containing one non-ASCII filename is enough to trigger this, which\nmakes it the likeliest of the path findings to be hit by accident rather than by\na hostile patch.\n\nFix: decode before the prefix strip — detect the surrounding quotes, unescape\n`\\\\`, `\\\"`, and octal sequences. Related to concern-7 in the doubt session,\nwhich is the other half of path recovery failing quietly instead of loudly.\n","status":"open","priority":1,"issue_type":"bug","owner":"muzaffar.mohammed@pix4d.com","created_at":"2026-08-30T18:24:03Z","created_by":"Muzaffar Mohammed","updated_at":"2026-08-30T18:24:03Z","labels":["parser","paths"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"hunk-commit-ats","title":"Harden the CI workflow: token permissions, action pinning, jj checksum","description":"Three supply-chain gaps in .github/workflows/ci.yml:\n\n1. No `permissions:` block, so `GITHUB_TOKEN` gets the repository default rather\n than the read access this job needs — and it is handed to a step explicitly\n as `GH_TOKEN` (line 20).\n2. Both actions are pinned to floating tags (`actions/checkout@v4`,\n `oven-sh/setup-bun@v2`), so a compromised or retagged release runs with that\n token.\n3. The jj release tarball is downloaded and installed with no checksum\n verification, so CI trusts whatever that asset holds at download time.\n\nFix: `permissions: contents: read` at workflow level, both actions pinned to\ncommit SHAs, and the jj download verified against the digest published with the\nrelease.\n","status":"open","priority":2,"issue_type":"chore","owner":"muzaffar.mohammed@pix4d.com","created_at":"2026-08-30T18:24:21Z","created_by":"Muzaffar Mohammed","updated_at":"2026-08-30T18:24:21Z","labels":["ci","security","supply-chain"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"hunk-commit-453","title":"Cross-check the parsed path against the path Hunk reports","description":"`findDisagreement` in src/patch/agreement.ts:18-21 compares hunk counts and line\nranges but never the path. `ReviewedFile` carries two: `file.path`, which Hunk\nresolved, and `patch.path`, which this extension parsed out of the same text —\nand only the second one ever reaches the filesystem, never compared against the\nfirst.\n\nThis module exists precisely to refuse when the two parses disagree, so a path\nmismatch belongs in it: same class of bug as a hunk-count mismatch, and it costs\none comparison.\n\nIt does not replace the containment check added for concern-8 — both sides can\nagree on a path that escapes the workspace — but it catches a parser that read\nthe wrong filename, which containment cannot. Likely to surface the quoted-path\nbug (hunk-commit-ys8) as a refusal rather than a wrong write.\n","status":"open","priority":2,"issue_type":"task","owner":"muzaffar.mohammed@pix4d.com","created_at":"2026-08-30T18:24:20Z","created_by":"Muzaffar Mohammed","updated_at":"2026-08-30T18:24:20Z","labels":["hardening","paths"],"dependencies":[{"issue_id":"hunk-commit-453","depends_on_id":"hunk-commit-ys8","type":"related","created_at":"2026-08-30T20:24:30Z","created_by":"Muzaffar Mohammed","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"hunk-commit-rd7","title":"Check whether diff.relative breaks path resolution","description":"Found while verifying that the concern-8 containment check would not reject\npaths when Hunk is launched from a subdirectory. It would not: git and jj both\nemit repo-root-relative paths from any directory, and `--relative` omits files\noutside the cwd rather than reaching them with `..`.\n\nBut that mode raises a separate question. With `diff.relative=true` in git\nconfig, paths become relative to the cwd (`a/foo.ts` for `src/deep/foo.ts` when\nrun from `src/deep`). `workspace.root` is always the repo root — `detectWorkspace`\nwalks upward — so `join(workspace.root, path)` would then resolve the wrong\nfile.\n\nUnverified: whether Hunk passes that config through, or normalises paths before\nhanding them to an extension. If it does pass through, the failure is loud\n(ENOENT into the catch in index.ts) rather than silent, so this is a correctness\nquestion, not a safety one.\n\nWork: confirm what Hunk does with `diff.relative=true`, and fix or document.\n","status":"open","priority":3,"issue_type":"task","owner":"muzaffar.mohammed@pix4d.com","created_at":"2026-08-30T18:24:21Z","created_by":"Muzaffar Mohammed","updated_at":"2026-08-30T18:24:21Z","labels":["investigate","paths"],"dependency_count":0,"dependent_count":0,"comment_count":0} From 2e21c0f947cede8bdf09f34021326a7f25ab8895 Mon Sep 17 00:00:00 2001 From: Muzaffar Omer Date: Sun, 30 Aug 2026 20:35:52 +0200 Subject: [PATCH 4/8] feat: add perles config and remove issues files --- .beads/issues.jsonl | 5 -- .perles/config.yaml | 174 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 174 insertions(+), 5 deletions(-) delete mode 100644 .beads/issues.jsonl create mode 100644 .perles/config.yaml diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl deleted file mode 100644 index 74d793f..0000000 --- a/.beads/issues.jsonl +++ /dev/null @@ -1,5 +0,0 @@ -{"_type":"issue","id":"hunk-commit-6af","title":"Discarding writes through a symlink, escaping the workspace","description":"`writeWorkingCopyFile` in index.ts (the discard environment, ~line 573) calls\n`writeFile`, which follows a symlink at the destination. The path containment\ncheck added for concern-8 does not cover this: the path is entirely legitimate,\nonly the destination's type is not.\n\nTrigger: a repository that tracks a symlink — git stores them as ordinary files\n— pointing outside the repo, e.g. `link -\u003e ~/.ssh/authorized_keys`, plus an edit\nto `link` in the review. Discarding that hunk writes *through* the link, outside\nthe workspace, with content the patch controls.\n\nThe same applies to the jj helper script (src/jj/script.ts): the `cat \"$stage/\nfiles/$path\" \u003e \"$right/$path\"` redirect follows a symlink in `$right` the same\nway.\n\n`removeWorkingCopyFile` is fine as it stands — `rm` does not follow symlinks.\n","status":"open","priority":1,"issue_type":"bug","owner":"muzaffar.mohammed@pix4d.com","created_at":"2026-08-30T18:24:03Z","created_by":"Muzaffar Mohammed","updated_at":"2026-08-30T18:24:03Z","labels":["discard","paths","security"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"hunk-commit-ys8","title":"Git-quoted paths are never decoded, so the wrong file is read and written","description":"`stripPathPrefix` in src/patch/parse.ts:68-74 does not unquote. git quotes any\npath containing non-ASCII or special characters, writing\n\n +++ \"b/caf\\303\\251\"\n\nrather than `+++ b/café`. The leading `\"` makes both `startsWith(\"a/\")` and\n`startsWith(\"b/\")` fail, so the prefix is not even stripped: the quotes and the\nC-style escapes survive into `patch.path` and are used as a literal filename.\nStaging or discarding then reads and writes a file that does not exist under\nthat name.\n\nA repository containing one non-ASCII filename is enough to trigger this, which\nmakes it the likeliest of the path findings to be hit by accident rather than by\na hostile patch.\n\nFix: decode before the prefix strip — detect the surrounding quotes, unescape\n`\\\\`, `\\\"`, and octal sequences. Related to concern-7 in the doubt session,\nwhich is the other half of path recovery failing quietly instead of loudly.\n","status":"open","priority":1,"issue_type":"bug","owner":"muzaffar.mohammed@pix4d.com","created_at":"2026-08-30T18:24:03Z","created_by":"Muzaffar Mohammed","updated_at":"2026-08-30T18:24:03Z","labels":["parser","paths"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"hunk-commit-ats","title":"Harden the CI workflow: token permissions, action pinning, jj checksum","description":"Three supply-chain gaps in .github/workflows/ci.yml:\n\n1. No `permissions:` block, so `GITHUB_TOKEN` gets the repository default rather\n than the read access this job needs — and it is handed to a step explicitly\n as `GH_TOKEN` (line 20).\n2. Both actions are pinned to floating tags (`actions/checkout@v4`,\n `oven-sh/setup-bun@v2`), so a compromised or retagged release runs with that\n token.\n3. The jj release tarball is downloaded and installed with no checksum\n verification, so CI trusts whatever that asset holds at download time.\n\nFix: `permissions: contents: read` at workflow level, both actions pinned to\ncommit SHAs, and the jj download verified against the digest published with the\nrelease.\n","status":"open","priority":2,"issue_type":"chore","owner":"muzaffar.mohammed@pix4d.com","created_at":"2026-08-30T18:24:21Z","created_by":"Muzaffar Mohammed","updated_at":"2026-08-30T18:24:21Z","labels":["ci","security","supply-chain"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"hunk-commit-453","title":"Cross-check the parsed path against the path Hunk reports","description":"`findDisagreement` in src/patch/agreement.ts:18-21 compares hunk counts and line\nranges but never the path. `ReviewedFile` carries two: `file.path`, which Hunk\nresolved, and `patch.path`, which this extension parsed out of the same text —\nand only the second one ever reaches the filesystem, never compared against the\nfirst.\n\nThis module exists precisely to refuse when the two parses disagree, so a path\nmismatch belongs in it: same class of bug as a hunk-count mismatch, and it costs\none comparison.\n\nIt does not replace the containment check added for concern-8 — both sides can\nagree on a path that escapes the workspace — but it catches a parser that read\nthe wrong filename, which containment cannot. Likely to surface the quoted-path\nbug (hunk-commit-ys8) as a refusal rather than a wrong write.\n","status":"open","priority":2,"issue_type":"task","owner":"muzaffar.mohammed@pix4d.com","created_at":"2026-08-30T18:24:20Z","created_by":"Muzaffar Mohammed","updated_at":"2026-08-30T18:24:20Z","labels":["hardening","paths"],"dependencies":[{"issue_id":"hunk-commit-453","depends_on_id":"hunk-commit-ys8","type":"related","created_at":"2026-08-30T20:24:30Z","created_by":"Muzaffar Mohammed","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"hunk-commit-rd7","title":"Check whether diff.relative breaks path resolution","description":"Found while verifying that the concern-8 containment check would not reject\npaths when Hunk is launched from a subdirectory. It would not: git and jj both\nemit repo-root-relative paths from any directory, and `--relative` omits files\noutside the cwd rather than reaching them with `..`.\n\nBut that mode raises a separate question. With `diff.relative=true` in git\nconfig, paths become relative to the cwd (`a/foo.ts` for `src/deep/foo.ts` when\nrun from `src/deep`). `workspace.root` is always the repo root — `detectWorkspace`\nwalks upward — so `join(workspace.root, path)` would then resolve the wrong\nfile.\n\nUnverified: whether Hunk passes that config through, or normalises paths before\nhanding them to an extension. If it does pass through, the failure is loud\n(ENOENT into the catch in index.ts) rather than silent, so this is a correctness\nquestion, not a safety one.\n\nWork: confirm what Hunk does with `diff.relative=true`, and fix or document.\n","status":"open","priority":3,"issue_type":"task","owner":"muzaffar.mohammed@pix4d.com","created_at":"2026-08-30T18:24:21Z","created_by":"Muzaffar Mohammed","updated_at":"2026-08-30T18:24:21Z","labels":["investigate","paths"],"dependency_count":0,"dependent_count":0,"comment_count":0} diff --git a/.perles/config.yaml b/.perles/config.yaml new file mode 100644 index 0000000..a0fdfd2 --- /dev/null +++ b/.perles/config.yaml @@ -0,0 +1,174 @@ +# Perles Configuration + +# Path to beads database directory (default: current directory) +# beads_dir: /path/to/project + +# UI settings +ui: + show_counts: true # Show issue counts in column headers + show_status_bar: true # Show status bar at bottom + # markdown_style: dark # Markdown rendering style: "dark" (default) or "light" + vim_mode: false # Enable vim keybindings in text input areas (orchestration mode) + + # Keybinding overrides (optional) + # keybindings: + # search: "ctrl+space" # Default: ctrl+space + # dashboard: "ctrl+o" # Default: ctrl+o + +# Theme configuration +# Use a preset theme or customize individual colors +theme: + # Use a preset (run 'perles themes' to see available presets): + # preset: catppuccin-mocha + # + # Available presets: + # default - Default perles theme + # catppuccin-mocha - Warm, cozy dark theme + # catppuccin-latte - Warm, cozy light theme + # dracula - Dark theme with vibrant colors + # nord - Arctic, north-bluish palette + # high-contrast - High contrast for accessibility + # + # Override specific colors (works with or without preset): + # colors: + # text.primary: "#FFFFFF" + # status.error: "#FF0000" + # priority.critical: "#FF5555" + # + # See all available color tokens with 'perles themes --help' or docs + +# Board views - each view is a named collection of columns +# Cycle through views with Shift+J (next) and Shift+K (previous) +views: + - name: Default + columns: + - name: Blocked + type: bql + query: "status = open and blocked = true" + color: "#FF8787" + + - name: Deferred + type: bql + query: "status = deferred or status = open and defer_until > now" + color: "#808000" + + - name: Ready + type: bql + query: "status = open and ready = true" + color: "#73F59F" + + - name: In Progress + type: bql + query: "status = in_progress" + color: "#54A0FF" + + - name: Closed + type: bql + query: "status = closed" + color: "#BBBBBB" + +# View options: +# name: Display name for the view (required) +# columns: List of columns for this view (required) +# +# Column options: +# name: Display name (required) +# type: bql or tree +# query: BQL query (required when type is bql) - see BQL syntax below +# issue_id: Issue Id (required when type is tree) +# tree_mode: deps or child (optional when type is tree) +# color: Hex color for column header +# +# BQL Query Syntax: +# Fields: type, priority, status, blocked, ready, label, title, id, created, updated +# Operators: = != < > <= >= ~ (contains) in not-in +# Examples: +# status = open +# type = bug and priority = P0 +# blocked = true +# label in (urgent, critical) +# title ~ auth + +# Orchestration mode settings +# Configure which AI client to use when entering orchestration mode +orchestration: + # AI client provider for the coordinator: "claude" (default), "amp", "codex", "opencode", or "cursor" + coordinator_client: claude + + # AI client provider for the workers: "claude" (default), "amp", "codex", "opencode", or "cursor" + worker_client: claude + + # Claude-specific settings (only used when client: claude) + claude: + model: claude-opus-4-8 # claude-opus-4-8 (default); aliases like opus, sonnet, haiku also work + + # Codex-specific settings (only used when client: codex) + codex: + model: gpt-5.5 # gpt-5.5 (default) + + # Amp-specific settings (only used when client: amp) + amp: + model: opus # opus (default) or sonnet + mode: smart # free, rush, or smart (default) + + # OpenCode-specific settings (only used when client: opencode) + opencode: + model: anthropic/claude-opus-4-8 # anthropic/claude-opus-4-8 (default) + + # Cursor-specific settings (only used when client: cursor) + # cursor: + # model: composer-1 # Model selection (uses Cursor's default if empty) + + # Workflow templates (Ctrl+P to open picker in orchestration mode) + # User workflows are loaded from ~/.perles/workflows/*.md + # workflows: + # # Define a user workflow (loaded from ~/.perles/workflows/) + # - name: "Code Review" + # description: "Multi-perspective code review" + # file: "code_review.md" + # + # # Disable a built-in workflow + # - name: "Debate" + # enabled: false + # + # # Override name/description of a built-in workflow + # - name: "Research Proposal" + # description: "Custom description for research workflow" + + # Timeouts for orchestration initialization phases + # All values use Go duration format (e.g., "30s", "2m", "1m30s") + # timeouts: + # worktree_creation: 30s # Git worktree creation timeout (default: 30s) + # coordinator_start: 60s # Coordinator startup timeout (default: 60s) + # workspace_setup: 30s # MCP server and infrastructure setup (default: 30s) + # max_total: 120s # Maximum total initialization time (default: 120s) + + # Sound Notifications + # Audio feedback for orchestration events. All events are enabled by default. + # To override the default sounds use the override_sounds for each event. + # Custom sounds must be WAV files located in ~/.perles/sounds/ + sound: + events: + # Plays when a workflow completes successfully + workflow_complete: + enabled: true + + # Plays when a review is approved + review_verdict_approve: + enabled: true + + # Plays when a review is denied + review_verdict_deny: + enabled: true + + # Plays when a worker runs out of context + worker_out_of_context: + enabled: true + + # Plays when the coordinator runs out of context + coordinator_out_of_context: + enabled: true + + # Plays for general user notifications + user_notification: + enabled: true From 49483f7280b28d48712683141b8e45d675c1c37b Mon Sep 17 00:00:00 2001 From: Muzaffar Omer Date: Sun, 30 Aug 2026 21:08:07 +0200 Subject: [PATCH 5/8] fix: refuse patches that describe a symlink or submodule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Git stores a symlink as a file whose content is its target, so one reaches the review looking like ordinary one-line text. Nothing here interpreted a file mode, so both write sites wrote it back as text — and both resolve the destination, so the write landed wherever the link pointed, outside the workspace. The path check could not see this: it settles the path text, and a symlink escapes without a suspicious path anywhere. The reachable case is an unmarked symlink in a jj review. An unmarked file is never read from the working copy, so no staleness check runs on it, yet it still becomes a restore instruction for the helper script. Refuse at checkReviewedFile, the one place both backends pass through and which runs for every file, marked or not. Regular modes are an allowlist: an unrecognised mode is one nobody has reasoned about. The write sites are hardened too, for when the working copy and the patch disagree: O_NOFOLLOW when discarding, and an unlink before both the redirect and the cp in the jj helper. The discard environment moves out of index.ts so the integration test drives the real one rather than a copy that could not have caught this. --- .beads/interactions.jsonl | 1 + index.ts | 29 +++++++----- src/discard/workingCopy.ts | 50 +++++++++++++++++++++ src/jj/script.ts | 13 ++++++ src/patch/modes.ts | 47 ++++++++++++++++++++ src/patch/parse.ts | 43 +++++++++++++++++- src/patch/paths.ts | 4 ++ src/review/check.ts | 18 +++++++- src/ui/messages.ts | 3 ++ test/discard.integration.test.ts | 49 +++++++++++++++----- test/hostile.test.ts | 76 ++++++++++++++++++++++++++++++++ test/jj.integration.test.ts | 45 ++++++++++++++++++- 12 files changed, 351 insertions(+), 27 deletions(-) create mode 100644 .beads/interactions.jsonl create mode 100644 src/discard/workingCopy.ts create mode 100644 src/patch/modes.ts diff --git a/.beads/interactions.jsonl b/.beads/interactions.jsonl new file mode 100644 index 0000000..b7f911c --- /dev/null +++ b/.beads/interactions.jsonl @@ -0,0 +1 @@ +{"id":"int-ab595789dc8f6cf0ef12855e7de8f614","kind":"field_change","created_at":"2026-08-30T19:08:28.153996Z","actor":"Muzaffar Mohammed","issue_id":"hunk-commit-6af","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Fixed, but the trigger in the description was wrong and the fix is broader.\n\nDiscarding a marked symlink was already blocked, incidentally: checkReviewedFile\nreads the working copy for the staleness check, that read follows the link and\nreturns the target file's content, which never matches the patch — so it refused\nas `stale`. Accidental, not a guarantee.\n\nThe reachable path is an *unmarked* symlink anywhere in a jj review.\nrequiresWorkingCopyCheck is false for a `revert` disposition, so such a file is\nnever read and no staleness gate runs on it — yet it still becomes a `restore`\ninstruction for the helper script, whose `cp` resolves the destination. Marking\none unrelated hunk and staging writes through the link, outside the workspace.\nConfirmed by disabling the check and watching the copy land outside the repo.\n\nAlso found: the restore loop's `cp -p` has the same defect as the `>` redirect;\nthe description only named the redirect.\n\nFix — refusal at the boundary, not a guard at each write site:\n- src/patch/parse.ts: FilePatch.declaredModes, read from `new file mode`,\n `deleted file mode`, `old mode`, `new mode` and the trailing field on `index`.\n Both sides, since a symlink -> regular change names the symlink on the old\n side only while the working copy still holds the link.\n- src/patch/modes.ts (new): unsupportedModeReason, an allowlist of 100644/100755.\n- src/review/check.ts: new `unsupported-type` refusal, right after the path\n check — the one place both backends pass through, and it runs for every file,\n marked or not.\n\nDefence in depth:\n- src/discard/workingCopy.ts (new): the discard environment, extracted from\n index.ts so the integration test drives the real one; writes with O_NOFOLLOW.\n- src/jj/script.ts: unlink_if_symlink before both the redirect and the cp.\n A function rather than `[ -L x ] && rm`, which aborts under `set -eu`.\n\nNot done: no guard on symlinked parent directories. Neither a git index nor a\njj tree can hold `a` as a symlink and `a/b` at once, so it is not reachable\nfrom tracked content.\n\n20 tests added; the four integration tests were verified to fail with the\ncheck disabled. 201 pass, typecheck clean.\n"}} diff --git a/index.ts b/index.ts index a19135e..51338de 100644 --- a/index.ts +++ b/index.ts @@ -1,5 +1,5 @@ -import { access, mkdir, readFile, rm, writeFile } from "node:fs/promises"; -import { dirname, join } from "node:path"; +import { access, readFile } from "node:fs/promises"; +import { join } from "node:path"; import type { ExtensionCommandContext, ExtensionDiffFile, @@ -7,6 +7,7 @@ import type { HunkExtensionAPI, } from "hunkdiff/extension"; import { discardMarkedHunks, type DiscardOutcome } from "./src/discard/discard"; +import { createWorkingCopyEnvironment } from "./src/discard/workingCopy"; import { createGitBackend } from "./src/git/backend"; import { createGitCommitBackend, findCommitBlocker } from "./src/git/commit"; import { autosquashCommand, createGitFixupBackend } from "./src/git/fixup"; @@ -525,6 +526,11 @@ async function report( return; } + if (outcome.kind === "unsupported-type") { + ctx.notify(messages.unsupportedType(outcome.path, outcome.detail), "error"); + return; + } + if (outcome.kind === "nothing-staged") { ctx.notify(messages.nothingToStage, "warning"); return; @@ -568,17 +574,11 @@ async function discard(ctx: ExtensionCommandContext, session: ReviewSession): Pr return; } - const resolve = (path: string) => join(workspace.root, path); - try { - const outcome = await discardMarkedHunks(request, { - readWorkingCopyFile: (path) => readFile(resolve(path), "utf8"), - writeWorkingCopyFile: async (path, content) => { - await mkdir(dirname(resolve(path)), { recursive: true }); - await writeFile(resolve(path), content, "utf8"); - }, - removeWorkingCopyFile: (path) => rm(resolve(path), { force: true }), - }); + const outcome = await discardMarkedHunks( + request, + createWorkingCopyEnvironment(workspace.root), + ); reportDiscard(ctx, session, outcome, workspace.kind); } catch (error) { @@ -607,6 +607,11 @@ function reportDiscard( return; } + if (outcome.kind === "unsupported-type") { + ctx.notify(messages.unsupportedType(outcome.path, outcome.detail), "error"); + return; + } + if (outcome.kind === "unsupported") { ctx.notify(messages.cannotDiscard(outcome.path, outcome.detail), "error"); return; diff --git a/src/discard/workingCopy.ts b/src/discard/workingCopy.ts new file mode 100644 index 0000000..c34ed42 --- /dev/null +++ b/src/discard/workingCopy.ts @@ -0,0 +1,50 @@ +import { constants as fsConstants } from "node:fs"; +import { mkdir, readFile, rm, open } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import type { DiscardEnvironment } from "./discard"; + +/** + * The working copy, as discarding sees it. + * + * Lives here rather than inline at the call site so the guarantees below are + * the ones the tests exercise. An environment assembled separately in a test + * would pass while the real one wrote through a symlink. + */ +export function createWorkingCopyEnvironment(root: string): DiscardEnvironment { + const resolve = (path: string) => join(root, path); + + return { + readWorkingCopyFile: (path) => readFile(resolve(path), "utf8"), + writeWorkingCopyFile: async (path, content) => { + await mkdir(dirname(resolve(path)), { recursive: true }); + await writeContainedFile(resolve(path), content); + }, + // `rm` unlinks the name it is given; unlike a write, it never follows a + // symlink to its target. + removeWorkingCopyFile: (path) => rm(resolve(path), { force: true }), + }; +} + +/** + * Write a file, refusing to follow a symbolic link at the destination. + * + * `checkReviewedFile` already turns away a patch that declares a symlink, so + * reaching one here means the working copy and the patch disagree — the file + * was replaced between the diff and this write. `O_NOFOLLOW` makes that + * disagreement an error (`ELOOP`) instead of a write to wherever the link + * points, which the path check cannot prevent: that establishes the *path* + * stays inside the workspace, and a symlink is about where the *destination* + * leads. + */ +async function writeContainedFile(absolutePath: string, content: string): Promise { + const handle = await open( + absolutePath, + fsConstants.O_WRONLY | fsConstants.O_CREAT | fsConstants.O_TRUNC | fsConstants.O_NOFOLLOW, + ); + + try { + await handle.writeFile(content, "utf8"); + } finally { + await handle.close(); + } +} diff --git a/src/jj/script.ts b/src/jj/script.ts index c927822..74858da 100644 --- a/src/jj/script.ts +++ b/src/jj/script.ts @@ -26,11 +26,23 @@ left="$1" right="$2" stage="$3" +# A symlink is unlinked before it is written to or copied over. Redirection +# and cp both resolve the destination, so either would write *through* a link +# in $right to wherever it points, outside the directory jj gave us. Written +# as a function so set -e does not trip over a false test, the way a +# test-and-rm one-liner would when the test fails. +unlink_if_symlink() { + if [ -L "$1" ]; then + rm -f "$1" + fi +} + # Replace content by redirection rather than by copying: it leaves the file's # existing mode alone, so an executable bit survives being rewritten. while IFS= read -r path; do [ -n "$path" ] || continue mkdir -p "$right/$(dirname "$path")" + unlink_if_symlink "$right/$path" cat "$stage/${CONTENT_DIRECTORY}/$path" > "$right/$path" done < "$stage/${WRITE_MANIFEST}" @@ -43,6 +55,7 @@ while IFS= read -r path; do [ -n "$path" ] || continue if [ -e "$left/$path" ]; then mkdir -p "$right/$(dirname "$path")" + unlink_if_symlink "$right/$path" cp -p "$left/$path" "$right/$path" else rm -f "$right/$path" diff --git a/src/patch/modes.ts b/src/patch/modes.ts new file mode 100644 index 0000000..0ead069 --- /dev/null +++ b/src/patch/modes.ts @@ -0,0 +1,47 @@ +/** + * Which file types this extension can rebuild. + * + * Every path here is reconstructed the same way: take the working copy's text, + * apply or revert the marked hunks, write the text back. That is only correct + * for a regular file. Git stores a symlink as a file whose content is its + * target, and a submodule as a file whose content is a commit id, so both + * arrive as ordinary-looking text hunks and neither survives being written + * back as text — a symlink would either be followed to wherever it points or + * flattened into a regular file, and a submodule is not a file at all. + * + * The alternative to refusing is teaching the whole pipeline about file types, + * which is a much larger change for cases a reviewer meets rarely. Refusing is + * the honest answer: this cannot rebuild them, so it declines to try. + */ + +/** The modes a patch may name: a regular file, executable or not. */ +const REGULAR_MODES = new Set(["100644", "100755"]); + +const MODE_NAMES: Readonly> = { + "120000": "a symbolic link", + "160000": "a submodule", + "040000": "a directory", + "040755": "a directory", +}; + +/** + * Why the file this patch describes cannot be rebuilt, or null when it can. + * + * An allowlist rather than a list of known-bad modes: a mode nobody here has + * seen is a mode nobody here has reasoned about, and the failure mode of + * guessing wrong is writing the wrong thing to disk. + */ +export function unsupportedModeReason(modes: readonly string[]): string | null { + for (const mode of modes) { + if (REGULAR_MODES.has(mode)) { + continue; + } + + const name = MODE_NAMES[mode]; + return name === undefined + ? `it has an unrecognised file mode (${mode})` + : `it is ${name}, which hunks cannot describe`; + } + + return null; +} diff --git a/src/patch/parse.ts b/src/patch/parse.ts index 8bdc629..b1e157a 100644 --- a/src/patch/parse.ts +++ b/src/patch/parse.ts @@ -50,6 +50,14 @@ export interface FilePatch { /** The old path, present only for a rename. */ readonly previousPath?: string; readonly change: FileChangeKind; + /** + * Every file mode the header names, in the order it names them. + * + * Both sides, not just the new one: a patch that turns a symlink into a + * regular file declares the symlink on the old side only, and the working + * copy it is about to be written to still holds the symlink. + */ + readonly declaredModes: readonly string[]; /** True when the patch carries no usable text hunks because the file is binary. */ readonly binary: boolean; readonly hunks: readonly PatchHunk[]; @@ -81,6 +89,7 @@ interface Header { renamedFrom?: string; renamedTo?: string; explicitChange?: "added" | "deleted"; + modes: string[]; binary: boolean; /** Index of the first line that is not part of the header. */ bodyStart: number; @@ -107,8 +116,29 @@ function parseGitLinePaths(rest: string): { old: string; new: string } | undefin return candidates.find((candidate) => candidate.old === candidate.new) ?? candidates[0]; } +/** + * Note a mode named by a header line, ignoring anything that is not one. + * + * Lenient on purpose: an unrecognised field is not a mode, and a mode this + * does not collect is one `unsupportedModeReason` never gets to reject. The + * strictness belongs there, where an unknown mode is refused rather than + * assumed harmless. + */ +function recordMode(header: Header, value: string): void { + const mode = value.trim(); + if (/^\d{6}$/.test(mode)) { + header.modes.push(mode); + } +} + function parseHeader(lines: readonly string[]): Header { - const header: Header = { oldPath: "", newPath: "", binary: false, bodyStart: lines.length }; + const header: Header = { + oldPath: "", + newPath: "", + modes: [], + binary: false, + bodyStart: lines.length, + }; for (const [index, line] of lines.entries()) { if (HUNK_HEADER.test(line)) { @@ -128,8 +158,18 @@ function parseHeader(lines: readonly string[]): Header { header.renamedTo = line.slice("rename to ".length); } else if (line.startsWith("new file mode")) { header.explicitChange = "added"; + recordMode(header, line.slice("new file mode".length)); } else if (line.startsWith("deleted file mode")) { header.explicitChange = "deleted"; + recordMode(header, line.slice("deleted file mode".length)); + } else if (line.startsWith("old mode ")) { + recordMode(header, line.slice("old mode ".length)); + } else if (line.startsWith("new mode ")) { + recordMode(header, line.slice("new mode ".length)); + } else if (line.startsWith("index ")) { + // `index .. `, where the mode appears only when it did + // not change — a changed one is on the `old mode`/`new mode` lines above. + recordMode(header, line.slice("index ".length).split(" ").slice(1).join(" ")); } else if (line.startsWith("Binary files ") || line.startsWith("GIT binary patch")) { header.binary = true; } @@ -258,6 +298,7 @@ export function parseFilePatch(patchText: string): FilePatch { return { ...resolvePaths(header), headerLines: lines.slice(0, header.bodyStart), + declaredModes: header.modes, binary: header.binary, hunks, }; diff --git a/src/patch/paths.ts b/src/patch/paths.ts index a3910d0..dacdbf1 100644 --- a/src/patch/paths.ts +++ b/src/patch/paths.ts @@ -8,6 +8,10 @@ * `/etc/x`, not an error. So containment has to be established before the * join, never by it. * + * This settles the path *text* only. Where the path leads is a separate + * question — a symlink escapes without a suspicious path anywhere — and is + * answered by `unsupportedModeReason` and by the write sites themselves. + * * Refusing costs nothing, because no working copy can describe what this * rejects: git and jj both emit repo-root-relative paths from any directory, * and git's one cwd-relative mode (`--relative`) omits files outside the cwd diff --git a/src/review/check.ts b/src/review/check.ts index f38c400..88c7596 100644 --- a/src/review/check.ts +++ b/src/review/check.ts @@ -1,6 +1,7 @@ import { parseDocument, type Document } from "../patch/document"; import { findDisagreement, type HostHunk } from "../patch/agreement"; import { parseFilePatch, type FilePatch } from "../patch/parse"; +import { unsupportedModeReason } from "../patch/modes"; import { unsafePathReason } from "../patch/paths"; import { findStaleHunk } from "../patch/select"; import { disposeFile, requiresWorkingCopyCheck, type FileDisposition, type FileMark } from "../staging/plan"; @@ -21,12 +22,15 @@ export interface ReviewedFile { * ways, so they are reported separately. `unsafe-path` is a different kind of * answer: not "this is out of date" but "this patch names a file no diff of a * working copy could name", which is a bug here or a patch from somewhere it - * should not have come from. + * should not have come from. `unsupported-type` is a third: the patch is + * perfectly well formed and names a real file, but the file is not one this + * extension can rebuild from text. */ export type ReviewRefusal = | { readonly kind: "stale"; readonly path: string; readonly detail: string } | { readonly kind: "disagreement"; readonly path: string; readonly detail: string } - | { readonly kind: "unsafe-path"; readonly path: string; readonly detail: string }; + | { readonly kind: "unsafe-path"; readonly path: string; readonly detail: string } + | { readonly kind: "unsupported-type"; readonly path: string; readonly detail: string }; /** One reviewed file, once every check has passed and its fate is known. */ export interface CheckedFile { @@ -81,6 +85,16 @@ export async function checkReviewedFile( return unsafe; } + // Immediately after the path check and for the same reason: this is the one + // place both backends pass through, so refusing here is what keeps a symlink + // out of the git write in `discard` and out of the jj staging directory at + // once. Checked for every file, marked or not — an unmarked file still + // becomes a `restore` instruction in a jj revision. + const unsupportedMode = unsupportedModeReason(patch.declaredModes); + if (unsupportedMode !== null) { + return { kind: "unsupported-type", path: patch.path, detail: unsupportedMode }; + } + const disagreement = findDisagreement(patch, file.hostHunks); if (disagreement) { return { kind: "disagreement", path: file.path, detail: disagreement }; diff --git a/src/ui/messages.ts b/src/ui/messages.ts index 9dca744..bfd24ad 100644 --- a/src/ui/messages.ts +++ b/src/ui/messages.ts @@ -104,6 +104,9 @@ export const messages = { unsafePath: (path: string, detail: string) => `Refusing to touch ${JSON.stringify(path)}: ${detail}. Nothing was changed — no ordinary diff names a file this way, so this is a bug or a patch worth being suspicious of. Please report it.`, + unsupportedType: (path: string, detail: string) => + `Cannot act on ${path}: ${detail}. Nothing was changed — use your VCS directly for this file.`, + /** Whatever the backend reported, whichever backend it was. */ failed: (detail: string) => `Could not finish: ${detail}`, diff --git a/test/discard.integration.test.ts b/test/discard.integration.test.ts index 3bfeed5..cdca7a1 100644 --- a/test/discard.integration.test.ts +++ b/test/discard.integration.test.ts @@ -1,7 +1,8 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; -import { dirname, join } from "node:path"; +import { readFile, readlink, rm, symlink } from "node:fs/promises"; +import { join } from "node:path"; import { discardMarkedHunks, type DiscardOutcome } from "../src/discard/discard"; +import { createWorkingCopyEnvironment } from "../src/discard/workingCopy"; import type { FileMark } from "../src/staging/plan"; import { createTestRepository, hasJujutsu, type TestRepository } from "./support/repo"; import { reviewFromPatch } from "./support/review"; @@ -42,17 +43,11 @@ async function discard(marks: Record): Promise }), ); - const resolve = (path: string) => join(repository.root, path); + // The environment the extension itself builds, not a copy of it: a copy + // would keep passing while the real one wrote through a symlink. return discardMarkedHunks( { files, marks: marksById }, - { - readWorkingCopyFile: (path) => readFile(resolve(path), "utf8"), - writeWorkingCopyFile: async (path, content) => { - await mkdir(dirname(resolve(path)), { recursive: true }); - await writeFile(resolve(path), content, "utf8"); - }, - removeWorkingCopyFile: (path) => rm(resolve(path), { force: true }), - }, + createWorkingCopyEnvironment(repository.root), ); } @@ -145,6 +140,38 @@ describeWithJj("discarding whole-file changes", () => { }); }); +describeWithJj("a symbolic link", () => { + /** + * Git and jj both store a symlink as a file whose content is its target, so + * one reaches the review looking like an ordinary one-line text file. What + * makes it dangerous is the destination, not the patch: a write at that path + * lands wherever the link points, which can be anywhere on the machine. + */ + beforeEach(async () => { + await repository.write("secret.txt", "do not touch\n"); + await symlink(join(repository.root, "secret.txt"), join(repository.root, "link")); + await repository.jj("commit", "-m", "base"); + + await rm(join(repository.root, "link")); + await symlink(join(repository.root, "elsewhere.txt"), join(repository.root, "link")); + }); + + test("is refused rather than rebuilt from its target text", async () => { + const outcome = await discard({ link: { kind: "whole" } }); + + expect(outcome).toMatchObject({ kind: "unsupported-type", path: "link" }); + }); + + test("leaves the file it points at untouched", async () => { + await discard({ link: { kind: "whole" } }); + + expect(await repository.read("secret.txt")).toBe("do not touch\n"); + expect(await readlink(join(repository.root, "link"))).toBe( + join(repository.root, "elsewhere.txt"), + ); + }); +}); + describeWithJj("what the confirmation promises", () => { test("a discard in jj really is recoverable with jj undo", async () => { await repository.write("f.txt", numberedLines(6)); diff --git a/test/hostile.test.ts b/test/hostile.test.ts index 4bef8d1..d111053 100644 --- a/test/hostile.test.ts +++ b/test/hostile.test.ts @@ -42,6 +42,24 @@ function patchRenamingFrom(from: string): string { ].join("\n"); } +/** + * A patch whose header carries `modeLines` — the shapes git uses to name a + * file's type. An unchanged mode rides on the `index` line; a changed one is + * spelled out; a created or deleted file names it once. + */ +function patchWithModeLines(...modeLines: readonly string[]): string { + return [ + "diff --git a/link b/link", + ...modeLines, + "--- a/link", + "+++ b/link", + "@@ -1,1 +1,1 @@", + "-before", + "+after", + "", + ].join("\n"); +} + const ESCAPING = "../../outside.txt"; const WHOLE: FileMark = { kind: "whole" }; @@ -133,6 +151,54 @@ describe("a patch naming a path outside the workspace", () => { }); }); +describe("a patch describing something that is not a regular file", () => { + /** + * Git stores a symlink as a file whose content is its target and a submodule + * as a file whose content is a commit id, so both arrive looking like + * ordinary one-line text. Only the mode says otherwise, and writing either + * back as text is wrong however contained the path is — a symlink would be + * followed to wherever it points. + */ + const shapes = [ + ["a symlink whose target changed", patchWithModeLines("index d43b9f..636062 120000")], + ["a new symlink", patchWithModeLines("new file mode 120000", "index 000000..636062")], + ["a deleted symlink", patchWithModeLines("deleted file mode 120000", "index 636062..000000")], + [ + "a symlink turning into a regular file", + patchWithModeLines("old mode 120000", "new mode 100644", "index d43b9f..636062"), + ], + ["a submodule", patchWithModeLines("index d43b9f..636062 160000")], + ["a mode nobody here has reasoned about", patchWithModeLines("index d43b9f..636062 100777")], + ] as const; + + test.each(shapes)("is refused before staging touches anything: %s", async (_name, patchText) => { + const { outcome, touched } = await stage(patchText); + + expect(outcome.kind).toBe("unsupported-type"); + expect(touched).toEqual([]); + }); + + test.each(shapes)( + "is refused before discarding touches anything: %s", + async (_name, patchText) => { + const { outcome, touched } = await discard(patchText); + + expect(outcome.kind).toBe("unsupported-type"); + expect(touched).toEqual([]); + }, + ); + + test("says what the file is, so the refusal is actionable", async () => { + const { outcome } = await discard(patchWithModeLines("index d43b9f..636062 120000")); + + expect(outcome).toMatchObject({ + kind: "unsupported-type", + path: "link", + detail: expect.stringContaining("symbolic link"), + }); + }); +}); + describe("an ordinary patch", () => { test("still stages", async () => { const { outcome, touched } = await stage(patchNaming("src/f.txt")); @@ -147,4 +213,14 @@ describe("an ordinary patch", () => { expect(outcome.kind).toBe("discarded"); expect(touched).toContain("write src/f.txt"); }); + + test.each([ + ["a plain file", "index d43b9f..636062 100644"], + ["an executable file", "index d43b9f..636062 100755"], + ["a file that gained its executable bit", "old mode 100644\nnew mode 100755"], + ])("stages when its mode is a regular one: %s", async (_name, modeLine) => { + const { outcome } = await stage(patchWithModeLines(modeLine)); + + expect(outcome.kind).toBe("staged"); + }); }); diff --git a/test/jj.integration.test.ts b/test/jj.integration.test.ts index 3880524..53560ef 100644 --- a/test/jj.integration.test.ts +++ b/test/jj.integration.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { readFile, rm } from "node:fs/promises"; +import { readFile, rm, symlink, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { createJjBackend } from "../src/jj/backend"; import type { JjDestination } from "../src/jj/tool"; @@ -204,6 +204,49 @@ describeWithJj("staging whole-file changes", () => { }); }); +describeWithJj("a symbolic link in the review", () => { + /** + * The reachable case, and the reason the type check sits in + * `checkReviewedFile` rather than at either write site. An *unmarked* file + * is never read from the working copy — leaving it behind is correct however + * stale it is — so nothing compares it against the patch. It still becomes a + * `restore` instruction for the helper script, whose `cp` resolves the + * destination: with a symlink there, the copy lands wherever it points, + * outside the directory jj handed us. + */ + // Both sides of the link point outside the repository, because both are + // written to: the helper reads through the old target and writes through the + // new one. + const outside = (name: string) => join(repository.root, "..", name); + + beforeEach(async () => { + await writeFile(outside("was.txt"), "old target\n", "utf8"); + await writeFile(outside("now.txt"), "do not touch\n", "utf8"); + await symlink(outside("was.txt"), join(repository.root, "link")); + await repository.write("text.txt", "before\n"); + await repository.jj("commit", "-m", "base"); + + await rm(join(repository.root, "link")); + await symlink(outside("now.txt"), join(repository.root, "link")); + await repository.write("text.txt", "after\n"); + }); + + test("refuses to stage the file it was not even marked on", async () => { + const outcome = await stage({ "text.txt": { kind: "whole" } }); + + expect(outcome).toMatchObject({ kind: "unsupported-type", path: "link" }); + }); + + test("leaves the file outside the workspace untouched", async () => { + await stage({ "text.txt": { kind: "whole" } }); + + expect(await readFile(outside("now.txt"), "utf8")).toBe("do not touch\n"); + expect(await readFile(outside("was.txt"), "utf8")).toBe("old target\n"); + // Nothing moved either: the refusal happens before any backend runs. + expect(await repository.jj("diff", "--git")).toContain("text.txt"); + }); +}); + describeWithJj("extracting a new revision", () => { beforeEach(async () => { await repository.write("f.txt", numberedLines(20)); From 88666a5e750c239e19ee4129d8319c70a25c914e Mon Sep 17 00:00:00 2001 From: Muzaffar Omer Date: Sun, 30 Aug 2026 21:19:47 +0200 Subject: [PATCH 6/8] chore(ci): pin actions and verify jj download The CI job hands GITHUB_TOKEN to a step as GH_TOKEN, so three supply-chain gaps compounded: no permissions block (the token inherited whatever the repository default happened to be), both actions on floating tags that can be retagged onto attacker code, and a jj tarball installed as root with neither a pinned version nor a digest check. Declare permissions: contents: read, pin actions/checkout and setup-bun to commit SHAs, and pin jj to v0.44.0 verified against a recorded SHA-256. jj publishes no checksum file, so the digest was recorded by hand: it catches a later swap of the asset, it does not attest the build. --- .github/workflows/ci.yml | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 41791f5..d7d0d06 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,22 +5,29 @@ on: branches: [main] pull_request: +permissions: + contents: read + jobs: check: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - - uses: oven-sh/setup-bun@v2 + - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 # The jj suites skip themselves when the binary is missing, so CI has to # install it — otherwise a whole backend goes green without being run. - name: Install jj env: GH_TOKEN: ${{ github.token }} + JJ_VERSION: v0.44.0 + JJ_SHA256: 0a07bab4641a55fd2bc2fd1563ba3a3f9a577584086ad74086a1c5b69b3ffce9 run: | - gh release download --repo jj-vcs/jj \ - --pattern 'jj-*-x86_64-unknown-linux-musl.tar.gz' --output jj.tar.gz + gh release download "$JJ_VERSION" --repo jj-vcs/jj \ + --pattern "jj-$JJ_VERSION-x86_64-unknown-linux-musl.tar.gz" \ + --output jj.tar.gz + echo "$JJ_SHA256 jj.tar.gz" | sha256sum -c - tar -xzf jj.tar.gz ./jj sudo install jj /usr/local/bin/jj rm jj jj.tar.gz From d6849d23a58921259a91a5dd5cdd54e299e0d620 Mon Sep 17 00:00:00 2001 From: Muzaffar Omer Date: Tue, 1 Sep 2026 18:02:49 +0200 Subject: [PATCH 7/8] chore(deps): pin devDependency ranges for Renovate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renovate cannot manage the 'latest' dist-tag, and a range as loose as '^5' never produces a PR because every new 5.x already satisfies it — both bumps would land silently in bun.lock instead of arriving as a reviewable diff. 'latest' was already decorative: CI runs 'bun install --frozen-lockfile', so the lockfile pin won and the dist-tag only took effect when someone regenerated the lockfile, moving the version with no diff anyone reviewed. Both ranges are set to the versions bun.lock already resolved, so no install changes. hunkdiff is left at ^0.20.0 — a caret on a 0.x version is already tight (>=0.20.0 <0.21.0). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01V5yBUZqwwtDFrkKLtANyLw --- bun.lock | 4 ++-- package.json | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bun.lock b/bun.lock index fde5803..c520d30 100644 --- a/bun.lock +++ b/bun.lock @@ -5,9 +5,9 @@ "": { "name": "hunk-jj-stage", "devDependencies": { - "@types/bun": "latest", + "@types/bun": "^1.4.0", "hunkdiff": "^0.20.0", - "typescript": "^5", + "typescript": "^5.9.3", }, }, }, diff --git a/package.json b/package.json index 184d83a..c0b0ab4 100644 --- a/package.json +++ b/package.json @@ -15,8 +15,8 @@ "typecheck": "tsc --noEmit" }, "devDependencies": { - "@types/bun": "latest", + "@types/bun": "^1.4.0", "hunkdiff": "^0.20.0", - "typescript": "^5" + "typescript": "^5.9.3" } } From 1374c5b91cd1fb67cab97adf75e0712c17538b3b Mon Sep 17 00:00:00 2001 From: Muzaffar Omer Date: Tue, 1 Sep 2026 18:02:49 +0200 Subject: [PATCH 8/8] ci(renovate): add Renovate config for npm and GitHub Actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Configures the hosted Mend Renovate app to manage npm devDependencies and GitHub Action pins. ci.yml already SHA-pins actions by hand, but a hand-pinned SHA is frozen forever — we get the supply-chain benefit and none of the security updates. config:best-practices keeps the pin and moves it. Cooldown against compromised uploads uses two settings that must go together: minimumReleaseAge alone only marks an internal check as pending, so Renovate still opens the PR and automerge can merge it unreviewed once the check flips green. internalChecksFilter 'strict' suppresses the PR until the version is old enough, turning the cooldown into an actual filter rather than a delay. Because that cooldown would also delay a release that fixes a CVE, security updates bypass both the age rule and the weekly schedule. hunkdiff never automerges: it is imported only as 'import type', so it is erased at compile time and 'bun test' passes identically on any version. Only tsc covers it, for 4 interfaces. It is also 0.x, where Renovate calls a breaking change a 'minor'. Written as JSON5 so the reasoning lives next to the settings it explains. Config is inert until the app is installed and Dependabot alerts are enabled. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01V5yBUZqwwtDFrkKLtANyLw --- renovate.json5 | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 renovate.json5 diff --git a/renovate.json5 b/renovate.json5 new file mode 100644 index 0000000..e5d4d56 --- /dev/null +++ b/renovate.json5 @@ -0,0 +1,69 @@ +{ + $schema: "https://docs.renovatebot.com/renovate-schema.json", + + // best-practices over recommended mainly for helpers:pinGitHubActionDigests. + // ci.yml already SHA-pins actions by hand, but a hand-pinned SHA is frozen + // forever: we get the supply-chain benefit and none of the security updates. + // The preset keeps the pin *and* moves it. + extends: [ + "config:best-practices", + "schedule:weekly", + ], + + // Cooldown against compromised uploads. The npm attack pattern is a malicious + // patch release that gets detected and unpublished within hours to a few days, + // so a 7-day window means we never see it. + // + // Both settings are required. minimumReleaseAge alone only marks an internal + // check as pending — Renovate still opens the PR, and with automerge enabled + // that PR can merge itself the moment the check flips green, unreviewed. + // "strict" suppresses the PR entirely until the version is old enough, which + // turns the cooldown from a delay into an actual filter. + minimumReleaseAge: "7 days", + internalChecksFilter: "strict", + + // The cooldown above would also delay a release that exists to *fix* a CVE, + // making us less safe against the likelier threat. Security updates therefore + // skip the age rule and the weekly schedule. Requires Dependabot alerts to be + // enabled on the repo — without that, this fast-path never fires. + vulnerabilityAlerts: { + minimumReleaseAge: null, + schedule: ["at any time"], + }, + + // Kept on deliberately: with internalChecksFilter "strict", some updates are + // intentionally invisible. Without the dashboard, "no pending updates" and + // "three updates suppressed" look identical. + dependencyDashboard: true, + + packageRules: [ + // Every dependency here is a devDependency, nothing ships to a consumer, + // and the package is not published to npm — so the blast radius of a bad + // automerge is "CI is red on main and we revert", not "users are broken". + { + matchManagers: ["npm"], + matchDepTypes: ["devDependencies"], + matchUpdateTypes: ["patch", "minor"], + automerge: true, + }, + { + matchManagers: ["github-actions"], + matchUpdateTypes: ["digest", "patch", "minor"], + automerge: true, + }, + + // hunkdiff is the one dependency CI cannot vouch for. It is imported only + // as `import type` (index.ts, src/ui/session.ts), so it is erased at compile + // time and never loaded at runtime — `bun test` passes identically on any + // version. Only `tsc --noEmit` covers it, and only for the 4 interfaces we + // import. It is also 0.x, where Renovate calls 0.20 -> 0.21 a "minor" even + // though that is breaking by convention. Green CI here means very little: + // read the changelog and load the extension in Hunk before merging. + // + // Listed last so it overrides the devDependencies rule above. + { + matchPackageNames: ["hunkdiff"], + automerge: false, + }, + ], +}