An opinionated, intent-driven development framework for product thinkers.
",
+ "+ A host-agnostic configuration layer for intent-driven development.
",
+ } {
+ if !strings.Contains(u, want) {
+ t.Errorf("unified diff missing %q:\n%s", want, u)
+ }
+ }
+ // The surrounding lines are preserved as context, not rewritten.
+ if !strings.Contains(u, " `
+### `abcd identity`
+
+Show this repo's canonical identity block and every surface held to it (read-only)
+
+**Usage:** `abcd identity`
+
+#### `abcd identity init`
+
+Record this repo's identity block and the pointer to it (adopts an existing block)
+
+**Usage:** `abcd identity init [flags]`
+
+**Flags:**
+
+```
+ --file string repo-relative file the identity block lives in (default .abcd/development/IDENTITY.md)
+ --heading string heading the identity block sits under (default "Identity (canonical)")
+ --pitch string the project's short elevator pitch (optional)
+ --tagline string the project's one-line tagline (required unless a block already exists)
+ --title string the project's title (required unless a block already exists)
+```
+
+#### `abcd identity render`
+
+Print the proposed correction for every drifted surface as a unified diff (writes nothing)
+
+**Usage:** `abcd identity render`
+
### `abcd intent`
Intent lifecycle; bare invocation is read-only status, quoted text files a draft
diff --git a/internal/core/positioning/init.go b/internal/core/positioning/init.go
new file mode 100644
index 0000000..154f984
--- /dev/null
+++ b/internal/core/positioning/init.go
@@ -0,0 +1,191 @@
+package positioning
+
+import (
+ "bytes"
+ "encoding/json"
+ "errors"
+ "fmt"
+ "os"
+ "path/filepath"
+ "strings"
+
+ "github.com/REPPL/abcd-cli/internal/fsutil"
+)
+
+// DefaultBlockLocation is where a repo's identity block lands when onboarding
+// does not name a location: one file in the durable record, under the heading
+// the parser and every generated pointer use.
+var DefaultBlockLocation = BlockLocation{
+ File: ".abcd/development/IDENTITY.md",
+ Heading: "Identity (canonical)",
+}
+
+// Init refusals, so a surface can render each one as its own message.
+var (
+ // ErrAlreadyAdopted: the repo has a registry already, and Init was given
+ // answers that would change what it points at. Repointing the canon is a
+ // deliberate edit, never a side effect of re-running onboarding.
+ ErrAlreadyAdopted = errors.New("this repo already records an identity block")
+ // ErrAnswersRequired: no block exists to adopt, and the required answers
+ // (title and tagline) were not supplied.
+ ErrAnswersRequired = errors.New("an identity block needs at least a title and a tagline")
+)
+
+// InitRequest is the onboarding interview's outcome. The host asks the
+// questions (the prepare surface carries the wording); this is what it hands
+// over. Pitch may be empty — it is skippable at onboarding.
+type InitRequest struct {
+ Title string
+ Tagline string
+ Pitch string
+ Location BlockLocation // zero value means DefaultBlockLocation
+}
+
+// InitResult reports what onboarding did, so a surface can say it plainly.
+type InitResult struct {
+ Location BlockLocation `json:"location"`
+ Block Block `json:"block"`
+ // AdoptedExisting: a block was already there and was registered as the
+ // canon rather than re-interviewed and rewritten.
+ AdoptedExisting bool `json:"adoptedExisting"`
+ // NoOp: the repo was already fully adopted; nothing was written.
+ NoOp bool `json:"noOp"`
+ // Wrote lists the repo-relative paths written, in order.
+ Wrote []string `json:"wrote,omitempty"`
+}
+
+// Init adopts the positioning check for a repo: it registers an identity block
+// and writes the pointer that records where it lives.
+//
+// It never re-interviews a repo that already has a block — an existing block at
+// the target location is adopted as the canon and left byte-for-byte alone,
+// because a second set of answers is exactly how a project ends up with two
+// canons. A repo whose registry is already committed is a no-op when no answers
+// are given, and a refusal when they are: repointing the canon is a deliberate
+// edit to the registry, not a side effect of re-running onboarding.
+func Init(root string, req InitRequest) (InitResult, error) {
+ loc := req.Location
+ if strings.TrimSpace(loc.File) == "" {
+ loc.File = DefaultBlockLocation.File
+ }
+ if strings.TrimSpace(loc.Heading) == "" {
+ loc.Heading = DefaultBlockLocation.Heading
+ }
+ if !fsutil.ValidRelPath(loc.File) {
+ return InitResult{}, fmt.Errorf("%w: %q", ErrBadLocation, loc.File)
+ }
+
+ answered := strings.TrimSpace(req.Title) != "" || strings.TrimSpace(req.Tagline) != "" || strings.TrimSpace(req.Pitch) != ""
+
+ if existing, adopted, err := LoadConfig(root); err != nil {
+ return InitResult{}, err
+ } else if adopted {
+ if answered {
+ return InitResult{}, fmt.Errorf("%w: it points at %s (%q); change the block there, or edit %s deliberately",
+ ErrAlreadyAdopted, existing.Block.File, existing.Block.Heading, ConfigRelPath)
+ }
+ block, err := ParseBlock(root, existing.Block)
+ if err != nil {
+ return InitResult{}, err
+ }
+ return InitResult{Location: existing.Block, Block: block, NoOp: true}, nil
+ }
+
+ res := InitResult{Location: loc}
+
+ // An existing block at the target location is the canon: register it,
+ // never rewrite it.
+ if block, err := ParseBlock(root, loc); err == nil {
+ res.Block, res.AdoptedExisting = block, true
+ } else if !errors.Is(err, ErrBlockFileMissing) && !errors.Is(err, ErrHeadingMissing) && !errors.Is(err, ErrBulletMissing) {
+ return InitResult{}, err
+ } else {
+ title, tagline := strings.TrimSpace(req.Title), strings.TrimSpace(req.Tagline)
+ if title == "" || tagline == "" {
+ return InitResult{}, fmt.Errorf("%w (the pitch is optional)", ErrAnswersRequired)
+ }
+ if err := writeBlock(root, loc, title, tagline, strings.TrimSpace(req.Pitch)); err != nil {
+ return InitResult{}, err
+ }
+ res.Wrote = append(res.Wrote, loc.File)
+ block, err := ParseBlock(root, loc)
+ if err != nil {
+ // The block we just wrote must parse; if it does not, the scaffold
+ // is broken and must not be registered as a canon.
+ return InitResult{}, fmt.Errorf("the scaffolded identity block does not parse: %w", err)
+ }
+ res.Block = block
+ }
+
+ if err := writeConfig(root, DefaultConfig(loc)); err != nil {
+ return InitResult{}, err
+ }
+ res.Wrote = append(res.Wrote, ConfigRelPath)
+ return res, nil
+}
+
+// blockSection renders the identity block as a markdown section. The prose
+// above the bullets says what the block is for, so a maintainer meeting the file
+// cold knows why editing it matters.
+func blockSection(heading, title, tagline, pitch string) string {
+ var b strings.Builder
+ fmt.Fprintf(&b, "## %s\n\n", heading)
+ b.WriteString("The single recorded home for how this project names itself. Every public\n" +
+ "surface — the README strapline, the package or plugin manifest description,\n" +
+ "the conventions file's opening — renders from these three lines; a surface\n" +
+ "that says something else is drift, fixed by re-rendering from here or by\n" +
+ "deliberately changing this block.\n\n")
+ fmt.Fprintf(&b, "- **Title:** %s\n", title)
+ fmt.Fprintf(&b, "- **Tagline:** %s\n", tagline)
+ if pitch != "" {
+ fmt.Fprintf(&b, "- **Pitch:** %s\n", pitch)
+ }
+ return b.String()
+}
+
+// writeBlock lands the identity section at loc: appended to the file when it
+// already exists (never replacing what is there), or as a new file with a title
+// heading when it does not.
+func writeBlock(root string, loc BlockLocation, title, tagline, pitch string) error {
+ path := filepath.Join(root, filepath.FromSlash(loc.File))
+ section := blockSection(loc.Heading, title, tagline, pitch)
+
+ existing, err := fsutil.ReadGuarded(path, maxBlockFileBytes)
+ switch {
+ case err == nil:
+ body := normalizeNewlines(existing)
+ if !strings.HasSuffix(body, "\n") {
+ body += "\n"
+ }
+ if !strings.HasSuffix(body, "\n\n") {
+ body += "\n"
+ }
+ return fsutil.WriteFileAtomicPreserveMode(path, []byte(body+section))
+ case os.IsNotExist(err):
+ if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+ return err
+ }
+ return fsutil.WriteFileAtomic(path, []byte("# Identity\n\n"+section), 0o644)
+ default:
+ return fmt.Errorf("%s: %s", loc.File, pathFreeReason(err))
+ }
+}
+
+// writeConfig lands the registry, pretty-printed with a trailing newline,
+// through the canonical atomic primitive.
+func writeConfig(root string, cfg Config) error {
+ path := filepath.Join(root, filepath.FromSlash(ConfigRelPath))
+ if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+ return err
+ }
+ var buf bytes.Buffer
+ enc := json.NewEncoder(&buf)
+ // The registry carries regexps full of characters HTML escaping would
+ // mangle beyond recognition on the round trip.
+ enc.SetEscapeHTML(false)
+ enc.SetIndent("", " ")
+ if err := enc.Encode(cfg); err != nil {
+ return err
+ }
+ return fsutil.WriteFileAtomic(path, buf.Bytes(), 0o644)
+}
diff --git a/internal/surface/cli/cli.go b/internal/surface/cli/cli.go
index 85d4f37..9f3916d 100644
--- a/internal/surface/cli/cli.go
+++ b/internal/surface/cli/cli.go
@@ -93,6 +93,7 @@ func NewRootCommand() *cobra.Command {
root.AddCommand(newAhoyCommand(&asJSON))
root.AddCommand(newAuditCommand(&asJSON))
+ root.AddCommand(newIdentityCommand(&asJSON))
var launchDryRun bool
launchCmd := &cobra.Command{
diff --git a/internal/surface/cli/identity.go b/internal/surface/cli/identity.go
new file mode 100644
index 0000000..b935b8c
--- /dev/null
+++ b/internal/surface/cli/identity.go
@@ -0,0 +1,195 @@
+package cli
+
+import (
+ "fmt"
+ "io"
+ "os"
+
+ "github.com/spf13/cobra"
+
+ "github.com/REPPL/abcd-cli/internal/core/positioning"
+ "github.com/REPPL/abcd-cli/internal/termsafe"
+)
+
+// newIdentityCommand wires the `abcd identity` family: the repo's canonical
+// self-description and the surfaces held to it.
+//
+// - bare — read-only status: the block, and every registered surface's verdict.
+// - render — the proposed correction for each drifted surface, as a unified
+// diff on stdout. It writes nothing, and no flag makes it: a deliberate
+// identity change is an edit to the block, after which the same proposal
+// flow chases the surfaces.
+// - init — the write path: scaffold the block and the pointer that records
+// where it lives. It adopts an existing block rather than re-interviewing.
+//
+// The drift check itself runs on every `abcd audit`; this family is where a
+// maintainer looks at the canon and at what a fix would look like.
+func newIdentityCommand(asJSON *bool) *cobra.Command {
+ identityCmd := &cobra.Command{
+ Use: "identity",
+ Short: "Show this repo's canonical identity block and every surface held to it (read-only)",
+ Args: cobra.NoArgs,
+ RunE: func(cmd *cobra.Command, _ []string) error {
+ root, cfg, err := loadPositioning()
+ if err != nil {
+ return err
+ }
+ rep, err := positioning.Check(root, cfg)
+ if err != nil {
+ return &exitError{Code: 2, Msg: "abcd identity: " + scrubPaths(err)}
+ }
+ // A status render, not a gate: drift here exits 0 and says so. The
+ // gate is `abcd audit`, which carries the tri-state exit code.
+ return render(cmd.OutOrStdout(), *asJSON, rep, func(w io.Writer) {
+ renderIdentity(w, rep)
+ })
+ },
+ }
+
+ identityCmd.AddCommand(&cobra.Command{
+ Use: "render",
+ Short: "Print the proposed correction for every drifted surface as a unified diff (writes nothing)",
+ Args: cobra.NoArgs,
+ RunE: func(cmd *cobra.Command, _ []string) error {
+ root, cfg, err := loadPositioning()
+ if err != nil {
+ return err
+ }
+ prop, err := positioning.Propose(root, cfg)
+ if err != nil {
+ return &exitError{Code: 2, Msg: "abcd identity render: " + scrubPaths(err)}
+ }
+ return render(cmd.OutOrStdout(), *asJSON, prop, func(w io.Writer) {
+ renderProposal(w, prop)
+ })
+ },
+ })
+
+ var title, tagline, pitch, file, heading string
+ initCmd := &cobra.Command{
+ Use: "init",
+ Short: "Record this repo's identity block and the pointer to it (adopts an existing block)",
+ Args: cobra.NoArgs,
+ RunE: func(cmd *cobra.Command, _ []string) error {
+ cwd, err := os.Getwd()
+ if err != nil {
+ return err
+ }
+ res, err := positioning.Init(cwd, positioning.InitRequest{
+ Title: title, Tagline: tagline, Pitch: pitch,
+ Location: positioning.BlockLocation{File: file, Heading: heading},
+ })
+ if err != nil {
+ return &exitError{Code: 1, Msg: "abcd identity init: " + scrubPaths(err)}
+ }
+ return render(cmd.OutOrStdout(), *asJSON, res, func(w io.Writer) {
+ renderIdentityInit(w, res)
+ })
+ },
+ }
+ initCmd.Flags().StringVar(&title, "title", "", "the project's title (required unless a block already exists)")
+ initCmd.Flags().StringVar(&tagline, "tagline", "", "the project's one-line tagline (required unless a block already exists)")
+ initCmd.Flags().StringVar(&pitch, "pitch", "", "the project's short elevator pitch (optional)")
+ initCmd.Flags().StringVar(&file, "file", "", "repo-relative file the identity block lives in (default "+positioning.DefaultBlockLocation.File+")")
+ initCmd.Flags().StringVar(&heading, "heading", "", "heading the identity block sits under (default \""+positioning.DefaultBlockLocation.Heading+"\")")
+ identityCmd.AddCommand(initCmd)
+
+ return identityCmd
+}
+
+// loadPositioning resolves the working directory's registry. A repo that has not
+// adopted the check is told how to, rather than shown an empty report that reads
+// like a clean bill of health.
+func loadPositioning() (string, positioning.Config, error) {
+ cwd, err := os.Getwd()
+ if err != nil {
+ return "", positioning.Config{}, err
+ }
+ cfg, ok, err := positioning.LoadConfig(cwd)
+ if err != nil {
+ return "", positioning.Config{}, &exitError{Code: 2, Msg: "abcd identity: " + scrubPaths(err)}
+ }
+ if !ok {
+ return "", positioning.Config{}, &exitError{Code: 2, Msg: "abcd identity: this repo records no identity block (" +
+ positioning.ConfigRelPath + " is absent) — run `abcd identity init` to record one"}
+ }
+ return cwd, cfg, nil
+}
+
+// identityGlyph marks a surface's verdict in the human render.
+func identityGlyph(s positioning.Status) string {
+ switch s {
+ case positioning.StatusOK:
+ return "✓"
+ case positioning.StatusDrifted, positioning.StatusUnlocatable:
+ return "⚠"
+ default:
+ return "•"
+ }
+}
+
+// renderIdentity prints the canonical block and one line per surface. The block
+// and every surface string is repo content, so it passes through the canonical
+// terminal sanitiser.
+func renderIdentity(w io.Writer, rep positioning.Report) {
+ fmt.Fprintf(w, "abcd identity — %s (%s)\n", termsafe.Sanitize(rep.Block.File), rep.Severity)
+ fmt.Fprintf(w, " title: %s\n", termsafe.Sanitize(rep.Block.Title))
+ fmt.Fprintf(w, " tagline: %s\n", termsafe.Sanitize(rep.Block.Tagline))
+ if rep.Block.Pitch != "" {
+ fmt.Fprintf(w, " pitch: %s\n", termsafe.Sanitize(rep.Block.Pitch))
+ }
+ for _, s := range rep.Surfaces {
+ loc := termsafe.Sanitize(s.File)
+ if loc == "" {
+ loc = "(absent)"
+ } else if s.Line > 0 {
+ loc = fmt.Sprintf("%s:%d", loc, s.Line)
+ }
+ fmt.Fprintf(w, "%s [%s] %s — %s\n", identityGlyph(s.Status), termsafe.Sanitize(s.ID), loc, s.Status)
+ if s.Status == positioning.StatusDrifted {
+ fmt.Fprintf(w, " says: %s\n", termsafe.Sanitize(s.Found))
+ fmt.Fprintf(w, " canonical: %s\n", termsafe.Sanitize(s.Canonical))
+ }
+ }
+ fmt.Fprintf(w, "abcd identity — %d surface(s) adrift of the block\n", rep.Drifted())
+}
+
+// renderProposal prints the unified diffs. The header states plainly that this
+// is a proposal: adopting it is the human's move, and abcd never makes it.
+func renderProposal(w io.Writer, p positioning.Proposal) {
+ if len(p.Diffs) == 0 {
+ fmt.Fprintf(w, "abcd identity render — nothing to propose (%d surface(s) adrift, none with a replaceable region)\n", p.Report.Drifted())
+ return
+ }
+ fmt.Fprintf(w, "abcd identity render — %d proposed change(s), nothing written. Adopt what you agree with.\n\n", len(p.Diffs))
+ for _, d := range p.Diffs {
+ // The diff body is the repo's own file content, so it is sanitised
+ // before it reaches a terminal.
+ fmt.Fprint(w, termsafe.Sanitize(d.Unified))
+ fmt.Fprintln(w)
+ }
+}
+
+// renderIdentityInit prints what onboarding did.
+func renderIdentityInit(w io.Writer, res positioning.InitResult) {
+ switch {
+ case res.NoOp:
+ fmt.Fprintf(w, "abcd identity init — already recorded at %s (%q); nothing written\n",
+ termsafe.Sanitize(res.Location.File), termsafe.Sanitize(res.Location.Heading))
+ case res.AdoptedExisting:
+ fmt.Fprintf(w, "abcd identity init — adopted the existing block at %s (%q)\n",
+ termsafe.Sanitize(res.Location.File), termsafe.Sanitize(res.Location.Heading))
+ default:
+ fmt.Fprintf(w, "abcd identity init — recorded the identity block at %s (%q)\n",
+ termsafe.Sanitize(res.Location.File), termsafe.Sanitize(res.Location.Heading))
+ }
+ fmt.Fprintf(w, " title: %s\n", termsafe.Sanitize(res.Block.Title))
+ fmt.Fprintf(w, " tagline: %s\n", termsafe.Sanitize(res.Block.Tagline))
+ if res.Block.Pitch != "" {
+ fmt.Fprintf(w, " pitch: %s\n", termsafe.Sanitize(res.Block.Pitch))
+ }
+ for _, p := range res.Wrote {
+ fmt.Fprintf(w, " wrote: %s\n", p)
+ }
+ fmt.Fprintln(w, " `abcd audit` now holds every registered surface to this block.")
+}
diff --git a/internal/surface/cli/identity_surface_test.go b/internal/surface/cli/identity_surface_test.go
new file mode 100644
index 0000000..7bf93b8
--- /dev/null
+++ b/internal/surface/cli/identity_surface_test.go
@@ -0,0 +1,361 @@
+package cli
+
+import (
+ "bytes"
+ "encoding/json"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "github.com/REPPL/abcd-cli/internal/core/positioning"
+)
+
+const idBrief = `## Identity (canonical)
+
+- **Title:** abcd — Agent-Based Configuration for Development
+- **Tagline:** A host-agnostic configuration layer for intent-driven development.
+- **Pitch:** A single Go binary that carries the why from idea to shipped
+ reality, usable as a plugin in compatible agent harnesses.
+`
+
+const idConfig = `{
+ "schema_version": 1,
+ "block": {"file": ".abcd/development/brief.md", "heading": "Identity (canonical)"},
+ "surfaces": [
+ {"id": "readme-strapline", "files": ["README.md"], "kind": "regexp",
+ "patterns": ["(?s)(.*?)
"], "requires": ["tagline"], "template": "{tagline}"}
+ ]
+}`
+
+const idCleanREADME = "\n\n
abcd
\n\n
A host-agnostic configuration layer for intent-driven development.
\n\n
\n\n
abcd
\n\n
An opinionated, intent-driven development framework for product thinkers.
\n\n
An opinionated, intent-driven development framework for product thinkers.
",
+ "+ A host-agnostic configuration layer for intent-driven development.
",
+ } {
+ if !strings.Contains(out, want) {
+ t.Errorf("render output missing %q:\n%s", want, out)
+ }
+ }
+}
+
+func TestIdentityRenderOnACleanRepoProposesNothing(t *testing.T) {
+ t.Chdir(adoptedRepo(t, idCleanREADME))
+
+ var stdout, stderr bytes.Buffer
+ if code := Run([]string{"identity", "render"}, &stdout, &stderr); code != 0 {
+ t.Fatalf("exit = %d, stderr = %s", code, stderr.String())
+ }
+ if strings.Contains(stdout.String(), "--- a/") {
+ t.Fatalf("clean repo produced a diff:\n%s", stdout.String())
+ }
+}
+
+// A repo that has not adopted the check is told how to, not crashed at.
+func TestIdentityWithoutARegistryExplainsHowToAdopt(t *testing.T) {
+ t.Chdir(t.TempDir())
+
+ var stdout, stderr bytes.Buffer
+ code := Run([]string{"identity"}, &stdout, &stderr)
+ if code == 0 {
+ t.Fatalf("un-adopted repo exited 0:\n%s", stdout.String())
+ }
+ combined := stdout.String() + stderr.String()
+ if !strings.Contains(combined, "identity init") {
+ t.Errorf("no pointer to `abcd identity init`:\n%s", combined)
+ }
+}
+
+// AC1: init lands a parseable block and the pointer that records its location.
+func TestIdentityInitScaffoldsTheBlockAndThePointer(t *testing.T) {
+ repo := idRepo(t, map[string]string{"README.md": idCleanREADME})
+ t.Chdir(repo)
+
+ var stdout, stderr bytes.Buffer
+ code := Run([]string{"identity", "init",
+ "--title", "abcd — Agent-Based Configuration for Development",
+ "--tagline", "A host-agnostic configuration layer for intent-driven development.",
+ "--pitch", "A single Go binary that carries the why from idea to shipped reality."},
+ &stdout, &stderr)
+ if code != 0 {
+ t.Fatalf("exit = %d, stderr = %s", code, stderr.String())
+ }
+
+ cfg, ok, err := positioning.LoadConfig(repo)
+ if err != nil || !ok {
+ t.Fatalf("LoadConfig after init = %v, %v", ok, err)
+ }
+ block, err := positioning.ParseBlock(repo, cfg.Block)
+ if err != nil {
+ t.Fatalf("the scaffolded block does not parse: %v", err)
+ }
+ if block.Tagline != "A host-agnostic configuration layer for intent-driven development." {
+ t.Errorf("Tagline = %q", block.Tagline)
+ }
+ if block.Title == "" || block.Pitch == "" {
+ t.Errorf("block = %+v", block)
+ }
+ // The check the init just enabled must now run, and find no drift: the
+ // repo's README already carries the tagline it was interviewed for.
+ var auditOut, auditErr bytes.Buffer
+ Run([]string{"audit", "--json"}, &auditOut, &auditErr)
+ var res struct {
+ Findings []struct {
+ RuleID string `json:"ruleId"`
+ Message string `json:"message"`
+ } `json:"findings"`
+ }
+ if err := json.Unmarshal(auditOut.Bytes(), &res); err != nil {
+ t.Fatalf("audit --json: %v\n%s", err, auditOut.String())
+ }
+ for _, f := range res.Findings {
+ if f.RuleID == "identity-positioning" {
+ t.Errorf("audit after init reports positioning drift: %s", f.Message)
+ }
+ }
+}
+
+// The pitch is skippable at onboarding; title and tagline are not.
+func TestIdentityInitRequiresTitleAndTagline(t *testing.T) {
+ t.Chdir(idRepo(t, map[string]string{"README.md": idCleanREADME}))
+ for _, args := range [][]string{
+ {"identity", "init", "--tagline", "t"},
+ {"identity", "init", "--title", "T"},
+ {"identity", "init"},
+ } {
+ var stdout, stderr bytes.Buffer
+ if code := Run(args, &stdout, &stderr); code == 0 {
+ t.Errorf("%v was accepted without both required answers", args)
+ }
+ }
+}
+
+func TestIdentityInitAcceptsNoPitch(t *testing.T) {
+ repo := idRepo(t, map[string]string{"README.md": idCleanREADME})
+ t.Chdir(repo)
+
+ var stdout, stderr bytes.Buffer
+ if code := Run([]string{"identity", "init", "--title", "T", "--tagline", "A host-agnostic configuration layer for intent-driven development."}, &stdout, &stderr); code != 0 {
+ t.Fatalf("exit = %d, stderr = %s", code, stderr.String())
+ }
+ cfg, _, err := positioning.LoadConfig(repo)
+ if err != nil {
+ t.Fatal(err)
+ }
+ block, err := positioning.ParseBlock(repo, cfg.Block)
+ if err != nil {
+ t.Fatalf("block without a pitch does not parse: %v", err)
+ }
+ if block.Pitch != "" {
+ t.Errorf("Pitch = %q, want empty", block.Pitch)
+ }
+}
+
+// Onboarding detects an existing block and registers it rather than
+// re-interviewing and minting a second canon.
+func TestIdentityInitDetectsAnExistingBlockInsteadOfReinterviewing(t *testing.T) {
+ repo := idRepo(t, map[string]string{
+ ".abcd/development/brief.md": idBrief,
+ "README.md": idCleanREADME,
+ })
+ t.Chdir(repo)
+
+ var stdout, stderr bytes.Buffer
+ code := Run([]string{"identity", "init",
+ "--file", ".abcd/development/brief.md", "--heading", "Identity (canonical)"}, &stdout, &stderr)
+ if code != 0 {
+ t.Fatalf("exit = %d, stderr = %s", code, stderr.String())
+ }
+ if !strings.Contains(stdout.String(), "existing") {
+ t.Errorf("init did not report that it adopted the existing block:\n%s", stdout.String())
+ }
+ // The block file is untouched — no second canon, no rewrite.
+ data, err := os.ReadFile(filepath.Join(repo, ".abcd/development/brief.md"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if string(data) != idBrief {
+ t.Fatal("init rewrote an existing identity block")
+ }
+ cfg, ok, err := positioning.LoadConfig(repo)
+ if err != nil || !ok || cfg.Block.Heading != "Identity (canonical)" {
+ t.Fatalf("pointer not registered: %+v %v %v", cfg, ok, err)
+ }
+}
+
+// Re-running init over a registered block is a no-op, not a clobber.
+func TestIdentityInitIsIdempotent(t *testing.T) {
+ repo := adoptedRepo(t, idCleanREADME)
+ t.Chdir(repo)
+ before := snapshotTree(t, repo)
+
+ var stdout, stderr bytes.Buffer
+ if code := Run([]string{"identity", "init"}, &stdout, &stderr); code != 0 {
+ t.Fatalf("exit = %d, stderr = %s", code, stderr.String())
+ }
+ for rel, was := range snapshotTree(t, repo) {
+ if before[rel] != was {
+ t.Errorf("re-running init modified %s", rel)
+ }
+ }
+}
+
+// A repo whose registry already points somewhere must never be silently
+// repointed by an init with different answers.
+func TestIdentityInitRefusesToRepointAnAdoptedRegistry(t *testing.T) {
+ repo := adoptedRepo(t, idCleanREADME)
+ t.Chdir(repo)
+
+ var stdout, stderr bytes.Buffer
+ code := Run([]string{"identity", "init", "--title", "Other", "--tagline", "Different."}, &stdout, &stderr)
+ if code == 0 {
+ t.Fatal("init overwrote an adopted registry")
+ }
+ cfg, _, err := positioning.LoadConfig(repo)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if cfg.Block.File != ".abcd/development/brief.md" {
+ t.Fatalf("registry was repointed to %q", cfg.Block.File)
+ }
+}
From 057956c5679f7ad12467d598eabd5deee89fc99f Mon Sep 17 00:00:00 2001
From: REPPL <77722411+REPPL@users.noreply.github.com>
Date: Tue, 28 Jul 2026 03:07:01 +0100
Subject: [PATCH 07/14] fix: keep line structure when sanitising a rendered
diff
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
`abcd identity render` passed its whole unified diff through the
single-line sanitiser, which masks newlines so an injected line break
cannot forge report lines. On a diff that is the wrong trade: every
newline became a '?' and the patch arrived as one unreadable smear that
no tool could apply.
termsafe gains SanitizeBlock — the same masking, applied per line with
the newlines preserved — and the render path uses it. Bare carriage
returns are still masked, so a line cannot be overwritten in place. The
surface test now asserts the markers open their own lines.
Assisted-by: Claude:claude-opus-5
---
internal/surface/cli/identity.go | 5 +++--
internal/surface/cli/identity_surface_test.go | 16 ++++++++++++++++
internal/termsafe/termsafe.go | 15 +++++++++++++++
internal/termsafe/termsafe_test.go | 15 +++++++++++++++
4 files changed, 49 insertions(+), 2 deletions(-)
diff --git a/internal/surface/cli/identity.go b/internal/surface/cli/identity.go
index b935b8c..58f96c1 100644
--- a/internal/surface/cli/identity.go
+++ b/internal/surface/cli/identity.go
@@ -164,8 +164,9 @@ func renderProposal(w io.Writer, p positioning.Proposal) {
fmt.Fprintf(w, "abcd identity render — %d proposed change(s), nothing written. Adopt what you agree with.\n\n", len(p.Diffs))
for _, d := range p.Diffs {
// The diff body is the repo's own file content, so it is sanitised
- // before it reaches a terminal.
- fmt.Fprint(w, termsafe.Sanitize(d.Unified))
+ // before it reaches a terminal — per line, because a diff IS lines and
+ // masking its newlines would smear it into one unreadable string.
+ fmt.Fprint(w, termsafe.SanitizeBlock(d.Unified))
fmt.Fprintln(w)
}
}
diff --git a/internal/surface/cli/identity_surface_test.go b/internal/surface/cli/identity_surface_test.go
index 7bf93b8..c26099a 100644
--- a/internal/surface/cli/identity_surface_test.go
+++ b/internal/surface/cli/identity_surface_test.go
@@ -177,6 +177,22 @@ func TestIdentityRenderPrintsTheProposedDiff(t *testing.T) {
t.Errorf("render output missing %q:\n%s", want, out)
}
}
+ // A diff is line-oriented: each marker must open its own line, or the
+ // output is one unreadable smear and cannot be piped to `git apply`.
+ for _, want := range []string{
+ "\n--- a/README.md\n",
+ "\n+++ b/README.md\n",
+ "\n- An opinionated",
+ "\n+
A host-agnostic",
+ } {
+ if !strings.Contains(out, want) {
+ t.Errorf("render output is not line-oriented, missing %q:\n%q", want, out)
+ }
+ }
+ // Sanitising must not turn the diff's newlines into replacement glyphs.
+ if strings.Contains(out, "README.md?") {
+ t.Errorf("render output flattened its newlines:\n%q", out)
+ }
}
func TestIdentityRenderOnACleanRepoProposesNothing(t *testing.T) {
diff --git a/internal/termsafe/termsafe.go b/internal/termsafe/termsafe.go
index 3b48641..f4c1863 100644
--- a/internal/termsafe/termsafe.go
+++ b/internal/termsafe/termsafe.go
@@ -37,6 +37,21 @@ func Sanitize(s string) string {
}, s)
}
+// SanitizeBlock sanitises multi-line text while keeping its line structure: each
+// line is sanitised on its own and the newlines are preserved. Sanitize masks a
+// newline (an injected line break must not forge extra report lines), which is
+// right for a value interpolated into one line and wrong for output that IS
+// lines — a rendered diff or a quoted file excerpt, where flattening the
+// newlines destroys the artefact the reader needs. Use this only where the line
+// breaks are the render's own, never for a single untrusted value.
+func SanitizeBlock(s string) string {
+ lines := strings.Split(s, "\n")
+ for i, l := range lines {
+ lines[i] = Sanitize(strings.TrimSuffix(l, "\r"))
+ }
+ return strings.Join(lines, "\n")
+}
+
// SanitizeAll sanitises every member of a slice, returning a new slice.
func SanitizeAll(in []string) []string {
if in == nil {
diff --git a/internal/termsafe/termsafe_test.go b/internal/termsafe/termsafe_test.go
index 65f039f..cbc2b20 100644
--- a/internal/termsafe/termsafe_test.go
+++ b/internal/termsafe/termsafe_test.go
@@ -10,6 +10,21 @@ func withRune(prefix string, r rune, suffix string) string {
return prefix + string(r) + suffix
}
+func TestSanitizeBlockKeepsLinesAndStillMasksAttackRunes(t *testing.T) {
+ in := "--- a/README.md\n" + withRune("+ strapline", 0x1b, "[31m") + "\n cONTEXT\n"
+ got := SanitizeBlock(in)
+ if want := "--- a/README.md\n+ strapline?[31m\n cONTEXT\n"; got != want {
+ t.Errorf("SanitizeBlock = %q, want %q", got, want)
+ }
+ // A carriage return would still let a line be overwritten in place.
+ if got := SanitizeBlock("a\r\nb"); got != "a\nb" {
+ t.Errorf("SanitizeBlock(CRLF) = %q, want %q", got, "a\nb")
+ }
+ if got := SanitizeBlock(withRune("a", 0x0d, "b")); got != "a?b" {
+ t.Errorf("SanitizeBlock(bare CR) = %q, want %q", got, "a?b")
+ }
+}
+
// TestSanitizeNeutralisesDisplayAttacks proves each terminal-display attack class
// is masked while ordinary text and tab survive.
func TestSanitizeNeutralisesDisplayAttacks(t *testing.T) {
From dff251090b306ec150892cb016c17e4953e776e4 Mon Sep 17 00:00:00 2001
From: REPPL <77722411+REPPL@users.noreply.github.com>
Date: Tue, 28 Jul 2026 03:10:09 +0100
Subject: [PATCH 08/14] docs: point abcd-cli at its own identity block and
surface the verb
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
.abcd/positioning.json registers abcd's canon — the brief product
chapter's "Identity (canonical)" section, unchanged — and the three
surfaces that render from it: the README strapline, the plugin manifest
description, and the AGENTS.md opening. All three come out clean.
Adds the /abcd:identity plugin command (bare / render / init, with the
interview wording and the detect-before-asking rule), its brief surface
page and registry row, the identity-positioning row in the audit rule
table, the prepare flow's identity step, the regenerated CLI reference
and surface snapshot, and a CHANGELOG entry.
Assisted-by: Claude:claude-opus-5
---
.../development/brief/04-surfaces/16-audit.md | 1 +
.../brief/04-surfaces/17-identity.md | 85 +++++++++++++++++++
.abcd/development/brief/04-surfaces/README.md | 3 +-
.abcd/positioning.json | 34 ++++++++
.abcd/work/DECISIONS.md | 3 +
CHANGELOG.md | 26 ++++++
commands/abcd/identity.md | 85 +++++++++++++++++++
commands/abcd/prepare-this-repo.md | 50 ++++++++++-
8 files changed, 284 insertions(+), 3 deletions(-)
create mode 100644 .abcd/development/brief/04-surfaces/17-identity.md
create mode 100644 .abcd/positioning.json
create mode 100644 commands/abcd/identity.md
diff --git a/.abcd/development/brief/04-surfaces/16-audit.md b/.abcd/development/brief/04-surfaces/16-audit.md
index f50f808..48e7d17 100644
--- a/.abcd/development/brief/04-surfaces/16-audit.md
+++ b/.abcd/development/brief/04-surfaces/16-audit.md
@@ -37,6 +37,7 @@ error — so `abcd audit` gates a repo's CI as well as backing onboarding.
| `decision-durability` | warn | a committed `.abcd/work/DECISIONS.md`; decisions not living only in the gitignored layer |
| `docs-currency` | warn | reuses the docs-lint engine where `docs/` exists |
| `privacy-hygiene` | error | no absolute local paths in committed files, honouring an `abcd-audit:allow` line waiver |
+| `identity-positioning` | warn | every registered surface still carries the canonical identity block's tagline (and pitch, where required); Where-gated on a committed `.abcd/positioning.json`, and per-repo upgradeable to `error` — see [`17-identity.md`](17-identity.md) |
## How it is built
diff --git a/.abcd/development/brief/04-surfaces/17-identity.md b/.abcd/development/brief/04-surfaces/17-identity.md
new file mode 100644
index 0000000..e345529
--- /dev/null
+++ b/.abcd/development/brief/04-surfaces/17-identity.md
@@ -0,0 +1,85 @@
+# `/abcd:identity` — Repo Positioning
+
+`/abcd:identity` holds every rendered surface of a repository to one canonical
+identity block. The bare and `render` forms are **strictly read-only**; `init` is
+the single write path, and it runs once, at onboarding.
+
+It answers a different question from `/abcd:audit`: `audit` reports whether the
+repo conforms to the working conventions as a whole, and runs the positioning
+check as one rule among them; `identity` is where a maintainer looks at the canon
+itself and at what a fix would read like.
+
+## The identity block
+
+The canonical home is a markdown block in the repo's own record — markdown stays
+the single source of truth, and the committed configuration records only where
+the block lives:
+
+```markdown
+## Identity (canonical)
+
+- **Title:** abcd — Agent-Based Configuration for Development
+- **Tagline:** A host-agnostic configuration layer for intent-driven development.
+- **Pitch:** A single Go binary that carries the why from idea to shipped
+ reality, usable as a plugin in compatible agent harnesses.
+```
+
+Title and tagline are required; the pitch is optional at onboarding and may wrap
+across lines. abcd's own block is the "Identity (canonical)" section of
+[`01-product/README.md`](../01-product/README.md).
+
+## The registry
+
+`.abcd/positioning.json` records the block's location, the family severity, and
+the registered surfaces. It sits beside the repo's other per-concern
+configuration (`docs-lint.json`, `record-lint.json`, `rules.json`) rather than
+under `.abcd/config/`, where `identity.json` already means the git
+commit-author pin.
+
+| field | meaning |
+|---|---|
+| `block` | `{file, heading}` — where the canonical block lives |
+| `severity` | `warn` (default: highlight, never gate) or `blocker` |
+| `surfaces[]` | `{id, files, kind, patterns \| field, requires, template}` |
+
+A surface names candidate `files` (the first that exists is checked, so one entry
+covers several manifest formats), a locator (`kind: "regexp"` with capture-group
+patterns, or `kind: "json_field"` with a top-level key), the block fields it
+`requires`, and the `template` a proposal renders from. An empty `surfaces` list
+means the three defaults; a non-empty one replaces them, so nothing is ever
+registered silently.
+
+## Behaviour
+
+```bash
+abcd identity --json # the block and every surface's verdict; exit 0
+abcd identity render # a unified diff per drifted surface; writes nothing
+abcd identity init … # record the block and the pointer to it
+```
+
+Comparison is by normalised containment: markup, dashes, line wrapping, and case
+are folded away, so a tagline bolded mid-sentence or wrapped across two lines is
+not drift, while a reworded one is. A drifted surface reports the file, the line,
+the exact text it says, and the canonical line it should carry.
+
+**Autonomous rewriting is permanently out of scope.** `render` proposes; the
+maintainer adopts. A deliberate change of positioning is an edit to the block,
+after which the same proposal flow chases the surfaces.
+
+`init` never re-interviews a repo that already has a block — it adopts it —
+and refuses to repoint a registry that is already committed.
+
+## The check
+
+The drift check runs as the `identity-positioning` rule on every `abcd audit`,
+Where-gated on a committed registry so an un-adopted repo is skipped rather than
+failed. Its acceptance corpus is [`iss-143`](../../../work/issues/resolved/iss-143-tagline-three-variant-drift.md),
+the recorded three-variant tagline drift this check exists to catch.
+
+## References
+
+- Plugin command: [`commands/abcd/identity.md`](../../../../commands/abcd/identity.md)
+- Spec: [`spc-19`](../../specs/open/spc-19-your-repo-says-the-same-thing-about-itself-everywhere-becaus.md)
+- Intent: [`itd-102`](../../intents/planned/itd-102-your-repo-says-the-same-thing-about-itself-everywhere-becaus.md)
+- Onboarding consumer: [`15-prepare-this-repo.md`](15-prepare-this-repo.md)
+- Conformance surface: [`16-audit.md`](16-audit.md)
diff --git a/.abcd/development/brief/04-surfaces/README.md b/.abcd/development/brief/04-surfaces/README.md
index 4d6f353..ed08c91 100644
--- a/.abcd/development/brief/04-surfaces/README.md
+++ b/.abcd/development/brief/04-surfaces/README.md
@@ -19,7 +19,8 @@ The brief's user-facing command surface is the set enumerated below (not all are
| 13 | `/abcd:consult` | shipped | Consult the local sources corpus (`~/.abcd/sources`) and record source→decision provenance — host-delegated command, no Go verb | [`13-consult.md`](13-consult.md) |
| 14 | `/abcd:ingest` | shipped | Register a URL or document into the local sources corpus — host-delegated command, no Go verb | [`14-ingest.md`](14-ingest.md) |
| 15 | `/abcd:prepare-this-repo` | shipped | Bring an owned repo up to abcd's conventions (interim bridge) — host-delegated command, no Go verb | [`15-prepare-this-repo.md`](15-prepare-this-repo.md) |
-| 16 | `/abcd:audit` | shipped | Check whether a repo conforms to the working conventions (three-tier layout, AGENTS.md router, durable decisions, docs currency, privacy hygiene) — read-only, tri-state exit; backs `prepare-this-repo` and gates CI (itd-85) | [`16-audit.md`](16-audit.md) |
+| 16 | `/abcd:audit` | shipped | Check whether a repo conforms to the working conventions (three-tier layout, AGENTS.md router, durable decisions, docs currency, privacy hygiene, identity positioning) — read-only, tri-state exit; backs `prepare-this-repo` and gates CI (itd-85) | [`16-audit.md`](16-audit.md) |
+| 17 | `/abcd:identity` | shipped | Repo positioning: the canonical identity block (title, tagline, pitch) and every rendered surface held to it. Bare renders the block and each surface's verdict; `render` prints the proposed correction as a unified diff and writes nothing (autonomous rewriting is permanently out of scope); `init` records the block and the `.abcd/positioning.json` pointer at onboarding, adopting an existing block rather than re-interviewing. The drift check itself runs as the `identity-positioning` rule on every audit, warn-tier and per-repo upgradeable to blocker (itd-102, spc-19) | [`17-identity.md`](17-identity.md) |
The **Status** column is machine-checked: the `surface_coverage` record-lint rule asserts every `shipped` row has a backing surface (`commands/abcd/.md` or `skills//`) and every `staged` row (a design target) has none — and, in reverse, that every real surface has a row here. The bare `/abcd` top-level is binary-backed (its command file is `commands/abcd.md`, not a `commands/abcd/.md` entry) and is exempt from the file check. Keeping this column honest is how the brief's surface set stays reconciled with the shipped binary; the semantic half — whether each row's *prose* matches binary behaviour — stays a release-gate agent check.
diff --git a/.abcd/positioning.json b/.abcd/positioning.json
new file mode 100644
index 0000000..3eee852
--- /dev/null
+++ b/.abcd/positioning.json
@@ -0,0 +1,34 @@
+{
+ "schema_version": 1,
+ "block": {
+ "file": ".abcd/development/brief/01-product/README.md",
+ "heading": "Identity (canonical)"
+ },
+ "severity": "warn",
+ "surfaces": [
+ {
+ "id": "readme-strapline",
+ "files": ["README.md"],
+ "kind": "regexp",
+ "patterns": ["(?s)(.*?)
"],
+ "requires": ["tagline"],
+ "template": "{tagline}"
+ },
+ {
+ "id": "manifest-description",
+ "files": [".claude-plugin/plugin.json"],
+ "kind": "json_field",
+ "field": "description",
+ "requires": ["tagline", "pitch"],
+ "template": "{tagline} {pitch}"
+ },
+ {
+ "id": "conventions-opening",
+ "files": ["AGENTS.md"],
+ "kind": "regexp",
+ "patterns": ["(?s)\\n+(.*?)(?:\\n\\n|\\z)"],
+ "requires": ["tagline"],
+ "template": "{tagline}"
+ }
+ ]
+}
diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md
index 008bed8..1547921 100644
--- a/.abcd/work/DECISIONS.md
+++ b/.abcd/work/DECISIONS.md
@@ -863,3 +863,6 @@ parallel-agent merge contention bites.
- 2026-07-27 — itd-93 parity tension resolved: Branch A. The scaffold template is the single source for release.yml/auto-release.yml; abcd-cli's live workflows are regenerated from it; the only sanctioned live diff is the additive workflow_dispatch rehearsal (trigger + non-publishing dry-run job). Rejected: substitution-gating the rehearsal off for abcd (weakens the parity guarantee on the one novel component).
- 2026-07-27 — GRILLED itd-101/102/103/104 to settlement (six decisions, each now in its intent's Grill Settlements + acceptance criteria): itd-103 guard fails open loudly, blocker/warn tiers with committed-only overrides, shell-token-aware command-position matching with an itd-81-style TNR-floored corpus; itd-101 refresh is manual-with-nagging (scheduled CI later, own sign-off), staleness warns at 180d and blocks releases at 365d, human verifications age on the same clock; itd-102 identity lives in a parseable markdown block (config points at it), check is warn-tier and never rewrites; itd-104 ideate is an optional verb, never a pre-capture gate, with a fresh-context off-policy adversary. All four drafts are grill-settled and await intent plan.
- 2026-07-28: itd-102 spec minted as spc-16 (fourth live iss-80 instance this run); renumbered to spc-19 by hand — this branch deliberately skips 16-18 (16 on #156, 17 on the itd-101 branch, 18 on #157).
+- 2026-07-28 — itd-102 implementation: repo positioning lives in a NEW `internal/core/positioning` package, not `internal/core/identity` — that package is the git commit-author gate (`.abcd/config/identity.json` pin, pre-commit hook) and shares nothing with repo self-description but the English word; folding them would put two unrelated concerns behind one name. The user-facing verb stays `abcd identity` (spc-19). For the same collision the registry is `.abcd/positioning.json` at the `.abcd/` top level (beside docs-lint.json / record-lint.json / rules.json), NOT `.abcd/config/identity.json`. Rejected: extending the commit-identity package; naming the verb `abcd positioning` (spc-19 fixes the verb).
+- 2026-07-28 — itd-102: surface comparison is normalised CONTAINMENT of each required block field (markup, dashes, wrapping, case folded; trailing sentence punctuation trimmed from the needle), not line equality. Equality would flag abcd's own three conforming surfaces, each of which carries the tagline in a different rendering (inside ``, concatenated with the pitch in the manifest, bolded mid-sentence and line-wrapped in AGENTS.md); containment catches all three iss-143 variants while accepting all three current ones. Consequence: `identity render` fires only on drift, and its template render is a proposal to adopt by hand, not a byte-exact reproduction of conforming prose.
+- 2026-07-28 — itd-102: no scaffold entry point existed in ahoy/prepare for a repo record block, so the verb family gained a minimal `identity init` (write path: block + pointer, atomic, adopts an existing block, refuses to repoint an adopted registry). `abcd launch scaffold` is release-machinery-specific and `ahoy install` writes only abcd's own plumbing; extending either would have widened its remit.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4130a66..6f3ea44 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,32 @@ called out in a **Breaking** section.
### Added
+- **`abcd identity` — one canonical identity block, and every surface held to
+ it** (itd-102, spc-19). A project's positioning fragments silently: the README
+ strapline, the package or plugin manifest description, and the conventions
+ file's opening are edited at different moments until three surfaces tell three
+ stories (the recorded iss-143 drift, which is this check's acceptance corpus).
+ A repo now records one canonical markdown block — `- **Title:**`,
+ `- **Tagline:**`, and an optional, wrappable `- **Pitch:**` under a recorded
+ heading — and `.abcd/positioning.json` records only where that block lives, how
+ loudly the family reports, and which surfaces render from it. Markdown stays
+ the single source of truth. The registry is data, not branches: a surface names
+ candidate files (the first present wins, so one entry covers several manifest
+ formats), a locator (a capture-group regexp or a top-level JSON field), the
+ block fields it must carry, and the template a proposal renders from; the three
+ defaults are the README strapline, the manifest description, and the
+ conventions opening, and a declared list replaces them so nothing is registered
+ silently. A new `identity-positioning` rule runs the check on **every**
+ `abcd audit`, naming the file, the exact drifted line, and the canonical line it
+ should carry — warn-tier by default (it highlights, it never gates) and
+ upgradeable per-repo to blocker. `abcd identity` renders the block and each
+ surface's verdict; `abcd identity render` prints the proposed correction as a
+ unified diff and **writes nothing** (autonomous rewriting is permanently out of
+ scope — adopting a proposal is always the maintainer's move); `abcd identity
+ init` records the block and the pointer at onboarding, adopting an existing
+ block rather than re-interviewing over it. `/abcd:prepare-this-repo` gains the
+ interview, detect-first.
+
- **`abcd launch scaffold` — the changelog-driven release-gate scaffolder**
(itd-93, spc-14). Writes the fixed release machinery into a managed repo that
lacks it: `.github/workflows/release.yml` (verify → build → publish, the verify
diff --git a/commands/abcd/identity.md b/commands/abcd/identity.md
new file mode 100644
index 0000000..b11f5d1
--- /dev/null
+++ b/commands/abcd/identity.md
@@ -0,0 +1,85 @@
+---
+name: identity
+description: Show this repo's canonical identity block — title, tagline, pitch — and every rendered surface held to it, and print the proposed correction for any that drifted, by invoking the abcd binary. The bare and render forms perform zero writes; init records the block and the pointer to it.
+argument-hint: "[render|init]"
+---
+
+# `/abcd:identity` repo positioning
+
+A project's positioning fragments silently: the README strapline, the package or
+plugin manifest description, and the conventions file's opening line are each
+edited at different moments, until three surfaces tell three stories. This
+command shows the one canonical identity block the repo records, and which
+surfaces still say it.
+
+## Bare — what this repo says about itself
+
+```bash
+abcd identity --json
+```
+
+emits `{ "block": …, "severity": …, "surfaces": [ … ] }`:
+
+- `block` — the canonical `title`, `tagline`, and optional `pitch`, plus the
+ `file` and `line` they are recorded at.
+- `severity` — the family's weight, `warn` (the default) or `blocker`.
+- `surfaces` — one entry per registered surface: its `id`, the `file` and `line`
+ checked, and a `status` of `ok`, `drifted`, `absent` (no such file in this
+ repo — not a fault), or `unlocatable` (the file is there but the locator
+ matches nothing, so drift would go unseen). A `drifted` entry carries `found`
+ (the exact text the surface says), `missing` (which block fields it no longer
+ carries), and `canonical` (what the block says it should).
+
+Report the block first, then any surface that is not `ok`, naming the file, the
+line, what it says, and what the block says. It exits `0` even when it reports
+drift: this form is a status render, and the gate is `abcd audit`.
+
+## `render` — the proposed correction
+
+```bash
+abcd identity render
+```
+
+prints a unified diff per drifted surface. It **writes nothing**, and no flag
+makes it: adopting a proposal is the maintainer's move. Show the diff, then ask
+whether to apply it. If the maintainer would rather change what the project says
+than what its surfaces say, the fix is an edit to the identity block, after which
+this same command chases the surfaces.
+
+## `init` — record the block
+
+Run this only as part of onboarding a repo, and only after the interview below.
+
+```bash
+abcd identity init --title "…" --tagline "…" [--pitch "…"] [--file ] [--heading ]
+```
+
+**Detect before you interview.** If the repo already carries an identity block,
+`init` adopts it and leaves it byte-for-byte alone — never ask the questions
+again over an existing answer, and never mint a second canon. Pass `--file` and
+`--heading` when the block already lives somewhere specific. A repo whose
+pointer is already committed is a no-op; passing answers to it is refused, because
+repointing the canon is a deliberate edit.
+
+### The interview
+
+Ask once, in the maintainer's own words:
+
+1. **Title** (required) — what the project is called, as it should read in a
+ heading.
+2. **Tagline** (required) — one line saying what it is. This is the line every
+ surface is held to.
+3. **Pitch** (optional — offer to skip) — two or three sentences a newcomer
+ could read and know whether this is for them.
+
+Then run `init` with the answers. It writes the block and the pointer
+(`.abcd/positioning.json`) that records where the block lives and which surfaces
+render from it — by default the README strapline, the plugin or package manifest
+description, and the conventions file's opening. Further surfaces are registrable
+in that same file; none are ever registered silently.
+
+If the `abcd` binary is not on `PATH`, fall back to
+`go run ./cmd/abcd identity --json` from the repo root, or tell the user to build
+it with `make build`.
+
+**User input:** $ARGUMENTS
diff --git a/commands/abcd/prepare-this-repo.md b/commands/abcd/prepare-this-repo.md
index 800fdd2..fc09192 100644
--- a/commands/abcd/prepare-this-repo.md
+++ b/commands/abcd/prepare-this-repo.md
@@ -115,11 +115,53 @@ target's `.abcd/.work.local/scratch/` (create the directory via
test the repo from `AGENTS.md` alone.
- The working-conventions section below, between markers. An existing
attribution policy in the file stays authoritative over the template.
-4. **Commit gates.** If the repo has no `.pre-commit-config.yaml`, offer the
+4. **Identity — detect first, interview only if there is nothing to detect.**
+ A project's positioning fragments silently: the README strapline, the
+ package or plugin manifest description, and the conventions file's opening
+ are edited at different moments until three surfaces tell three stories.
+ One canonical block stops that.
+
+ **Detect before asking.** Look for an identity block the repo already
+ carries — three bullets (`- **Title:**`, `- **Tagline:**`, `- **Pitch:**`)
+ under a heading such as "Identity (canonical)", typically in a brief or
+ product chapter under `.abcd/development/`. If one exists, register it and
+ ask nothing:
+
+ ```bash
+ abcd identity init --file --heading ""
+ ```
+
+ `init` adopts the existing block byte-for-byte and writes only the pointer.
+ Re-interviewing over an answer the repo already gives is how a project ends
+ up with two canons.
+
+ **Only if there is no block**, ask once, in the maintainer's own words:
+
+ - **Title** (required) — what the project is called, as it should read in a
+ heading.
+ - **Tagline** (required) — one line saying what it is. This is the line
+ every surface is held to.
+ - **Pitch** (optional — offer to skip) — two or three sentences a newcomer
+ could read and know whether this is for them.
+
+ Then record the answers:
+
+ ```bash
+ abcd identity init --title "…" --tagline "…" [--pitch "…"]
+ ```
+
+ This writes the markdown block (markdown stays the source of truth) and
+ `.abcd/positioning.json`, which records where the block lives and which
+ surfaces render from it. From then on `abcd audit` reports any surface that
+ drifts from it, and `abcd identity render` proposes the correction as a
+ diff. abcd never rewrites a surface itself — adopting a proposal is always
+ the maintainer's move.
+
+5. **Commit gates.** If the repo has no `.pre-commit-config.yaml`, offer the
secrets + absolute-path gate config (template at
`~/ABCDevelopment/.agents/templates/pre-commit-config.yaml`, if present);
activate with `pre-commit install`.
-5. **Attribution (opt-in only).** If the user says this repo requires AI
+6. **Attribution (opt-in only).** If the user says this repo requires AI
disclosure, install the `prepare-commit-msg` hook from
`~/ABCDevelopment/.agents/templates/` (if present) into `.githooks/`, set
`core.hooksPath`, and add the AI-attribution section to `AGENTS.md`.
@@ -167,5 +209,9 @@ record. Commit only content that is about this repository.
`.work/` layout was fully migrated (with sign-off) or fully left alone.
- `AGENTS.md` carries verified repo facts and the marked, nameless
working-conventions section; the done-test passes.
+- One identity block is recorded and registered — adopted where the repo
+ already had one, interviewed only where it did not — and `abcd identity`
+ reports every registered surface as `ok`, or the drift it reports was shown
+ to the maintainer with the proposed diff.
- Nothing from `private-names.txt` and no abcd-internal content appears in any
committed or published artefact.
From 0ef6de324d087bd02a6dc4a873aa10195a11ba2a Mon Sep 17 00:00:00 2001
From: REPPL <77722411+REPPL@users.noreply.github.com>
Date: Tue, 28 Jul 2026 03:14:23 +0100
Subject: [PATCH 09/14] fix: emit a unified diff that patch tools actually
accept
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The proposal rendered a hunk no tool would apply in three cases, all
found by pointing `git apply --check` at it:
- a file ending in a newline was split into N+1 lines, so the hunk
carried a phantom trailing context line that is not in the file;
- a file NOT ending in a newline got no no-newline marker, which every
patch tool requires;
- a CRLF file got an LF patch, which cannot match its bytes.
Line endings now round-trip (the diff and the proposed content both
carry CRLF back when the file uses it), the marker is emitted against
the trailing context line or against both sides of the change, and a
locator whose paragraph terminator is end-of-file no longer swallows the
file's final newline into the replaced span — which also panicked the
renderer by indexing past the last line.
The regression test drives five shapes through `git apply --check`.
Assisted-by: Claude:claude-opus-5
---
internal/core/positioning/check.go | 10 +++-
internal/core/positioning/render.go | 67 ++++++++++++++++++-----
internal/core/positioning/render_test.go | 67 +++++++++++++++++++++++
internal/core/positioning/zzprobe_test.go | 48 ++++++++++++++++
4 files changed, 176 insertions(+), 16 deletions(-)
create mode 100644 internal/core/positioning/zzprobe_test.go
diff --git a/internal/core/positioning/check.go b/internal/core/positioning/check.go
index 174bccc..31d05df 100644
--- a/internal/core/positioning/check.go
+++ b/internal/core/positioning/check.go
@@ -199,7 +199,15 @@ func (s Surface) locateSpan(text string) (span, bool, error) {
if loc == nil || loc[2] < 0 {
continue
}
- return span{start: loc[2], end: loc[3], text: text[loc[2]:loc[3]]}, true, nil
+ // A pattern whose paragraph terminator is end-of-file captures the
+ // file's final newline(s). They are not part of what the surface
+ // SAYS, and a replacement span that swallowed them would delete the
+ // file's line ending, so the span stops at the last real character.
+ start, end := loc[2], loc[3]
+ for end > start && (text[end-1] == '\n' || text[end-1] == '\r') {
+ end--
+ }
+ return span{start: start, end: end, text: text[start:end]}, true, nil
}
return span{}, false, nil
}
diff --git a/internal/core/positioning/render.go b/internal/core/positioning/render.go
index a782824..0f5b636 100644
--- a/internal/core/positioning/render.go
+++ b/internal/core/positioning/render.go
@@ -1,6 +1,7 @@
package positioning
import (
+ "bytes"
"encoding/json"
"fmt"
"strings"
@@ -94,24 +95,53 @@ func Propose(root string, cfg Config) (Proposal, error) {
replacement = string(enc[1 : len(enc)-1])
}
proposed := text[:sp.start] + replacement + text[sp.end:]
+ // The locators work over newline-normalised text, but the proposal has
+ // to match the bytes on disk or no patch tool will apply it: a file
+ // written with CRLF gets its line endings back.
+ crlf := bytes.Contains(data, []byte("\r\n"))
+ onDisk := proposed
+ if crlf {
+ onDisk = strings.ReplaceAll(proposed, "\n", "\r\n")
+ }
p.Diffs = append(p.Diffs, SurfaceDiff{
SurfaceID: res.ID,
File: res.File,
- Proposed: proposed,
- Unified: unifiedDiff(res.File, text, proposed, sp.start, sp.end, replacement),
+ Proposed: onDisk,
+ Unified: unifiedDiff(res.File, text, sp.start, sp.end, replacement, crlf),
})
}
return p, nil
}
+// noNewlineMarker is the unified-diff notation for a final line that the file
+// does not terminate with a newline. Omitting it makes every patch tool refuse.
+const noNewlineMarker = "\\ No newline at end of file\n"
+
// unifiedDiff renders the single-region replacement as a unified diff. The
// change is always one contiguous span, so the patch is exactly one hunk: the
// lines the span touches, replaced, flanked by up to diffContext unchanged
// lines. That is enough to be a real patch without pulling in a diff algorithm.
-func unifiedDiff(file, old, proposed string, start, end int, replacement string) string {
- oldLines := strings.Split(old, "\n")
- firstLine := strings.Count(old[:start], "\n")
- lastLine := strings.Count(old[:end], "\n")
+//
+// old is the newline-normalised file text and start/end are byte offsets into
+// it; crlf says the file on disk uses CRLF, in which case the emitted lines
+// carry the carriage return back so the patch matches the real bytes.
+func unifiedDiff(file, old string, start, end int, replacement string, crlf bool) string {
+ eol := "\n"
+ if crlf {
+ eol = "\r\n"
+ }
+ // A file ending in a newline has N lines, not N+1: splitting the raw text
+ // yields a phantom trailing empty line that is not in the file, and a hunk
+ // carrying it as context can never apply. Where the file really does not
+ // end in a newline, the hunk says so with the marker instead.
+ finalNewline := strings.HasSuffix(old, "\n")
+ oldLines := strings.Split(strings.TrimSuffix(old, "\n"), "\n")
+
+ // Clamped: a span that reached past the last content line (a locator that
+ // swallowed the file's trailing newline) must not index past oldLines.
+ firstLine := min(strings.Count(old[:start], "\n"), len(oldLines)-1)
+ lastLine := min(strings.Count(old[:end], "\n"), len(oldLines)-1)
+ end = min(end, lineEndOffset(old, lastLine))
// The replaced lines, rebuilt: everything on the first line before the
// span, the replacement, everything on the last line after it.
@@ -127,23 +157,30 @@ func unifiedDiff(file, old, proposed string, start, end int, replacement string)
before := oldLines[ctxStart:firstLine]
after := oldLines[lastLine+1 : ctxEnd+1]
+ // The hunk reaches the file's last line, so an absent final newline has to
+ // be declared — against the shared context line if there is one after the
+ // change, otherwise against both sides of the change itself.
+ truncated := !finalNewline && ctxEnd == len(oldLines)-1
+
var b strings.Builder
fmt.Fprintf(&b, "--- a/%s\n+++ b/%s\n", file, file)
fmt.Fprintf(&b, "@@ -%d,%d +%d,%d @@\n",
ctxStart+1, len(before)+len(removed)+len(after),
ctxStart+1, len(before)+len(added)+len(after))
for _, l := range before {
- b.WriteString(" " + l + "\n")
- }
- for _, l := range removed {
- b.WriteString("-" + l + "\n")
- }
- for _, l := range added {
- b.WriteString("+" + l + "\n")
+ b.WriteString(" " + l + eol)
}
- for _, l := range after {
- b.WriteString(" " + l + "\n")
+ writeSide := func(prefix string, lines []string, mark bool) {
+ for i, l := range lines {
+ b.WriteString(prefix + l + eol)
+ if mark && i == len(lines)-1 {
+ b.WriteString(noNewlineMarker)
+ }
+ }
}
+ writeSide("-", removed, truncated && len(after) == 0)
+ writeSide("+", added, truncated && len(after) == 0)
+ writeSide(" ", after, truncated && len(after) > 0)
return b.String()
}
diff --git a/internal/core/positioning/render_test.go b/internal/core/positioning/render_test.go
index 1d7cd8f..70ea702 100644
--- a/internal/core/positioning/render_test.go
+++ b/internal/core/positioning/render_test.go
@@ -2,9 +2,12 @@ package positioning
import (
"os"
+ "os/exec"
"path/filepath"
"strings"
"testing"
+
+ "github.com/REPPL/abcd-cli/internal/gittest"
)
// snapshot records every file's content and modification time under root.
@@ -146,6 +149,70 @@ func TestProposeSkipsUnlocatableSurfaces(t *testing.T) {
}
}
+// The proposal is only useful if it is a real patch. `git apply` is the
+// arbiter: a phantom trailing line, a wrong hunk count, or a missing
+// no-newline marker all make it refuse.
+func TestProposedDiffAppliesWithGitApply(t *testing.T) {
+ if _, err := exec.LookPath("git"); err != nil {
+ t.Skip("git not on PATH")
+ }
+ cases := map[string]map[string]string{
+ // The drifted line sits near end-of-file, so the trailing context
+ // reaches the last line.
+ "drift at end of file": {
+ "brief.md": canonBrief,
+ "README.md": "# Widget\n\nAn opinionated widget framework for busy people.
\n",
+ },
+ "no trailing newline": {
+ "brief.md": canonBrief,
+ "README.md": "# Widget\n\nAn opinionated widget framework for busy people.
",
+ },
+ "single-line manifest": {
+ "brief.md": canonBrief,
+ "plugin.json": `{"name":"w","description":"The widget, but faster."}` + "\n",
+ },
+ "crlf line endings": {
+ "brief.md": canonBrief,
+ "README.md": "# Widget\r\n\r\nAn opinionated widget framework.
\r\n\r\n## More\r\n",
+ },
+ "drift mid-file": {
+ "brief.md": canonBrief,
+ "README.md": "# Widget\n\nintro\n\nAn opinionated widget framework.
\n\nmore\n\ntail\n",
+ },
+ }
+ for name, files := range cases {
+ t.Run(name, func(t *testing.T) {
+ root := writeRepo(t, files)
+ gitInit(t, root)
+ p, err := Propose(root, abcdLikeConfig())
+ if err != nil {
+ t.Fatalf("Propose: %v", err)
+ }
+ if len(p.Diffs) == 0 {
+ t.Fatal("no diff produced")
+ }
+ patch := filepath.Join(t.TempDir(), "p.patch")
+ if err := os.WriteFile(patch, []byte(p.Unified()), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ cmd := exec.Command("git", "-C", root, "apply", "--check", patch)
+ cmd.Env = gittest.Env(t)
+ if out, err := cmd.CombinedOutput(); err != nil {
+ t.Fatalf("git apply --check refused the proposal: %v: %s\n--- patch ---\n%s", err, out, p.Unified())
+ }
+ })
+ }
+}
+
+func gitInit(t *testing.T, root string) {
+ t.Helper()
+ cmd := exec.Command("git", "-C", root, "init", "-q")
+ cmd.Env = gittest.Env(t)
+ if out, err := cmd.CombinedOutput(); err != nil {
+ t.Fatalf("git init: %v: %s", err, out)
+ }
+}
+
func TestProposedApplicationRemovesTheDrift(t *testing.T) {
root := writeRepo(t, map[string]string{
"brief.md": canonBrief, "README.md": drift143README,
diff --git a/internal/core/positioning/zzprobe_test.go b/internal/core/positioning/zzprobe_test.go
new file mode 100644
index 0000000..dad3ca4
--- /dev/null
+++ b/internal/core/positioning/zzprobe_test.go
@@ -0,0 +1,48 @@
+
+package positioning
+
+import (
+ "os"
+ "os/exec"
+ "path/filepath"
+ "testing"
+)
+
+func TestZZProbe(t *testing.T) {
+ cases := map[string]string{
+ "trailing_nl": "# Widget\n\nAn opinionated widget framework for busy people.
\n",
+ "no_trailing_nl": "# Widget\n\nAn opinionated widget framework for busy people.
",
+ "midfile": "# Widget\n\nintro\n\nAn opinionated widget framework.
\n\nmore\n\ntail\n",
+ "crlf": "# Widget\r\n\r\nAn opinionated widget framework.
\r\n\r\n## More\r\n",
+ "blank_after": "# Widget\n\nAn opinionated widget framework.
\n\n",
+ }
+ for name, readme := range cases {
+ t.Run(name, func(t *testing.T) {
+ root := writeRepo(t, map[string]string{"brief.md": canonBrief, "README.md": readme})
+ gitInit(t, root)
+ p, err := Propose(root, abcdLikeConfig())
+ if err != nil {
+ t.Fatalf("Propose: %v", err)
+ }
+ if len(p.Diffs) == 0 {
+ t.Logf("NO DIFF")
+ return
+ }
+ t.Logf("ORIG=%q", readme)
+ t.Logf("PATCH:\n%s---END---", p.Unified())
+ patch := filepath.Join(t.TempDir(), "p.patch")
+ os.WriteFile(patch, []byte(p.Unified()), 0o644)
+ out, err := exec.Command("git", "-C", root, "apply", patch).CombinedOutput()
+ if err != nil {
+ t.Errorf("git apply FAILED: %v: %s", err, out)
+ return
+ }
+ got, _ := os.ReadFile(filepath.Join(root, "README.md"))
+ t.Logf("APPLIED=%q", string(got))
+ t.Logf("PROPOSED=%q", p.Diffs[0].Proposed)
+ if string(got) != p.Diffs[0].Proposed {
+ t.Errorf("MISMATCH applied vs Proposed")
+ }
+ })
+ }
+}
From 8b67070b1f56435d2a75081706e0c95a025a543e Mon Sep 17 00:00:00 2001
From: REPPL <77722411+REPPL@users.noreply.github.com>
Date: Tue, 28 Jul 2026 03:16:34 +0100
Subject: [PATCH 10/14] test: prove the applied patch equals the proposed
content
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The diff test only asked whether `git apply` accepted the hunk. The
patch and the SurfaceDiff.Proposed field are two renderings of the same
change, so it now applies the patch for real and compares the resulting
file to Proposed byte for byte — if they ever disagree, one of them is
lying to the maintainer. A trailing-blank-line shape joins the cases.
Assisted-by: Claude:claude-opus-5
---
internal/core/positioning/render_test.go | 19 ++++++++-
internal/core/positioning/zzprobe_test.go | 48 -----------------------
2 files changed, 17 insertions(+), 50 deletions(-)
delete mode 100644 internal/core/positioning/zzprobe_test.go
diff --git a/internal/core/positioning/render_test.go b/internal/core/positioning/render_test.go
index 70ea702..b6d890c 100644
--- a/internal/core/positioning/render_test.go
+++ b/internal/core/positioning/render_test.go
@@ -179,6 +179,10 @@ func TestProposedDiffAppliesWithGitApply(t *testing.T) {
"brief.md": canonBrief,
"README.md": "# Widget\n\nintro\n\nAn opinionated widget framework.
\n\nmore\n\ntail\n",
},
+ "trailing blank line": {
+ "brief.md": canonBrief,
+ "README.md": "# Widget\n\nAn opinionated widget framework.
\n\n",
+ },
}
for name, files := range cases {
t.Run(name, func(t *testing.T) {
@@ -195,10 +199,21 @@ func TestProposedDiffAppliesWithGitApply(t *testing.T) {
if err := os.WriteFile(patch, []byte(p.Unified()), 0o644); err != nil {
t.Fatal(err)
}
- cmd := exec.Command("git", "-C", root, "apply", "--check", patch)
+ cmd := exec.Command("git", "-C", root, "apply", patch)
cmd.Env = gittest.Env(t)
if out, err := cmd.CombinedOutput(); err != nil {
- t.Fatalf("git apply --check refused the proposal: %v: %s\n--- patch ---\n%s", err, out, p.Unified())
+ t.Fatalf("git apply refused the proposal: %v: %s\n--- patch ---\n%s", err, out, p.Unified())
+ }
+ // The patch and the Proposed content are two renderings of the same
+ // change; if they disagree, one of them is lying to the maintainer.
+ for _, d := range p.Diffs {
+ got, rerr := os.ReadFile(filepath.Join(root, d.File))
+ if rerr != nil {
+ t.Fatal(rerr)
+ }
+ if string(got) != d.Proposed {
+ t.Errorf("%s after applying the patch = %q, but Proposed = %q", d.File, got, d.Proposed)
+ }
}
})
}
diff --git a/internal/core/positioning/zzprobe_test.go b/internal/core/positioning/zzprobe_test.go
deleted file mode 100644
index dad3ca4..0000000
--- a/internal/core/positioning/zzprobe_test.go
+++ /dev/null
@@ -1,48 +0,0 @@
-
-package positioning
-
-import (
- "os"
- "os/exec"
- "path/filepath"
- "testing"
-)
-
-func TestZZProbe(t *testing.T) {
- cases := map[string]string{
- "trailing_nl": "# Widget\n\nAn opinionated widget framework for busy people.
\n",
- "no_trailing_nl": "# Widget\n\nAn opinionated widget framework for busy people.
",
- "midfile": "# Widget\n\nintro\n\nAn opinionated widget framework.
\n\nmore\n\ntail\n",
- "crlf": "# Widget\r\n\r\nAn opinionated widget framework.
\r\n\r\n## More\r\n",
- "blank_after": "# Widget\n\nAn opinionated widget framework.
\n\n",
- }
- for name, readme := range cases {
- t.Run(name, func(t *testing.T) {
- root := writeRepo(t, map[string]string{"brief.md": canonBrief, "README.md": readme})
- gitInit(t, root)
- p, err := Propose(root, abcdLikeConfig())
- if err != nil {
- t.Fatalf("Propose: %v", err)
- }
- if len(p.Diffs) == 0 {
- t.Logf("NO DIFF")
- return
- }
- t.Logf("ORIG=%q", readme)
- t.Logf("PATCH:\n%s---END---", p.Unified())
- patch := filepath.Join(t.TempDir(), "p.patch")
- os.WriteFile(patch, []byte(p.Unified()), 0o644)
- out, err := exec.Command("git", "-C", root, "apply", patch).CombinedOutput()
- if err != nil {
- t.Errorf("git apply FAILED: %v: %s", err, out)
- return
- }
- got, _ := os.ReadFile(filepath.Join(root, "README.md"))
- t.Logf("APPLIED=%q", string(got))
- t.Logf("PROPOSED=%q", p.Diffs[0].Proposed)
- if string(got) != p.Diffs[0].Proposed {
- t.Errorf("MISMATCH applied vs Proposed")
- }
- })
- }
-}
From 857cecbbc44464a6f05555d5e75499535a59866c Mon Sep 17 00:00:00 2001
From: REPPL <77722411+REPPL@users.noreply.github.com>
Date: Tue, 28 Jul 2026 03:25:34 +0100
Subject: [PATCH 11/14] fix: keep CRLF on the human render path, and refuse an
incomplete block
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Two review findings, both reproduced before the fix.
SanitizeBlock dropped the carriage return from every line, so the core's
CRLF round-trip was voided at the front door: `abcd identity render`
printed an LF patch that no tool would apply to a CRLF surface, while
the same repo's --json output carried a correct one. The CR of a CRLF
pair is committed by the newline that follows it and can overprint
nothing, so it now survives; every bare carriage return — including one
inside a run before a newline, and one trailing the final line — is
still masked. Asserted at the CLI boundary, which is where the guarantee
has to hold; the core test could not see this.
`identity init` routed a heading with incomplete bullets into the
scaffold branch, but writeBlock only appends: the append left a second
copy of the heading, the parser still read the broken first one, Init
failed, no registry was written, and every retry appended again. Only an
absent file or heading is fixable by appending, so an incomplete block
is now refused before any write, naming the file and the missing bullet.
Also backs the finding-message truncation off to a rune boundary: the
byte cut split a multi-byte rune and %q rendered the remains as byte
escapes in the report.
Assisted-by: Claude:claude-opus-5
---
internal/core/audit/rule_positioning.go | 11 ++-
internal/core/audit/rule_positioning_test.go | 26 +++++++
internal/core/positioning/init.go | 29 ++++++--
internal/surface/cli/identity_surface_test.go | 74 +++++++++++++++++++
internal/termsafe/termsafe.go | 30 ++++++--
internal/termsafe/termsafe_test.go | 19 ++++-
6 files changed, 169 insertions(+), 20 deletions(-)
diff --git a/internal/core/audit/rule_positioning.go b/internal/core/audit/rule_positioning.go
index a2cd4cc..687c673 100644
--- a/internal/core/audit/rule_positioning.go
+++ b/internal/core/audit/rule_positioning.go
@@ -4,6 +4,7 @@ import (
"fmt"
"path/filepath"
"strings"
+ "unicode/utf8"
"github.com/REPPL/abcd-cli/internal/core/positioning"
"github.com/REPPL/abcd-cli/internal/fsutil"
@@ -111,11 +112,17 @@ func severityFor(cfg positioning.Config) Severity {
}
// truncate shortens quoted surface text for a one-line finding, collapsing the
-// wrapping so a paragraph reads as the line it drifted on.
+// wrapping so a paragraph reads as the line it drifted on. The cut backs off to
+// a rune boundary: slicing bytes mid-rune would put a replacement glyph in the
+// message, and the surface text is arbitrary UTF-8 from the audited repo.
func truncate(s string) string {
s = strings.Join(strings.Fields(s), " ")
if len(s) <= findingMaxLen {
return s
}
- return s[:findingMaxLen] + "…"
+ cut := findingMaxLen
+ for cut > 0 && !utf8.RuneStart(s[cut]) {
+ cut--
+ }
+ return s[:cut] + "…"
}
diff --git a/internal/core/audit/rule_positioning_test.go b/internal/core/audit/rule_positioning_test.go
index 75cfcfc..953ac40 100644
--- a/internal/core/audit/rule_positioning_test.go
+++ b/internal/core/audit/rule_positioning_test.go
@@ -3,6 +3,7 @@ package audit_test
import (
"strings"
"testing"
+ "unicode/utf8"
"github.com/REPPL/abcd-cli/internal/core/audit"
)
@@ -160,6 +161,31 @@ func TestPositioningMissingBlockIsAFinding(t *testing.T) {
}
}
+// A long drifted paragraph is truncated for the one-line message, but the cut
+// must land on a rune boundary: surface text is arbitrary UTF-8 from the
+// audited repo, and a mid-rune slice puts a replacement glyph in the report.
+func TestPositioningTruncationLandsOnARuneBoundary(t *testing.T) {
+ // Em dashes are 3 bytes each and 200 is not a multiple of 3, so a fixed
+ // 200-byte cut lands on the last byte of a rune rather than on a boundary.
+ long := strings.Repeat("—", 120)
+ res := newFixtureRepo(t).conforming().withPositioning().
+ file("README.md", "\n").commit().run()
+
+ f := findingFor(res, "identity-positioning")
+ if f == nil {
+ t.Fatal("long drifted strapline produced no finding")
+ }
+ // The quoted text is rendered with %q, which escapes an invalid byte as
+ // \xNN rather than replacing it — so a mid-rune cut shows up as byte-escape
+ // litter in the report, not as a replacement glyph.
+ if strings.Contains(f.Message, `\x`) {
+ t.Errorf("finding message carries a byte escape from a mid-rune cut: %s", f.Message)
+ }
+ if !utf8.ValidString(f.Message) {
+ t.Errorf("finding message is not valid UTF-8: %q", f.Message)
+ }
+}
+
// Findings must never carry an absolute developer path, on any error path.
func TestPositioningFindingsCarryNoAbsolutePath(t *testing.T) {
cases := map[string]func(*repoBuilder) *repoBuilder{
diff --git a/internal/core/positioning/init.go b/internal/core/positioning/init.go
index 154f984..0f6f393 100644
--- a/internal/core/positioning/init.go
+++ b/internal/core/positioning/init.go
@@ -95,11 +95,24 @@ func Init(root string, req InitRequest) (InitResult, error) {
// An existing block at the target location is the canon: register it,
// never rewrite it.
- if block, err := ParseBlock(root, loc); err == nil {
+ block, perr := ParseBlock(root, loc)
+ switch {
+ case perr == nil:
res.Block, res.AdoptedExisting = block, true
- } else if !errors.Is(err, ErrBlockFileMissing) && !errors.Is(err, ErrHeadingMissing) && !errors.Is(err, ErrBulletMissing) {
- return InitResult{}, err
- } else {
+
+ case errors.Is(perr, ErrBulletMissing):
+ // The heading is already there but its bullets are incomplete. Only an
+ // ABSENT file or heading is fixable by appending a section; appending to
+ // this one would leave two copies of the same heading, of which the
+ // parser still reads the broken first — so every retry would append
+ // again and none would ever succeed. Refuse before writing anything and
+ // name what a human has to repair.
+ return InitResult{}, fmt.Errorf("%w — complete it by hand, then re-run to register it", perr)
+
+ case !errors.Is(perr, ErrBlockFileMissing) && !errors.Is(perr, ErrHeadingMissing):
+ return InitResult{}, perr
+
+ default:
title, tagline := strings.TrimSpace(req.Title), strings.TrimSpace(req.Tagline)
if title == "" || tagline == "" {
return InitResult{}, fmt.Errorf("%w (the pitch is optional)", ErrAnswersRequired)
@@ -108,13 +121,13 @@ func Init(root string, req InitRequest) (InitResult, error) {
return InitResult{}, err
}
res.Wrote = append(res.Wrote, loc.File)
- block, err := ParseBlock(root, loc)
- if err != nil {
+ written, werr := ParseBlock(root, loc)
+ if werr != nil {
// The block we just wrote must parse; if it does not, the scaffold
// is broken and must not be registered as a canon.
- return InitResult{}, fmt.Errorf("the scaffolded identity block does not parse: %w", err)
+ return InitResult{}, fmt.Errorf("the scaffolded identity block does not parse: %w", werr)
}
- res.Block = block
+ res.Block = written
}
if err := writeConfig(root, DefaultConfig(loc)); err != nil {
diff --git a/internal/surface/cli/identity_surface_test.go b/internal/surface/cli/identity_surface_test.go
index c26099a..953534b 100644
--- a/internal/surface/cli/identity_surface_test.go
+++ b/internal/surface/cli/identity_surface_test.go
@@ -4,11 +4,13 @@ import (
"bytes"
"encoding/json"
"os"
+ "os/exec"
"path/filepath"
"strings"
"testing"
"github.com/REPPL/abcd-cli/internal/core/positioning"
+ "github.com/REPPL/abcd-cli/internal/gittest"
)
const idBrief = `## Identity (canonical)
@@ -195,6 +197,44 @@ func TestIdentityRenderPrintsTheProposedDiff(t *testing.T) {
}
}
+// The patch the HUMAN sees must be as applicable as the one in --json. The
+// core's CRLF round-trip is worthless if the render path flattens it, and a
+// core-level test cannot catch that — this asserts it at the front door.
+func TestIdentityRenderKeepsCRLFOnTheHumanPath(t *testing.T) {
+ crlf := strings.ReplaceAll(idDriftedREADME, "\n", "\r\n")
+ repo := idRepo(t, map[string]string{
+ ".abcd/development/brief.md": idBrief,
+ ".abcd/positioning.json": idConfig,
+ "README.md": crlf,
+ })
+ t.Chdir(repo)
+
+ var stdout, stderr bytes.Buffer
+ if code := Run([]string{"identity", "render"}, &stdout, &stderr); code != 0 {
+ t.Fatalf("exit = %d, stderr = %s", code, stderr.String())
+ }
+ out := stdout.String()
+ if !strings.Contains(out, "- An opinionated, intent-driven development framework for product thinkers.
\r\n") {
+ t.Errorf("human patch dropped the carriage return a CRLF surface needs:\n%q", out)
+ }
+ // And it is a patch git will take.
+ if _, err := exec.LookPath("git"); err != nil {
+ t.Skip("git not on PATH")
+ }
+ runGitT(t, repo, "init", "-q")
+ patch := filepath.Join(t.TempDir(), "p.patch")
+ // Drop the two header lines the human render prefixes the diffs with.
+ body := out[strings.Index(out, "--- a/"):]
+ if err := os.WriteFile(patch, []byte(body), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ cmd := exec.Command("git", "-C", repo, "apply", "--check", patch)
+ cmd.Env = gittest.Env(t)
+ if o, err := cmd.CombinedOutput(); err != nil {
+ t.Fatalf("git apply --check refused the rendered patch: %v: %s\n%q", err, o, body)
+ }
+}
+
func TestIdentityRenderOnACleanRepoProposesNothing(t *testing.T) {
t.Chdir(adoptedRepo(t, idCleanREADME))
@@ -356,6 +396,40 @@ func TestIdentityInitIsIdempotent(t *testing.T) {
}
}
+// A heading that already exists but carries incomplete bullets cannot be
+// repaired by appending a second copy of itself: init must refuse before it
+// writes anything, name the file, and stay refusing on a re-run.
+func TestIdentityInitRefusesAnIncompleteExistingBlock(t *testing.T) {
+ const partial = "## Identity (canonical)\n\n- **Title:** Widget\n"
+ repo := idRepo(t, map[string]string{
+ ".abcd/development/IDENTITY.md": partial,
+ "README.md": idCleanREADME,
+ })
+ t.Chdir(repo)
+
+ for attempt := 1; attempt <= 3; attempt++ {
+ var stdout, stderr bytes.Buffer
+ code := Run([]string{"identity", "init", "--title", "Widget", "--tagline", "A sharper widget."}, &stdout, &stderr)
+ if code == 0 {
+ t.Fatalf("attempt %d: init accepted an incomplete existing block", attempt)
+ }
+ if !strings.Contains(stderr.String()+stdout.String(), ".abcd/development/IDENTITY.md") {
+ t.Errorf("attempt %d: refusal does not name the file to repair: %s", attempt, stderr.String())
+ }
+ }
+ // Nothing was appended, and no registry was left half-adopted.
+ data, err := os.ReadFile(filepath.Join(repo, ".abcd/development/IDENTITY.md"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if string(data) != partial {
+ t.Fatalf("init rewrote or appended to the existing block:\n%s", data)
+ }
+ if _, ok, err := positioning.LoadConfig(repo); err != nil || ok {
+ t.Fatalf("a refused init left a registry behind: ok=%v err=%v", ok, err)
+ }
+}
+
// A repo whose registry already points somewhere must never be silently
// repointed by an init with different answers.
func TestIdentityInitRefusesToRepointAnAdoptedRegistry(t *testing.T) {
diff --git a/internal/termsafe/termsafe.go b/internal/termsafe/termsafe.go
index f4c1863..f060265 100644
--- a/internal/termsafe/termsafe.go
+++ b/internal/termsafe/termsafe.go
@@ -38,16 +38,32 @@ func Sanitize(s string) string {
}
// SanitizeBlock sanitises multi-line text while keeping its line structure: each
-// line is sanitised on its own and the newlines are preserved. Sanitize masks a
-// newline (an injected line break must not forge extra report lines), which is
-// right for a value interpolated into one line and wrong for output that IS
-// lines — a rendered diff or a quoted file excerpt, where flattening the
-// newlines destroys the artefact the reader needs. Use this only where the line
-// breaks are the render's own, never for a single untrusted value.
+// line is sanitised on its own and the line terminators are preserved. Sanitize
+// masks a newline (an injected line break must not forge extra report lines),
+// which is right for a value interpolated into one line and wrong for output
+// that IS lines — a rendered diff or a quoted file excerpt, where flattening the
+// terminators destroys the artefact the reader needs. Use this only where the
+// line breaks are the render's own, never for a single untrusted value.
+//
+// The carriage return of a CRLF pair survives; every other carriage return is
+// masked. That distinction is load-bearing in both directions: a BARE CR moves
+// the cursor to the column zero of the line being written, so it can overprint
+// what the reader already saw — the attack Sanitize masks — whereas the CR of a
+// CRLF is immediately committed by its newline and can overprint nothing. And a
+// rendered patch against a CRLF file must carry those pairs or no patch tool
+// will apply it, so dropping them would silently void the artefact.
func SanitizeBlock(s string) string {
lines := strings.Split(s, "\n")
for i, l := range lines {
- lines[i] = Sanitize(strings.TrimSuffix(l, "\r"))
+ // A trailing carriage return is half of a CRLF pair only when a newline
+ // actually follows it — that is, on every element except the last, which
+ // by construction has no newline after it. A trailing CR there is bare
+ // and is masked like any other.
+ if i < len(lines)-1 && strings.HasSuffix(l, "\r") {
+ lines[i] = Sanitize(l[:len(l)-1]) + "\r"
+ continue
+ }
+ lines[i] = Sanitize(l)
}
return strings.Join(lines, "\n")
}
diff --git a/internal/termsafe/termsafe_test.go b/internal/termsafe/termsafe_test.go
index cbc2b20..64ffcc7 100644
--- a/internal/termsafe/termsafe_test.go
+++ b/internal/termsafe/termsafe_test.go
@@ -16,13 +16,26 @@ func TestSanitizeBlockKeepsLinesAndStillMasksAttackRunes(t *testing.T) {
if want := "--- a/README.md\n+ strapline?[31m\n cONTEXT\n"; got != want {
t.Errorf("SanitizeBlock = %q, want %q", got, want)
}
- // A carriage return would still let a line be overwritten in place.
- if got := SanitizeBlock("a\r\nb"); got != "a\nb" {
- t.Errorf("SanitizeBlock(CRLF) = %q, want %q", got, "a\nb")
+ // CRLF survives: the carriage return of a CRLF pair is committed by the
+ // newline that follows it, so it cannot overwrite a line — and a rendered
+ // patch against a CRLF file must keep it or no patch tool will apply.
+ if got := SanitizeBlock("a\r\nb"); got != "a\r\nb" {
+ t.Errorf("SanitizeBlock(CRLF) = %q, want %q", got, "a\r\nb")
}
+ // A bare carriage return mid-line still lets a line be overwritten in
+ // place, so it is still masked.
if got := SanitizeBlock(withRune("a", 0x0d, "b")); got != "a?b" {
t.Errorf("SanitizeBlock(bare CR) = %q, want %q", got, "a?b")
}
+ // ...including a run of them before the newline: only the one immediately
+ // preceding it is part of the pair.
+ if got := SanitizeBlock("a\r\r\nb"); got != "a?\r\nb" {
+ t.Errorf("SanitizeBlock(double CR) = %q, want %q", got, "a?\r\nb")
+ }
+ // A trailing bare CR with no newline after it is not a CRLF pair.
+ if got := SanitizeBlock(withRune("a", 0x0d, "")); got != "a?" {
+ t.Errorf("SanitizeBlock(trailing bare CR) = %q, want %q", got, "a?")
+ }
}
// TestSanitizeNeutralisesDisplayAttacks proves each terminal-display attack class
From b00c796516d5a967f9ef7cf33e123864f651be5c Mon Sep 17 00:00:00 2001
From: REPPL <77722411+REPPL@users.noreply.github.com>
Date: Tue, 28 Jul 2026 03:44:44 +0100
Subject: [PATCH 12/14] fix: contain every positioning read and write to the
repository root
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The positioning check resolved every configured path with
filepath.Join(root, rel) behind fsutil.ValidRelPath. That guard is LEXICAL —
it rejects ".." and absolute paths and can say nothing about symlinks — and
fsutil.ReadGuarded's O_NOFOLLOW guards only the LEAF. An audited repo that
COMMITS a symlinked directory (git mode 120000: `etclink -> /etc`, or
`home -> ../../..`) and names a file through it therefore had its read resolve
straight out of the repository. A surface's content is quoted verbatim into
the Report, so `abcd audit` printed an outside file into its finding and
`abcd identity --json` emitted up to 1 MiB of it whole; SSH keys, cloud
credentials, and host tokens are all reachable from a relative-ancestor link.
Containment now rests on os.Root, the pattern the lifeboat embark path already
uses, so the OS enforces it against every ancestor component rather than a
string check that cannot see one. All four sites route through a root opened
per operation: the surface read (check), the canonical block read (block), the
registry read (config), and onboarding's block and registry writes (init) —
plus the proposal's re-read, which carries surface bytes into a diff.
ValidRelPath stays as the cheap first reject; it is no longer load-bearing.
fsutil gains the contained counterparts of its three primitives, so the
one-canonical-primitive invariant holds and no caller hand-rolls containment:
ReadGuardedInRoot keeps every leaf guarantee ReadGuarded gives (symlinked leaf
refused, non-regular refused, O_NONBLOCK, the byte cap enforced on both the
fstat size and the bytes read) and adds os.SameFile to close the lstat-to-open
swap; WriteFileAtomicInRoot and WriteFileAtomicPreserveModeInRoot keep the
write, fchmod, fsync, rename, parent-fsync ordering with an unpredictable temp
name created inside the root.
A refusal is never an absence: an escaping path is not an os.IsNotExist error,
so the loop that skips absent candidates fails closed on it, and the new
ErrSurfaceUnreadable sentinel names the refusal to a caller.
The regression test builds the attack in both real shapes — an absolute target
and a relative ancestor — and asserts that no byte from outside the root
reaches a Report, a Block, a Config, a proposal, or an error message, and that
onboarding writes nothing outside. Before the change it reproduces the leak at
all four sites.
Assisted-by: Claude:claude-opus-5
---
CHANGELOG.md | 5 +-
internal/core/positioning/block.go | 37 +++-
internal/core/positioning/check.go | 43 ++--
internal/core/positioning/config.go | 15 +-
internal/core/positioning/containment_test.go | 204 ++++++++++++++++++
internal/core/positioning/init.go | 42 ++--
internal/core/positioning/render.go | 11 +-
internal/fsutil/fsutil.go | 176 +++++++++++++++
internal/fsutil/root_test.go | 173 +++++++++++++++
9 files changed, 663 insertions(+), 43 deletions(-)
create mode 100644 internal/core/positioning/containment_test.go
create mode 100644 internal/fsutil/root_test.go
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6f3ea44..b6b8737 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -36,7 +36,10 @@ called out in a **Breaking** section.
scope — adopting a proposal is always the maintainer's move); `abcd identity
init` records the block and the pointer at onboarding, adopting an existing
block rather than re-interviewing over it. `/abcd:prepare-this-repo` gains the
- interview, detect-first.
+ interview, detect-first. Every path the registry names is read and written
+ inside an OS-enforced containment root, so an audited repo that commits a
+ symlinked directory cannot make the check read a file the repository does not
+ own and quote it into the report.
- **`abcd launch scaffold` — the changelog-driven release-gate scaffolder**
(itd-93, spc-14). Writes the fixed release machinery into a managed repo that
diff --git a/internal/core/positioning/block.go b/internal/core/positioning/block.go
index a65262e..0964374 100644
--- a/internal/core/positioning/block.go
+++ b/internal/core/positioning/block.go
@@ -17,7 +17,6 @@ import (
"errors"
"fmt"
"os"
- "path/filepath"
"regexp"
"strings"
@@ -94,6 +93,34 @@ var bulletRe = regexp.MustCompile(`^ {0,3}[-*]\s+\*\*([A-Za-z]+):\*\*\s*(.*)$`)
// sentinel-wrapped error, never a zero Block with a nil error — a caller must
// not read "no block" as "the block says nothing".
func ParseBlock(root string, loc BlockLocation) (Block, error) {
+ if !fsutil.ValidRelPath(loc.File) {
+ return Block{}, fmt.Errorf("%w: %q", ErrBadLocation, loc.File)
+ }
+ r, err := openRepoRoot(root)
+ if err != nil {
+ return Block{}, fmt.Errorf("%w: %s: %s", ErrBlockFileMissing, loc.File, pathFreeReason(err))
+ }
+ defer r.Close()
+ return parseBlockIn(r, loc)
+}
+
+// openRepoRoot opens root as an os.Root containment scope. Every positioning
+// read and write resolves through one of these, so a repository that COMMITS a
+// symlinked directory (git mode 120000) as an ancestor of a configured path
+// cannot walk the operation outside itself: fsutil.ValidRelPath is lexical and
+// cannot see a symlink, and a leaf-only O_NOFOLLOW guard never looks at the
+// ancestors. The OS enforces containment instead.
+//
+// A root is opened per operation and closed by the caller, never cached: a
+// cached handle would answer for a tree that has since moved.
+func openRepoRoot(root string) (*os.Root, error) {
+ return os.OpenRoot(root)
+}
+
+// parseBlockIn is ParseBlock with the containment root already open, so a caller
+// that is already reading the repo (Check, Init) opens one root for the whole
+// operation.
+func parseBlockIn(r *os.Root, loc BlockLocation) (Block, error) {
if !fsutil.ValidRelPath(loc.File) {
return Block{}, fmt.Errorf("%w: %q", ErrBadLocation, loc.File)
}
@@ -102,11 +129,11 @@ func ParseBlock(root string, loc BlockLocation) (Block, error) {
return Block{}, fmt.Errorf("%w: heading is empty", ErrBadLocation)
}
- data, err := fsutil.ReadGuarded(filepath.Join(root, filepath.FromSlash(loc.File)), maxBlockFileBytes)
+ data, err := fsutil.ReadGuardedInRoot(r, loc.File, maxBlockFileBytes)
if err != nil {
- // A missing, symlinked, or oversize block file is all one thing to a
- // caller — the canon could not be read where the configuration said it
- // was. The reason rides along, but stripped of its absolute path: this
+ // A missing, symlinked, oversize, or root-escaping block file is all one
+ // thing to a caller — the canon could not be read where the configuration
+ // said it was. The reason rides along, but stripped of its absolute path: this
// error reaches an audit finding, which must never carry one (iss-81).
return Block{}, fmt.Errorf("%w: %s: %s", ErrBlockFileMissing, loc.File, pathFreeReason(err))
}
diff --git a/internal/core/positioning/check.go b/internal/core/positioning/check.go
index 31d05df..a160eb7 100644
--- a/internal/core/positioning/check.go
+++ b/internal/core/positioning/check.go
@@ -2,9 +2,9 @@ package positioning
import (
"encoding/json"
+ "errors"
"fmt"
"os"
- "path/filepath"
"regexp"
"strings"
@@ -15,6 +15,12 @@ import (
// the cap stops a device or runaway file being read unbounded.
const maxSurfaceBytes = 1 << 20
+// ErrSurfaceUnreadable: a registered surface file is present but could not be
+// read safely — it escapes the repository root through a symlinked component,
+// its leaf is a symlink or a device, or it is oversize. It is never an absence:
+// a surface that cannot be read must not read as "no drift".
+var ErrSurfaceUnreadable = errors.New("surface file could not be read")
+
// Status is one surface's outcome.
type Status string
@@ -76,13 +82,18 @@ func Check(root string, cfg Config) (Report, error) {
if err := cfg.Validate(); err != nil {
return Report{}, err
}
- block, err := ParseBlock(root, cfg.Block)
+ r, err := openRepoRoot(root)
+ if err != nil {
+ return Report{}, fmt.Errorf("%w: %s: %s", ErrBlockFileMissing, cfg.Block.File, pathFreeReason(err))
+ }
+ defer r.Close()
+ block, err := parseBlockIn(r, cfg.Block)
if err != nil {
return Report{}, err
}
rep := Report{Block: block, Severity: cfg.severity()}
for _, s := range cfg.surfaces() {
- res, err := checkSurface(root, s, block)
+ res, err := checkSurface(r, s, block)
if err != nil {
return Report{}, err
}
@@ -93,10 +104,10 @@ func Check(root string, cfg Config) (Report, error) {
// checkSurface resolves the surface's file, locates its self-description, and
// compares it to the block.
-func checkSurface(root string, s Surface, b Block) (SurfaceResult, error) {
+func checkSurface(r *os.Root, s Surface, b Block) (SurfaceResult, error) {
res := SurfaceResult{ID: s.ID, Status: StatusAbsent, Canonical: s.render(b)}
- rel, data, ok, err := readFirstPresent(root, s.Files)
+ rel, data, ok, err := readFirstPresent(r, s.Files)
if err != nil {
return SurfaceResult{}, err
}
@@ -135,22 +146,28 @@ func checkSurface(root string, s Surface, b Block) (SurfaceResult, error) {
}
// readFirstPresent returns the first candidate that exists, its repo-relative
-// path, and its contents. Every read is guarded: a surface file in an arbitrary
-// audited repo may be a symlink, a device, or enormous.
-func readFirstPresent(root string, files []string) (rel string, data []byte, ok bool, err error) {
+// path, and its contents. Every read is guarded AND contained: the file names
+// come from committed configuration in an arbitrary audited repo, so a candidate
+// may be a symlink, a device, enormous, or sitting behind a symlinked ancestor
+// directory that reaches outside the repository entirely. ValidRelPath is the
+// cheap lexical reject; the containment root is what actually holds the read
+// inside the repo, and a surface's content is quoted verbatim into the report,
+// so nothing from outside may ever be read here.
+func readFirstPresent(r *os.Root, files []string) (rel string, data []byte, ok bool, err error) {
for _, f := range files {
if !fsutil.ValidRelPath(f) {
return "", nil, false, fmt.Errorf("%w: surface file %q is not a repo-relative path", ErrConfigInvalid, f)
}
- b, rerr := fsutil.ReadGuarded(filepath.Join(root, filepath.FromSlash(f)), maxSurfaceBytes)
+ b, rerr := fsutil.ReadGuardedInRoot(r, f, maxSurfaceBytes)
if rerr != nil {
if os.IsNotExist(rerr) {
continue
}
- // A present-but-unreadable surface (symlinked, oversize, a device)
- // is not an absence: report it rather than skipping it silently.
- // Path-free — this reaches an audit finding (iss-81).
- return f, nil, false, fmt.Errorf("reading surface %s: %s", f, pathFreeReason(rerr))
+ // A present-but-unreadable surface (symlinked, oversize, a device, a
+ // path escaping the root) is not an absence: report it rather than
+ // skipping it silently. Path-free — this reaches an audit finding
+ // (iss-81).
+ return f, nil, false, fmt.Errorf("%w: %s: %s", ErrSurfaceUnreadable, f, pathFreeReason(rerr))
}
return f, b, true, nil
}
diff --git a/internal/core/positioning/config.go b/internal/core/positioning/config.go
index 5a93a01..7d19c35 100644
--- a/internal/core/positioning/config.go
+++ b/internal/core/positioning/config.go
@@ -5,7 +5,6 @@ import (
"errors"
"fmt"
"os"
- "path/filepath"
"regexp"
"strings"
@@ -152,8 +151,18 @@ func DefaultConfig(loc BlockLocation) Config {
// error when the file is present but unusable — a malformed registry must never
// read as "no drift".
func LoadConfig(root string) (Config, bool, error) {
- path := filepath.Join(root, filepath.FromSlash(ConfigRelPath))
- data, err := fsutil.ReadGuarded(path, maxConfigBytes)
+ r, err := openRepoRoot(root)
+ if err != nil {
+ if os.IsNotExist(err) {
+ return Config{}, false, nil
+ }
+ return Config{}, false, fmt.Errorf("%w: %s: %s", ErrConfigInvalid, ConfigRelPath, pathFreeReason(err))
+ }
+ defer r.Close()
+ // Contained, not merely lexically checked: ConfigRelPath is a fixed path,
+ // but a hostile repo can commit `.abcd` itself as a symlink and have the
+ // loader read — and then act on — a registry the repository does not own.
+ data, err := fsutil.ReadGuardedInRoot(r, ConfigRelPath, maxConfigBytes)
if err != nil {
if os.IsNotExist(err) {
return Config{}, false, nil
diff --git a/internal/core/positioning/containment_test.go b/internal/core/positioning/containment_test.go
new file mode 100644
index 0000000..2803529
--- /dev/null
+++ b/internal/core/positioning/containment_test.go
@@ -0,0 +1,204 @@
+package positioning
+
+import (
+ "errors"
+ "fmt"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+)
+
+// containmentCanary stands in for whatever a hostile repo is actually aiming at
+// — an SSH key, a credentials file, a host token. If it ever reaches a Report, a
+// Block, or an error message, content from outside the repository escaped.
+const containmentCanary = "SUPER-SECRET-CONTAINMENT-CANARY"
+
+// hostileRepo builds the attack every lexical path check misses: a repository
+// that COMMITS a symlinked directory (git mode 120000) and then names a file
+// through it. Nothing in the configured path contains ".." or a leading "/", so
+// fsutil.ValidRelPath accepts it and filepath.Join resolves it straight through
+// the link to a file the repository does not own.
+//
+// Both real shapes are built: an absolute target (`etclink -> /etc`) and a
+// relative one (`home -> ../../..`), which is the form that reaches a developer's
+// dotfiles from an arbitrary clone location.
+//
+// It returns the repo root and the outside directory the link lands in.
+func hostileRepo(t *testing.T, linkName string, absolute bool, outsideFiles map[string]string) (repo, outside string) {
+ t.Helper()
+ base := t.TempDir()
+ outside = filepath.Join(base, "outside")
+ if err := os.MkdirAll(outside, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ for rel, content := range outsideFiles {
+ p := filepath.Join(outside, filepath.FromSlash(rel))
+ if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(p, []byte(content), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ }
+ repo = filepath.Join(base, "repo")
+ if err := os.MkdirAll(repo, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ target := "../outside" // a relative ancestor escape, resolved from repo/
+ if absolute {
+ target = outside
+ }
+ if err := os.Symlink(target, filepath.Join(repo, linkName)); err != nil {
+ t.Fatal(err)
+ }
+ return repo, outside
+}
+
+// writeInto drops files into an existing directory (writeRepo makes its own).
+func writeInto(t *testing.T, dir string, files map[string]string) {
+ t.Helper()
+ for rel, content := range files {
+ p := filepath.Join(dir, filepath.FromSlash(rel))
+ if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(p, []byte(content), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ }
+}
+
+const containmentBlock = `# Identity
+
+## Identity (canonical)
+
+- **Title:** Contained
+- **Tagline:** A tagline no surface carries.
+`
+
+// TestCheckRefusesASurfaceReachedThroughASymlinkedAncestor is the leak this
+// package's containment exists to stop: a cloned repo declares a surface behind
+// a committed symlinked directory, and the check reads a file outside the repo
+// and quotes it verbatim into the Report — which `abcd audit` prints and
+// `abcd identity --json` emits whole.
+func TestCheckRefusesASurfaceReachedThroughASymlinkedAncestor(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ link string
+ absolute bool
+ }{
+ {"absolute target", "etclink", true},
+ {"relative ancestor target", "home", false},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ repo, _ := hostileRepo(t, tc.link, tc.absolute, map[string]string{
+ "secret.txt": containmentCanary + "\n",
+ })
+ writeInto(t, repo, map[string]string{"IDENTITY.md": containmentBlock})
+
+ cfg := Config{
+ SchemaVersion: 1,
+ Block: BlockLocation{File: "IDENTITY.md", Heading: "Identity (canonical)"},
+ Surfaces: []Surface{{
+ ID: "leak",
+ Files: []string{tc.link + "/secret.txt"},
+ Kind: KindRegexp,
+ Patterns: []string{`(?s)^(.*)$`},
+ Requires: []string{"tagline"},
+ }},
+ }
+ // The lexical guard accepts the path: it has no ".." and no leading
+ // "/", so containment cannot rest on it.
+ if err := cfg.Validate(); err != nil {
+ t.Fatalf("fixture is not the attack under test (config refused lexically): %v", err)
+ }
+
+ rep, err := Check(repo, cfg)
+ if err == nil {
+ t.Fatalf("Check read through a symlinked ancestor instead of refusing; report = %+v", rep)
+ }
+ if !errors.Is(err, ErrSurfaceUnreadable) {
+ t.Fatalf("err = %v, want ErrSurfaceUnreadable", err)
+ }
+ assertNoCanary(t, "Check error", err.Error())
+ assertNoCanary(t, "Check report", fmt.Sprintf("%+v", rep))
+
+ // The proposal path re-reads the surface file; it must refuse too.
+ prop, perr := Propose(repo, cfg)
+ if perr == nil {
+ t.Fatalf("Propose read through a symlinked ancestor; proposal = %+v", prop)
+ }
+ assertNoCanary(t, "Propose proposal", fmt.Sprintf("%+v", prop))
+ })
+ }
+}
+
+// TestParseBlockRefusesABlockReachedThroughASymlinkedAncestor closes the same
+// hole on the canon: a registry that points the block at a path behind a
+// committed symlink must not parse a file outside the repository as the repo's
+// own identity.
+func TestParseBlockRefusesABlockReachedThroughASymlinkedAncestor(t *testing.T) {
+ repo, _ := hostileRepo(t, "home", false, map[string]string{
+ "IDENTITY.md": "## Identity (canonical)\n\n- **Title:** " + containmentCanary +
+ "\n- **Tagline:** " + containmentCanary + "\n",
+ })
+ loc := BlockLocation{File: "home/IDENTITY.md", Heading: "Identity (canonical)"}
+
+ b, err := ParseBlock(repo, loc)
+ if err == nil {
+ t.Fatalf("ParseBlock read through a symlinked ancestor; block = %+v", b)
+ }
+ if !errors.Is(err, ErrBlockFileMissing) {
+ t.Fatalf("err = %v, want ErrBlockFileMissing", err)
+ }
+ assertNoCanary(t, "ParseBlock error", err.Error())
+ assertNoCanary(t, "ParseBlock block", fmt.Sprintf("%+v", b))
+}
+
+// TestLoadConfigRefusesARegistryReachedThroughASymlinkedAncestor: the registry
+// itself sits at a fixed path, but a hostile repo can commit `.abcd` as a
+// symlink and have the loader read a registry it does not own.
+func TestLoadConfigRefusesARegistryReachedThroughASymlinkedAncestor(t *testing.T) {
+ repo, _ := hostileRepo(t, ".abcd", false, map[string]string{
+ "positioning.json": `{"schema_version":1,"block":{"file":"IDENTITY.md","heading":"` +
+ containmentCanary + `"}}`,
+ })
+
+ cfg, ok, err := LoadConfig(repo)
+ if err == nil {
+ t.Fatalf("LoadConfig read a registry through a symlinked ancestor (ok=%v, cfg=%+v)", ok, cfg)
+ }
+ if ok {
+ t.Error("LoadConfig reported the registry as adopted despite refusing it")
+ }
+ assertNoCanary(t, "LoadConfig error", err.Error())
+ assertNoCanary(t, "LoadConfig config", fmt.Sprintf("%+v", cfg))
+}
+
+// TestInitRefusesToWriteThroughASymlinkedAncestor: onboarding writes two files,
+// and the write path needs the same containment as the read path — a location
+// behind a committed symlink must not land the scaffolded block outside the
+// repository.
+func TestInitRefusesToWriteThroughASymlinkedAncestor(t *testing.T) {
+ repo, outside := hostileRepo(t, "home", false, nil)
+
+ res, err := Init(repo, InitRequest{
+ Title: "Contained",
+ Tagline: "A tagline.",
+ Location: BlockLocation{File: "home/IDENTITY.md", Heading: "Identity (canonical)"},
+ })
+ if err == nil {
+ t.Fatalf("Init wrote through a symlinked ancestor; result = %+v", res)
+ }
+ if _, serr := os.Lstat(filepath.Join(outside, "IDENTITY.md")); serr == nil {
+ t.Fatal("Init created a file outside the repository root")
+ }
+}
+
+func assertNoCanary(t *testing.T, what, s string) {
+ t.Helper()
+ if strings.Contains(s, containmentCanary) {
+ t.Fatalf("%s carries content from outside the repository root: %s", what, s)
+ }
+}
diff --git a/internal/core/positioning/init.go b/internal/core/positioning/init.go
index 0f6f393..5af9485 100644
--- a/internal/core/positioning/init.go
+++ b/internal/core/positioning/init.go
@@ -6,7 +6,6 @@ import (
"errors"
"fmt"
"os"
- "path/filepath"
"strings"
"github.com/REPPL/abcd-cli/internal/fsutil"
@@ -75,6 +74,15 @@ func Init(root string, req InitRequest) (InitResult, error) {
return InitResult{}, fmt.Errorf("%w: %q", ErrBadLocation, loc.File)
}
+ // One containment root for the whole operation: every read and every write
+ // below resolves through it, so neither the block location nor the registry
+ // path can land outside the repository via a symlinked ancestor.
+ r, err := openRepoRoot(root)
+ if err != nil {
+ return InitResult{}, err
+ }
+ defer r.Close()
+
answered := strings.TrimSpace(req.Title) != "" || strings.TrimSpace(req.Tagline) != "" || strings.TrimSpace(req.Pitch) != ""
if existing, adopted, err := LoadConfig(root); err != nil {
@@ -84,7 +92,7 @@ func Init(root string, req InitRequest) (InitResult, error) {
return InitResult{}, fmt.Errorf("%w: it points at %s (%q); change the block there, or edit %s deliberately",
ErrAlreadyAdopted, existing.Block.File, existing.Block.Heading, ConfigRelPath)
}
- block, err := ParseBlock(root, existing.Block)
+ block, err := parseBlockIn(r, existing.Block)
if err != nil {
return InitResult{}, err
}
@@ -95,7 +103,7 @@ func Init(root string, req InitRequest) (InitResult, error) {
// An existing block at the target location is the canon: register it,
// never rewrite it.
- block, perr := ParseBlock(root, loc)
+ block, perr := parseBlockIn(r, loc)
switch {
case perr == nil:
res.Block, res.AdoptedExisting = block, true
@@ -117,11 +125,11 @@ func Init(root string, req InitRequest) (InitResult, error) {
if title == "" || tagline == "" {
return InitResult{}, fmt.Errorf("%w (the pitch is optional)", ErrAnswersRequired)
}
- if err := writeBlock(root, loc, title, tagline, strings.TrimSpace(req.Pitch)); err != nil {
+ if err := writeBlock(r, loc, title, tagline, strings.TrimSpace(req.Pitch)); err != nil {
return InitResult{}, err
}
res.Wrote = append(res.Wrote, loc.File)
- written, werr := ParseBlock(root, loc)
+ written, werr := parseBlockIn(r, loc)
if werr != nil {
// The block we just wrote must parse; if it does not, the scaffold
// is broken and must not be registered as a canon.
@@ -130,7 +138,7 @@ func Init(root string, req InitRequest) (InitResult, error) {
res.Block = written
}
- if err := writeConfig(root, DefaultConfig(loc)); err != nil {
+ if err := writeConfig(r, DefaultConfig(loc)); err != nil {
return InitResult{}, err
}
res.Wrote = append(res.Wrote, ConfigRelPath)
@@ -159,11 +167,12 @@ func blockSection(heading, title, tagline, pitch string) string {
// writeBlock lands the identity section at loc: appended to the file when it
// already exists (never replacing what is there), or as a new file with a title
// heading when it does not.
-func writeBlock(root string, loc BlockLocation, title, tagline, pitch string) error {
- path := filepath.Join(root, filepath.FromSlash(loc.File))
+// The read and both writes resolve through r, so a location behind a symlinked
+// ancestor refuses instead of scaffolding the block outside the repository.
+func writeBlock(r *os.Root, loc BlockLocation, title, tagline, pitch string) error {
section := blockSection(loc.Heading, title, tagline, pitch)
- existing, err := fsutil.ReadGuarded(path, maxBlockFileBytes)
+ existing, err := fsutil.ReadGuardedInRoot(r, loc.File, maxBlockFileBytes)
switch {
case err == nil:
body := normalizeNewlines(existing)
@@ -173,12 +182,9 @@ func writeBlock(root string, loc BlockLocation, title, tagline, pitch string) er
if !strings.HasSuffix(body, "\n\n") {
body += "\n"
}
- return fsutil.WriteFileAtomicPreserveMode(path, []byte(body+section))
+ return fsutil.WriteFileAtomicPreserveModeInRoot(r, loc.File, []byte(body+section))
case os.IsNotExist(err):
- if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
- return err
- }
- return fsutil.WriteFileAtomic(path, []byte("# Identity\n\n"+section), 0o644)
+ return fsutil.WriteFileAtomicInRoot(r, loc.File, []byte("# Identity\n\n"+section), 0o644)
default:
return fmt.Errorf("%s: %s", loc.File, pathFreeReason(err))
}
@@ -186,11 +192,7 @@ func writeBlock(root string, loc BlockLocation, title, tagline, pitch string) er
// writeConfig lands the registry, pretty-printed with a trailing newline,
// through the canonical atomic primitive.
-func writeConfig(root string, cfg Config) error {
- path := filepath.Join(root, filepath.FromSlash(ConfigRelPath))
- if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
- return err
- }
+func writeConfig(r *os.Root, cfg Config) error {
var buf bytes.Buffer
enc := json.NewEncoder(&buf)
// The registry carries regexps full of characters HTML escaping would
@@ -200,5 +202,5 @@ func writeConfig(root string, cfg Config) error {
if err := enc.Encode(cfg); err != nil {
return err
}
- return fsutil.WriteFileAtomic(path, buf.Bytes(), 0o644)
+ return fsutil.WriteFileAtomicInRoot(r, ConfigRelPath, buf.Bytes(), 0o644)
}
diff --git a/internal/core/positioning/render.go b/internal/core/positioning/render.go
index 0f5b636..14852f3 100644
--- a/internal/core/positioning/render.go
+++ b/internal/core/positioning/render.go
@@ -59,6 +59,15 @@ func Propose(root string, cfg Config) (Proposal, error) {
byID[s.ID] = s
}
+ // The re-read is contained exactly as the check's read was: a proposal
+ // carries the surface's current bytes into its diff, so an uncontained read
+ // here would leak outside content through the patch instead of the report.
+ r, err := openRepoRoot(root)
+ if err != nil {
+ return Proposal{}, err
+ }
+ defer r.Close()
+
p := Proposal{Report: rep}
for _, res := range rep.Surfaces {
if res.Status != StatusDrifted {
@@ -68,7 +77,7 @@ func Propose(root string, cfg Config) (Proposal, error) {
if !ok {
continue
}
- _, data, present, rerr := readFirstPresent(root, []string{res.File})
+ _, data, present, rerr := readFirstPresent(r, []string{res.File})
if rerr != nil {
return Proposal{}, rerr
}
diff --git a/internal/fsutil/fsutil.go b/internal/fsutil/fsutil.go
index 396760b..75cefcf 100644
--- a/internal/fsutil/fsutil.go
+++ b/internal/fsutil/fsutil.go
@@ -8,12 +8,21 @@
// WriteFileAtomicPreserveMode / IsRealDir rather than keep divergent copies
// (the one-canonical-primitive invariant, guarded by
// TestNoNonCanonicalAtomicWritePrimitives).
+//
+// Each read/write primitive comes in two forms. The plain form takes a path and
+// guards the LEAF only; the …InRoot form takes an *os.Root and resolves every
+// component inside it, which is what contains a path whose ANCESTOR is a symlink
+// planted by untrusted content. Any path that arrives as data — a committed
+// configuration value, a packed manifest entry — belongs in the …InRoot form.
package fsutil
import (
+ "crypto/rand"
+ "encoding/hex"
"errors"
"io"
"os"
+ "path"
"path/filepath"
"syscall"
)
@@ -62,6 +71,67 @@ func ReadGuarded(path string, limit int64) ([]byte, error) {
return data, nil
}
+// ReadGuardedInRoot is ReadGuarded resolved inside an os.Root containment
+// scope. rel is a slash-separated path relative to root; every component is
+// resolved by the OS within root, so a symlinked ANCESTOR directory — the shape
+// a hostile clone commits as git mode 120000, `etclink -> /etc` or
+// `home -> ../../..` — cannot walk the read outside the root. ValidRelPath is
+// lexical and cannot see a symlink, so containment must rest here, not on it.
+//
+// It keeps every guarantee ReadGuarded gives at the leaf: a symlinked leaf is
+// refused (lstat, never followed), the descriptor is confirmed to be the very
+// file that was vetted (os.SameFile closes the lstat→open swap), a non-regular
+// leaf returns ErrNotRegular, O_NONBLOCK stops a FIFO or device blocking the
+// open, and the caller's byte cap is enforced against both the fstat size and
+// the bytes actually read.
+//
+// Errors are returned raw where the OS raised them, so a caller can still test
+// os.IsNotExist; a path that escapes the root is NOT an os.IsNotExist error, so
+// a caller skipping absent files fails closed on the escape rather than treating
+// it as "the file is not there".
+func ReadGuardedInRoot(root *os.Root, rel string, limit int64) ([]byte, error) {
+ fi, err := root.Lstat(rel)
+ if err != nil {
+ return nil, err
+ }
+ if fi.Mode()&os.ModeSymlink != 0 {
+ return nil, ErrNotRegular
+ }
+ if !fi.Mode().IsRegular() {
+ return nil, ErrNotRegular
+ }
+ f, err := root.OpenFile(rel, os.O_RDONLY|syscall.O_NONBLOCK, 0)
+ if err != nil {
+ return nil, err
+ }
+ defer f.Close()
+ st, err := f.Stat()
+ if err != nil {
+ return nil, err
+ }
+ if !st.Mode().IsRegular() {
+ return nil, ErrNotRegular
+ }
+ if !os.SameFile(fi, st) {
+ // Swapped between the lstat and the open. os.Root already stops the
+ // swap escaping the root, but the descriptor is no longer the file that
+ // was vetted, so it is refused rather than read.
+ return nil, ErrNotRegular
+ }
+ if st.Size() > limit {
+ return nil, ErrTooBig
+ }
+ data, err := io.ReadAll(io.LimitReader(f, limit+1))
+ if err != nil {
+ return nil, err
+ }
+ if int64(len(data)) > limit {
+ // Grew past the cap between fstat and read (a size TOCTOU).
+ return nil, ErrTooBig
+ }
+ return data, nil
+}
+
// WriteFileAtomic writes data to path durably: a temp file in the target
// directory is written, chmod'd to perm on its open descriptor, flushed,
// fsync'd, then renamed over the target, and finally the parent directory is
@@ -131,6 +201,112 @@ func WriteFileAtomicPreserveMode(path string, data []byte) error {
return WriteFileAtomic(path, data, perm)
}
+// WriteFileAtomicInRoot is WriteFileAtomic resolved inside an os.Root
+// containment scope: rel is a slash-separated path relative to root, and the
+// missing parents, the temp file, the rename, and the parent fsync all happen
+// through root. A symlinked ancestor committed by a hostile repo therefore
+// cannot land the write outside the root — the counterpart of
+// ReadGuardedInRoot, and the reason a write path validated only lexically is not
+// contained.
+//
+// The commit point and the ordering are WriteFileAtomic's: write, fchmod on the
+// open descriptor (never chmod-by-name), fsync, close, rename, parent fsync.
+// The temp name is unpredictable so the enumerable-name swap WriteFileAtomic
+// guards against has nothing to aim at.
+func WriteFileAtomicInRoot(root *os.Root, rel string, data []byte, perm os.FileMode) error {
+ dir := path.Dir(rel)
+ if dir != "." {
+ if err := root.MkdirAll(dir, 0o755); err != nil {
+ return err
+ }
+ }
+ tmp, tmpRel, err := createTempInRoot(root, dir)
+ if err != nil {
+ return err
+ }
+ abandon := func(err error) error {
+ tmp.Close()
+ _ = root.Remove(tmpRel)
+ return err
+ }
+ if _, err := tmp.Write(data); err != nil {
+ return abandon(err)
+ }
+ if err := tmp.Chmod(perm); err != nil {
+ return abandon(err)
+ }
+ if err := tmp.Sync(); err != nil {
+ return abandon(err)
+ }
+ if err := tmp.Close(); err != nil {
+ _ = root.Remove(tmpRel)
+ return err
+ }
+ if err := root.Rename(tmpRel, rel); err != nil {
+ _ = root.Remove(tmpRel)
+ return err
+ }
+ syncDirInRoot(root, dir)
+ return nil
+}
+
+// WriteFileAtomicPreserveModeInRoot is WriteFileAtomicInRoot that keeps the
+// target's existing permission bits when it already exists, defaulting to 0644
+// for a new file — the contained counterpart of
+// WriteFileAtomicPreserveMode.
+func WriteFileAtomicPreserveModeInRoot(root *os.Root, rel string, data []byte) error {
+ perm := os.FileMode(0o644)
+ fi, err := root.Lstat(rel)
+ switch {
+ case err == nil:
+ if fi.Mode().IsRegular() {
+ perm = fi.Mode().Perm()
+ }
+ case !notPresent(err):
+ // A real stat fault (a transient I/O error, an escaping path, EACCES) is
+ // NOT "absent" — defaulting to 0644 here would silently widen an existing
+ // restrictive mode, and an escaping path must refuse rather than proceed.
+ return err
+ }
+ return WriteFileAtomicInRoot(root, rel, data, perm)
+}
+
+// createTempInRoot creates an exclusively-owned temp file in dir inside root,
+// returning it and its root-relative path. os.CreateTemp cannot be used here: it
+// resolves a path outside any containment scope, which is precisely what the
+// root exists to prevent.
+func createTempInRoot(root *os.Root, dir string) (*os.File, string, error) {
+ var buf [8]byte
+ for range 100 {
+ if _, err := rand.Read(buf[:]); err != nil {
+ return nil, "", err
+ }
+ rel := ".abcd-tmp-" + hex.EncodeToString(buf[:])
+ if dir != "." {
+ rel = dir + "/" + rel
+ }
+ f, err := root.OpenFile(rel, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o600)
+ if err == nil {
+ return f, rel, nil
+ }
+ if !errors.Is(err, os.ErrExist) {
+ return nil, "", err
+ }
+ }
+ return nil, "", errors.New("fsutil: could not create a temp file in the containment root")
+}
+
+// syncDirInRoot fsyncs dir inside root so a crash right after the rename cannot
+// lose it. Some filesystems refuse a directory fsync; that is tolerated.
+func syncDirInRoot(root *os.Root, dir string) {
+ d, err := root.Open(dir)
+ if err != nil {
+ return
+ }
+ _ = d.Sync()
+ _ = d.Close()
+}
+
// syncParent fsyncs the directory so a crash right after the rename cannot lose
// it. Some filesystems refuse a directory fsync; that is tolerated.
func syncParent(dir string) {
diff --git a/internal/fsutil/root_test.go b/internal/fsutil/root_test.go
new file mode 100644
index 0000000..72f4260
--- /dev/null
+++ b/internal/fsutil/root_test.go
@@ -0,0 +1,173 @@
+package fsutil
+
+import (
+ "errors"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+)
+
+// rootFixture builds a directory that a hostile repository could ship: a real
+// file, a symlinked leaf, and two symlinked ANCESTOR directories — one with an
+// absolute target, one with a relative one — each pointing at a sibling
+// directory the root does not contain.
+func rootFixture(t *testing.T) (dir, outside string) {
+ t.Helper()
+ base := t.TempDir()
+ outside = filepath.Join(base, "outside")
+ if err := os.MkdirAll(outside, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("OUTSIDE"), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ dir = filepath.Join(base, "root")
+ if err := os.MkdirAll(filepath.Join(dir, "sub"), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(filepath.Join(dir, "real.txt"), []byte("INSIDE"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Symlink("real.txt", filepath.Join(dir, "leaflink.txt")); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Symlink(outside, filepath.Join(dir, "abslink")); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Symlink("../outside", filepath.Join(dir, "rellink")); err != nil {
+ t.Fatal(err)
+ }
+ return dir, outside
+}
+
+func openFixtureRoot(t *testing.T, dir string) *os.Root {
+ t.Helper()
+ r, err := os.OpenRoot(dir)
+ if err != nil {
+ t.Fatal(err)
+ }
+ t.Cleanup(func() { r.Close() })
+ return r
+}
+
+func TestReadGuardedInRootReadsAContainedFile(t *testing.T) {
+ dir, _ := rootFixture(t)
+ r := openFixtureRoot(t, dir)
+ got, err := ReadGuardedInRoot(r, "real.txt", 1<<20)
+ if err != nil {
+ t.Fatalf("ReadGuardedInRoot: %v", err)
+ }
+ if string(got) != "INSIDE" {
+ t.Fatalf("read %q, want %q", got, "INSIDE")
+ }
+}
+
+// The containment guarantee ValidRelPath cannot give: neither path holds a ".."
+// or a leading "/", so only the root refuses them.
+func TestReadGuardedInRootRefusesASymlinkedAncestor(t *testing.T) {
+ dir, _ := rootFixture(t)
+ r := openFixtureRoot(t, dir)
+ for _, rel := range []string{"abslink/secret.txt", "rellink/secret.txt"} {
+ if !ValidRelPath(rel) {
+ t.Fatalf("fixture is not the case under test: ValidRelPath(%q) is false", rel)
+ }
+ data, err := ReadGuardedInRoot(r, rel, 1<<20)
+ if err == nil {
+ t.Fatalf("ReadGuardedInRoot(%q) read outside the root: %q", rel, data)
+ }
+ if os.IsNotExist(err) {
+ t.Fatalf("ReadGuardedInRoot(%q) reported an escape as absence: %v", rel, err)
+ }
+ if strings.Contains(string(data), "OUTSIDE") {
+ t.Fatalf("ReadGuardedInRoot(%q) returned outside content", rel)
+ }
+ }
+}
+
+// The leaf guarantees ReadGuarded gives must survive the move into the root.
+func TestReadGuardedInRootKeepsTheLeafGuards(t *testing.T) {
+ dir, _ := rootFixture(t)
+ r := openFixtureRoot(t, dir)
+
+ if _, err := ReadGuardedInRoot(r, "leaflink.txt", 1<<20); !errors.Is(err, ErrNotRegular) {
+ t.Fatalf("symlinked leaf: err = %v, want ErrNotRegular", err)
+ }
+ if _, err := ReadGuardedInRoot(r, "sub", 1<<20); !errors.Is(err, ErrNotRegular) {
+ t.Fatalf("directory leaf: err = %v, want ErrNotRegular", err)
+ }
+ if _, err := ReadGuardedInRoot(r, "real.txt", 3); !errors.Is(err, ErrTooBig) {
+ t.Fatalf("oversize: err = %v, want ErrTooBig", err)
+ }
+ if _, err := ReadGuardedInRoot(r, "absent.txt", 1<<20); !os.IsNotExist(err) {
+ t.Fatalf("absent: err = %v, want an IsNotExist error", err)
+ }
+}
+
+func TestWriteFileAtomicInRootWritesAndRefusesAnEscape(t *testing.T) {
+ dir, outside := rootFixture(t)
+ r := openFixtureRoot(t, dir)
+
+ if err := WriteFileAtomicInRoot(r, "deep/nested/out.txt", []byte("hello"), 0o600); err != nil {
+ t.Fatalf("WriteFileAtomicInRoot: %v", err)
+ }
+ got, err := os.ReadFile(filepath.Join(dir, "deep", "nested", "out.txt"))
+ if err != nil || string(got) != "hello" {
+ t.Fatalf("read back = %q, %v", got, err)
+ }
+ if fi, serr := os.Stat(filepath.Join(dir, "deep", "nested", "out.txt")); serr != nil || fi.Mode().Perm() != 0o600 {
+ t.Fatalf("mode = %v, %v; want 0600", fi, serr)
+ }
+ // No temp file survives the write.
+ entries, err := os.ReadDir(filepath.Join(dir, "deep", "nested"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(entries) != 1 {
+ t.Fatalf("directory holds %d entries, want only the written file", len(entries))
+ }
+
+ for _, rel := range []string{"abslink/planted.txt", "rellink/planted.txt"} {
+ if err := WriteFileAtomicInRoot(r, rel, []byte("planted"), 0o600); err == nil {
+ t.Fatalf("WriteFileAtomicInRoot(%q) wrote through a symlinked ancestor", rel)
+ }
+ }
+ if entries, err := os.ReadDir(outside); err == nil {
+ for _, e := range entries {
+ if e.Name() != "secret.txt" {
+ t.Fatalf("a write landed outside the root: %s", e.Name())
+ }
+ }
+ }
+}
+
+func TestWriteFileAtomicPreserveModeInRootKeepsTheExistingMode(t *testing.T) {
+ dir, _ := rootFixture(t)
+ r := openFixtureRoot(t, dir)
+
+ if err := os.WriteFile(filepath.Join(dir, "moded.txt"), []byte("a"), 0o640); err != nil {
+ t.Fatal(err)
+ }
+ if err := WriteFileAtomicPreserveModeInRoot(r, "moded.txt", []byte("b")); err != nil {
+ t.Fatalf("WriteFileAtomicPreserveModeInRoot: %v", err)
+ }
+ fi, err := os.Stat(filepath.Join(dir, "moded.txt"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if fi.Mode().Perm() != 0o640 {
+ t.Fatalf("mode = %v, want 0640", fi.Mode().Perm())
+ }
+
+ // A new file defaults to 0644, and an escaping path refuses rather than
+ // silently defaulting.
+ if err := WriteFileAtomicPreserveModeInRoot(r, "fresh.txt", []byte("c")); err != nil {
+ t.Fatalf("new file: %v", err)
+ }
+ if fi, err := os.Stat(filepath.Join(dir, "fresh.txt")); err != nil || fi.Mode().Perm() != 0o644 {
+ t.Fatalf("new-file mode = %v, %v; want 0644", fi, err)
+ }
+ if err := WriteFileAtomicPreserveModeInRoot(r, "rellink/planted.txt", []byte("d")); err == nil {
+ t.Fatal("WriteFileAtomicPreserveModeInRoot wrote through a symlinked ancestor")
+ }
+}
From d68003714327e42ce3ec4be43fa18db9b63f7620 Mon Sep 17 00:00:00 2001
From: REPPL <77722411+REPPL@users.noreply.github.com>
Date: Tue, 28 Jul 2026 03:44:57 +0100
Subject: [PATCH 13/14] docs: state unifiedDiff's uniform-line-ending
assumption
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
One CRLF anywhere in a surface file makes every emitted hunk line carry a
carriage return, so a file with mixed endings gets a diff whose LF-only lines
gain a CR they do not have on disk. Recording it rather than handling it: the
failure is loud, not silent — the context lines stop matching and `git apply`
refuses the patch, so nothing is ever written on the strength of a wrong diff.
Assisted-by: Claude:claude-opus-5
---
internal/core/positioning/render.go | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/internal/core/positioning/render.go b/internal/core/positioning/render.go
index 14852f3..a49ff84 100644
--- a/internal/core/positioning/render.go
+++ b/internal/core/positioning/render.go
@@ -134,6 +134,13 @@ const noNewlineMarker = "\\ No newline at end of file\n"
// old is the newline-normalised file text and start/end are byte offsets into
// it; crlf says the file on disk uses CRLF, in which case the emitted lines
// carry the carriage return back so the patch matches the real bytes.
+//
+// The CRLF handling assumes a file's line endings are uniform: one CRLF
+// anywhere makes every emitted line carry a carriage return, so a mixed-ending
+// file yields a hunk whose LF-only lines gain a CR they do not have on disk.
+// That is left as it is because the failure is loud rather than silent — the
+// context lines no longer match, `git apply` refuses the patch, and nothing is
+// written on the strength of a wrong diff.
func unifiedDiff(file, old string, start, end int, replacement string, crlf bool) string {
eol := "\n"
if crlf {
From 328b161fc15fe1fdfb3596b4e7f52e019dbca8a0 Mon Sep 17 00:00:00 2001
From: REPPL <77722411+REPPL@users.noreply.github.com>
Date: Tue, 28 Jul 2026 03:59:31 +0100
Subject: [PATCH 14/14] chore: record the two non-blocking findings from the
containment review
Both sit beside the escape the containment fix closed and neither is one:
an unbounded surface count in a committed registry (a per-run memory bound,
not a boundary crossing), and a registry naming a surface under .git/, which
stays inside the containment root and reaches only the operator's own stdout.
Recorded rather than fixed so the containment change stays one purpose.
Assisted-by: Claude:claude-opus-5
---
...ng-check-bounds-each-surface-read-to-1-mib-but.md | 12 ++++++++++++
...sitioning-registry-may-name-a-surface-inside-g.md | 12 ++++++++++++
2 files changed, 24 insertions(+)
create mode 100644 .abcd/work/issues/open/iss-149-the-positioning-check-bounds-each-surface-read-to-1-mib-but.md
create mode 100644 .abcd/work/issues/open/iss-150-a-committed-positioning-registry-may-name-a-surface-inside-g.md
diff --git a/.abcd/work/issues/open/iss-149-the-positioning-check-bounds-each-surface-read-to-1-mib-but.md b/.abcd/work/issues/open/iss-149-the-positioning-check-bounds-each-surface-read-to-1-mib-but.md
new file mode 100644
index 0000000..160ca60
--- /dev/null
+++ b/.abcd/work/issues/open/iss-149-the-positioning-check-bounds-each-surface-read-to-1-mib-but.md
@@ -0,0 +1,12 @@
+---
+schema_version: 1
+id: "iss-149"
+slug: "the-positioning-check-bounds-each-surface-read-to-1-mib-but"
+severity: "minor"
+category: "bug"
+source: "agent-finding"
+found_during: "itd-102-security-review"
+found_at: "internal/core/positioning/config.go"
+---
+
+The positioning check bounds each surface read to 1 MiB but nothing bounds the number of surfaces a committed registry may declare, so one abcd audit run over a hostile repo can be made to hold an unbounded multiple of that cap. Detector: a Config.Validate test that refuses a registry declaring more than a fixed surface count.
\ No newline at end of file
diff --git a/.abcd/work/issues/open/iss-150-a-committed-positioning-registry-may-name-a-surface-inside-g.md b/.abcd/work/issues/open/iss-150-a-committed-positioning-registry-may-name-a-surface-inside-g.md
new file mode 100644
index 0000000..dd356bd
--- /dev/null
+++ b/.abcd/work/issues/open/iss-150-a-committed-positioning-registry-may-name-a-surface-inside-g.md
@@ -0,0 +1,12 @@
+---
+schema_version: 1
+id: "iss-150"
+slug: "a-committed-positioning-registry-may-name-a-surface-inside-g"
+severity: "minor"
+category: "bug"
+source: "agent-finding"
+found_during: "itd-102-security-review"
+found_at: "internal/core/positioning/config.go"
+---
+
+A committed positioning registry may name a surface inside .git (ValidRelPath accepts .git/config and it is inside the containment root), so abcd identity --json quotes .git/config — including a credential-bearing remote URL — into SurfaceResult.Found. Pre-existing; not an escape, and the output reaches only the operator's own stdout. Detector: a Config.Validate test refusing surface candidates under .git/. Found during the itd-102 containment review.
\ No newline at end of file
+
+