Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,22 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht

## [Unreleased]

### Added

- Recognize a payload projection: a symlink whose resolved target stays inside
the repository and lands on the `template/` file backing that exact path. A
repository that owns the payload is not a downstream installation of itself,
so it may reference generic documents instead of copying them. `doctor` then
inspects the payload the link resolves to, instead of reporting
`manifest.managed_unreadable` and `governance.unsafe_symlink` for every such
document.

Recognition requires the repository to actually carry a `template/memory-bank`
tree. Symlinks that leave the repository root, or point at anything other than
the payload file backing the path, remain unsafe and are reported exactly as
before. Directory traversal in `lint` and `doctor` is unchanged, and `init`,
`pull` and the ownership lock are untouched.

## [2.2.0] - 2026-08-15

### Changed
Expand Down
26 changes: 21 additions & 5 deletions internal/doctor/doctor.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/dapi/memory-bank-cli/internal/lint"
"github.com/dapi/memory-bank-cli/internal/ownership"
"github.com/dapi/memory-bank-cli/internal/projection"
"gopkg.in/yaml.v3"
)

Expand Down Expand Up @@ -140,7 +141,7 @@ func (report *Report) checkIdentityAndDrift(agentFile, scopeRoot string) {
}
report.add(Finding{Code: "agent.entrypoint_missing", Severity: severity, Group: "agent_integration", Path: agentFile, Message: message, Remediation: fmt.Sprintf("Create the agent instruction file and link it to %s/README.md.", scopeRoot)})
} else if !strings.Contains(string(contents), scopeRoot+"/README.md") {
report.add(Finding{Code: "agent.memory_bank_link_missing", Severity: Error, Group: "agent_integration", Path: agentFile, Message: fmt.Sprintf("Agent instructions do not route readers to %s/README.md.", scopeRoot), Remediation: fmt.Sprintf("Add a repository-relative link to %s/README.md or run memory-bank-cli pull with the same --agent-file.", scopeRoot)})
report.add(Finding{Code: "agent.memory_bank_link_missing", Severity: Error, Group: "agent_integration", Path: agentFile, Message: fmt.Sprintf("Agent instructions do not route readers to %s/README.md.", scopeRoot), Remediation: fmt.Sprintf("Add a repository-relative link to %s/README.md or run memory-bank-cli pull with the same --agent-file.", scopeRoot)})
}
if exists && lockErr == nil {
agentReport, err := ownership.InspectAgentInstructions(report.RepoRoot, agentFile)
Expand Down Expand Up @@ -170,12 +171,12 @@ func (report *Report) checkManagedDrift(lock ownership.Lock) {
if os.IsNotExist(err) {
code, message = "manifest.managed_missing", "Managed file recorded by the lock is missing."
}
report.add(Finding{Code: code, Severity: Error, Group: "manifest", Path: filePath, Message: message, Remediation: "Restore the file from the pinned template with memory-bank-cli pull."})
report.add(Finding{Code: code, Severity: Error, Group: "manifest", Path: filePath, Message: message, Remediation: "Restore the file from the pinned template with memory-bank-cli pull."})
continue
}
digest := fmt.Sprintf("sha256:%x", sha256.Sum256(data))
if digest != contract.PayloadDigest {
report.add(Finding{Code: "manifest.managed_content_drift", Severity: Error, Group: "manifest", Path: filePath, Message: "Managed file content differs from the lock payload digest.", Remediation: "Review the local change, then restore it through memory-bank-cli pull."})
report.add(Finding{Code: "manifest.managed_content_drift", Severity: Error, Group: "manifest", Path: filePath, Message: "Managed file content differs from the lock payload digest.", Remediation: "Review the local change, then restore it through memory-bank-cli pull."})
}
mode := observedPayloadMode(info.Mode().Perm())
if contract.PayloadMode != "" && mode != "" && mode != contract.PayloadMode {
Expand Down Expand Up @@ -209,17 +210,32 @@ func readRegularWithinRoot(repoRoot, relativePath string) ([]byte, fs.FileInfo,
return nil, nil, err
}
if info.Mode()&os.ModeSymlink != 0 {
// A template source repository may project its own payload instead
// of copying it. Such a link resolves inside the repository onto
// the payload file backing this exact path, so reading through it
// yields the payload itself. Any other symlink stays unsafe.
//
// Read the path Resolve verified rather than walking the link
// again: a second traversal could follow a link re-pointed in the
// meantime.
if resolved, projected := projection.Resolve(repoRoot, relativePath); projected {
return readResolvedRegular(resolved, relativePath)
}
return nil, nil, fmt.Errorf("unsafe symlink in path %q", relativePath)
}
}
info, err := os.Stat(current)
return readResolvedRegular(current, relativePath)
}

func readResolvedRegular(path, relativePath string) ([]byte, fs.FileInfo, error) {
info, err := os.Stat(path)
if err != nil {
return nil, nil, err
}
if !info.Mode().IsRegular() {
return nil, info, fmt.Errorf("path %q is not a regular file", relativePath)
}
data, err := os.ReadFile(current)
data, err := os.ReadFile(path)
return data, info, err
}

Expand Down
43 changes: 43 additions & 0 deletions internal/doctor/doctor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -789,3 +789,46 @@ func hasFinding(report Report, code string) bool {
}
return false
}

func writeProjectionFixture(t *testing.T, root, relative, contents string) {
t.Helper()
full := filepath.Join(root, filepath.FromSlash(relative))
if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(full, []byte(contents), 0o644); err != nil {
t.Fatal(err)
}
}

// A governed document that links outside the repository, or dangles, is what
// the unsafe-symlink finding exists for. Walking past it silently would remove
// the check the projection support was supposed to leave intact.
func TestGovernanceStillReportsUnsafeGovernedSymlinks(t *testing.T) {
repo, outside := t.TempDir(), t.TempDir()
writeProjectionFixture(t, outside, "evil.md", "---\nstatus: active\n---\n")
if err := os.MkdirAll(filepath.Join(repo, "memory-bank", "flows"), 0o755); err != nil {
t.Fatal(err)
}
if err := os.Symlink(filepath.Join(outside, "evil.md"), filepath.Join(repo, "memory-bank", "flows", "evil.md")); err != nil {
t.Skipf("symlinks are unavailable: %v", err)
}
if err := os.Symlink(filepath.Join("..", "missing.md"), filepath.Join(repo, "memory-bank", "flows", "dangling.md")); err != nil {
t.Skipf("symlinks are unavailable: %v", err)
}

report := Report{RepoRoot: repo, Findings: []Finding{}}
report.checkGovernance("memory-bank")

found := map[string]bool{}
for _, finding := range report.Findings {
if finding.Code == "governance.unsafe_symlink" {
found[finding.Path] = true
}
}
for _, expected := range []string{"memory-bank/flows/evil.md", "memory-bank/flows/dangling.md"} {
if !found[expected] {
t.Fatalf("no unsafe_symlink finding for %s: %#v", expected, report.Findings)
}
}
}
16 changes: 13 additions & 3 deletions internal/doctor/governance.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"strings"

"github.com/dapi/memory-bank-cli/internal/lint"
"github.com/dapi/memory-bank-cli/internal/projection"
"gopkg.in/yaml.v3"
)

Expand Down Expand Up @@ -46,11 +47,20 @@ func (report *Report) checkGovernance(scopeRoot string) {
return err
}
documentPath := filepath.ToSlash(relative)
readPath := fullPath
if entry.Type()&os.ModeSymlink != 0 {
report.add(Finding{Code: "governance.unsafe_symlink", Severity: Error, Group: "frontmatter_governance", Path: documentPath, Message: "Governed document is a symlink.", Remediation: "Replace it with a regular repository-owned Markdown file."})
return nil
// A template source repository may project a governed document from
// its own payload. The document behind such a link is the payload
// itself, so it is governed exactly as the payload is. Read the path
// the check verified rather than walking the link again.
resolved, projected := projection.Resolve(report.RepoRoot, documentPath)
if !projected {
report.add(Finding{Code: "governance.unsafe_symlink", Severity: Error, Group: "frontmatter_governance", Path: documentPath, Message: "Governed document is a symlink.", Remediation: "Replace it with a regular repository-owned Markdown file, or point it at the payload file this path installs from."})
return nil
}
readPath = resolved
}
data, err := os.ReadFile(fullPath)
data, err := os.ReadFile(readPath)
if err != nil {
return err
}
Expand Down
94 changes: 94 additions & 0 deletions internal/projection/projection.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
// Package projection recognises an intentional payload projection: a symlink
// inside a template source repository that points at the repository's own
// payload instead of a copy of it.
//
// A repository that owns template/ is not a downstream installation of itself.
// Duplicating the payload into the installed tree buys nothing there, so such a
// repository may represent generic documents as symlinks into template/. The
// symlink then equals the payload by construction and cannot drift.
//
// This is deliberately narrow. The symlink guards elsewhere in this CLI exist to
// stop a write from escaping the repository root through a redirected path, and
// that protection is unchanged: a link is only a projection when it resolves
// inside the same repository AND lands exactly on the payload file that backs
// the destination path. Everything else stays an unsafe path.
package projection

import (
"os"
"path/filepath"
"strings"

"github.com/dapi/memory-bank-cli/internal/ownership"
)

// PayloadRoot is the tracked payload tree of a template source repository, so a
// destination path X is backed by PayloadRoot/X. It deliberately reuses the
// canonical constant: two definitions of a security-relevant path would let a
// rename silently turn every projection back into an unsafe symlink.
const PayloadRoot = ownership.CanonicalTemplateRoot

// Resolve reports whether the repository-relative destination path resolves,
// through one or more symlinks, to the payload file that backs it, and returns
// that resolved payload path.
//
// The returned path is what the destination actually reads. A caller must not
// assume it matches an incoming source payload: the local payload can be older
// than the source being installed, and only comparing content can tell.
//
// It returns false rather than an error for the ordinary negative cases — a
// regular file, a broken link, a missing payload counterpart — because callers
// use this to decide between two legitimate behaviours, not to detect faults.
func Resolve(repoRoot, relative string) (string, bool) {
if repoRoot == "" || relative == "" {
return "", false
}
osRelative := filepath.FromSlash(relative)
if !filepath.IsLocal(osRelative) {
return "", false
}

// Only a repository that actually carries a payload tree can project from
// it. Without this, an unrelated directory named template/ in a downstream
// repository would be enough to accept a symlink where the lock expects a
// regular managed file.
if info, err := os.Stat(filepath.Join(repoRoot, PayloadRoot, ownership.DownstreamPayloadRoot)); err != nil || !info.IsDir() {
return "", false
}
resolvedRoot, err := filepath.EvalSymlinks(repoRoot)
if err != nil {
return "", false
}
destination, err := filepath.EvalSymlinks(filepath.Join(repoRoot, osRelative))
if err != nil {
return "", false
}
payload, err := filepath.EvalSymlinks(filepath.Join(repoRoot, PayloadRoot, osRelative))
if err != nil {
return "", false
}
if destination != payload || !within(resolvedRoot, destination) {
return "", false
}
return destination, true
}

// IsPayloadProjection reports whether the destination path is a projection of
// the payload file backing it.
func IsPayloadProjection(repoRoot, relative string) bool {
_, ok := Resolve(repoRoot, relative)
return ok
}

// within reports whether target is the root itself or lives below it. Both
// paths must already be resolved, so no symlink can move the target afterwards.
func within(root, target string) bool {
relative, err := filepath.Rel(root, target)
if err != nil {
return false
}
if relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) {
return false
}
return true
}
96 changes: 96 additions & 0 deletions internal/projection/projection_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package projection

import (
"os"
"path/filepath"
"testing"
)

func symlinkForTest(t *testing.T, target, link string) {
t.Helper()
if err := os.MkdirAll(filepath.Dir(link), 0o755); err != nil {
t.Fatal(err)
}
if err := os.Symlink(target, link); err != nil {
t.Skipf("symlinks are unavailable: %v", err)
}
}

func writeForTest(t *testing.T, root, relative, contents string) string {
t.Helper()
full := filepath.Join(root, filepath.FromSlash(relative))
if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(full, []byte(contents), 0o644); err != nil {
t.Fatal(err)
}
return full
}

func TestRecognisesFileProjection(t *testing.T) {
repo := t.TempDir()
writeForTest(t, repo, "template/memory-bank/dna/rule.md", "payload\n")
symlinkForTest(t, "../../template/memory-bank/dna/rule.md", filepath.Join(repo, "memory-bank/dna/rule.md"))

if !IsPayloadProjection(repo, "memory-bank/dna/rule.md") {
t.Fatal("symlink into the repository's own payload must be recognised as a projection")
}
}

func TestRecognisesProjectionThroughDirectorySymlink(t *testing.T) {
repo := t.TempDir()
writeForTest(t, repo, "template/memory-bank/flows/routing.md", "payload\n")
symlinkForTest(t, "../template/memory-bank/flows", filepath.Join(repo, "memory-bank/flows"))

if !IsPayloadProjection(repo, "memory-bank/flows/routing.md") {
t.Fatal("a directory symlink into the payload projects the files below it")
}
}

func TestRejectsSymlinkEscapingTheRepository(t *testing.T) {
repo, outside := t.TempDir(), t.TempDir()
writeForTest(t, repo, "template/memory-bank/dna/rule.md", "payload\n")
writeForTest(t, outside, "rule.md", "payload\n")
symlinkForTest(t, filepath.Join(outside, "rule.md"), filepath.Join(repo, "memory-bank/dna/rule.md"))

if IsPayloadProjection(repo, "memory-bank/dna/rule.md") {
t.Fatal("a link leaving the repository root must never count as a projection")
}
}

func TestRejectsSymlinkToADifferentPayloadFile(t *testing.T) {
repo := t.TempDir()
writeForTest(t, repo, "template/memory-bank/dna/rule.md", "payload\n")
writeForTest(t, repo, "template/memory-bank/dna/other.md", "payload\n")
symlinkForTest(t, "../../template/memory-bank/dna/other.md", filepath.Join(repo, "memory-bank/dna/rule.md"))

if IsPayloadProjection(repo, "memory-bank/dna/rule.md") {
t.Fatal("only the payload file backing this exact destination is a projection")
}
}

func TestRejectsRegularFileAndBrokenLink(t *testing.T) {
repo := t.TempDir()
writeForTest(t, repo, "template/memory-bank/dna/rule.md", "payload\n")
writeForTest(t, repo, "memory-bank/dna/rule.md", "local override\n")
symlinkForTest(t, "../../template/memory-bank/dna/missing.md", filepath.Join(repo, "memory-bank/dna/broken.md"))

if IsPayloadProjection(repo, "memory-bank/dna/rule.md") {
t.Fatal("a regular file is an override, not a projection")
}
if IsPayloadProjection(repo, "memory-bank/dna/broken.md") {
t.Fatal("a broken link is not a projection")
}
}

func TestRejectsPathsOutsideTheRepositoryRoot(t *testing.T) {
repo := t.TempDir()
writeForTest(t, repo, "template/memory-bank/dna/rule.md", "payload\n")

for _, relative := range []string{"", "../escape.md", "/absolute.md"} {
if IsPayloadProjection(repo, relative) {
t.Fatalf("path %q must not be treated as a projection", relative)
}
}
}
Loading