Skip to content

Commit bce7629

Browse files
committed
fix(init): refuse symlinked parent components under .mdsmith/
The prior guard only refused a symlinked top-level .mdsmith; an intermediate symlink (.mdsmith/wordlists -> /tmp/out) would still let MkdirAll/WriteFile resolve a pack file out of tree. refuseSymlinkedParents now Lstats every existing component from .mdsmith down to the file's parent and refuses any symlink before creating or writing anything. Covered by TestRefuseSymlinkedParents and TestWriteScaffolds_RefusesSymlinkedIntermediateDir. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012fET55utKej7P3VWhiNnLW
1 parent aed94cd commit bce7629

2 files changed

Lines changed: 92 additions & 14 deletions

File tree

cmd/mdsmith/main.go

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -460,19 +460,17 @@ func applyPacks(names []string, w io.Writer) error {
460460
// Existence is checked with statTarget, which refuses a symlinked target
461461
// rather than following it — a planted link must not divert the write
462462
// outside the pack directory. Each path is validated against the pack
463-
// contract (relative, under .mdsmith/) first, and a symlinked .mdsmith
464-
// directory is refused up front, so a buggy or hostile pack cannot write
465-
// out of tree. Progress lines go to w.
463+
// contract (relative, under .mdsmith/) and every existing parent
464+
// component is checked for a symlink first, so a buggy or hostile pack
465+
// cannot write out of tree. Progress lines go to w.
466466
func writeScaffolds(files []pack.File, w io.Writer) error {
467-
// A symlinked .mdsmith directory would let MkdirAll/WriteFile follow it
468-
// and land pack files wherever it points; refuse it before any write.
469-
if info, err := os.Lstat(mdsmithDir); err == nil && info.Mode()&fs.ModeSymlink != 0 {
470-
return fmt.Errorf("%s is a symlink; refusing to write pack files through it", mdsmithDir)
471-
}
472467
for _, f := range files {
473468
if err := validatePackPath(f.Path); err != nil {
474469
return err
475470
}
471+
if err := refuseSymlinkedParents(f.Path); err != nil {
472+
return err
473+
}
476474
if dir := filepath.Dir(f.Path); dir != "." {
477475
if err := os.MkdirAll(dir, 0o755); err != nil {
478476
return fmt.Errorf("creating %s: %w", dir, err)
@@ -494,6 +492,35 @@ func writeScaffolds(files []pack.File, w io.Writer) error {
494492
return nil
495493
}
496494

495+
// refuseSymlinkedParents fails if any existing directory component of p —
496+
// from the top-level .mdsmith down to its immediate parent — is a
497+
// symlink. A symlinked component (.mdsmith itself, or an intermediate
498+
// like .mdsmith/wordlists -> /tmp/out) would let the following MkdirAll
499+
// and WriteFile resolve p to a location outside .mdsmith/, so pack writes
500+
// must refuse it before touching the filesystem. Components that do not
501+
// exist yet are fine: MkdirAll creates them as real directories, and once
502+
// a component is absent everything below it is absent too.
503+
func refuseSymlinkedParents(p string) error {
504+
dir := filepath.Dir(p)
505+
if dir == "." {
506+
return nil
507+
}
508+
components := strings.Split(dir, string(filepath.Separator))
509+
for i := range components {
510+
prefix := filepath.Join(components[:i+1]...)
511+
info, err := os.Lstat(prefix)
512+
switch {
513+
case errors.Is(err, fs.ErrNotExist):
514+
return nil
515+
case err != nil:
516+
return fmt.Errorf("checking %s: %w", prefix, err)
517+
case info.Mode()&fs.ModeSymlink != 0:
518+
return fmt.Errorf("%s is a symlink; refusing to write pack files through it", prefix)
519+
}
520+
}
521+
return nil
522+
}
523+
497524
// mdsmithDir is the workspace subdirectory every additive pack writes
498525
// under; validatePackPath keeps pack files confined to it.
499526
const mdsmithDir = ".mdsmith"

cmd/mdsmith/main_unit_test.go

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1269,8 +1269,8 @@ func TestRunInit_Add_UnknownPack_ExitsTwo(t *testing.T) {
12691269
func TestRunInit_Add_ScaffoldError(t *testing.T) {
12701270
dir := t.TempDir()
12711271
t.Chdir(dir)
1272-
// .mdsmith is a regular file, so the pack's MkdirAll fails and runInit
1273-
// must surface it as exit 2.
1272+
// .mdsmith is a regular file, so the pack's parent-directory checks
1273+
// fail and runInit must surface the scaffold error as exit 2.
12741274
require.NoError(t, os.WriteFile(".mdsmith", []byte("x"), 0o644))
12751275

12761276
captureStderr(func() {
@@ -1344,14 +1344,18 @@ func TestWriteScaffolds_WritesAndSkips(t *testing.T) {
13441344
func TestWriteScaffolds_MkdirError(t *testing.T) {
13451345
dir := t.TempDir()
13461346
t.Chdir(dir)
1347-
// .mdsmith is a regular file, so MkdirAll(.mdsmith/wordlists) fails
1348-
// with ENOTDIR — driving the directory-creation error branch.
1349-
require.NoError(t, os.WriteFile(".mdsmith", []byte("x"), 0o644))
1347+
// .mdsmith is a real directory, but .mdsmith/wordlists is a regular
1348+
// file. The parent chain has no symlink, so refuseSymlinkedParents
1349+
// passes and MkdirAll(.mdsmith/wordlists) then fails because the target
1350+
// already exists as a file — driving the directory-creation error
1351+
// branch.
1352+
require.NoError(t, os.Mkdir(".mdsmith", 0o755))
1353+
require.NoError(t, os.WriteFile(filepath.Join(".mdsmith", "wordlists"), []byte("x"), 0o644))
13501354

13511355
files := []pack.File{{Path: filepath.Join(".mdsmith", "wordlists", "a.yaml"), Data: []byte("entries:\n - x\n")}}
13521356
err := writeScaffolds(files, io.Discard)
13531357
require.Error(t, err)
1354-
assert.Contains(t, err.Error(), filepath.Join(".mdsmith", "wordlists"))
1358+
assert.Contains(t, err.Error(), "creating "+filepath.Join(".mdsmith", "wordlists"))
13551359
}
13561360

13571361
func TestWriteScaffolds_RefusesSymlink(t *testing.T) {
@@ -1482,6 +1486,53 @@ func TestWriteScaffolds_RefusesSymlinkedMdsmithDir(t *testing.T) {
14821486
assert.True(t, os.IsNotExist(statErr), "nothing written through the symlinked .mdsmith")
14831487
}
14841488

1489+
func TestWriteScaffolds_RefusesSymlinkedIntermediateDir(t *testing.T) {
1490+
dir := t.TempDir()
1491+
t.Chdir(dir)
1492+
require.NoError(t, os.Mkdir(".mdsmith", 0o755))
1493+
target := filepath.Join(dir, "elsewhere")
1494+
require.NoError(t, os.Mkdir(target, 0o755))
1495+
// .mdsmith is a real directory, but an intermediate component
1496+
// (.mdsmith/wordlists) is a symlink out of tree. The write must still
1497+
// be refused rather than following it.
1498+
require.NoError(t, os.Symlink(target, filepath.Join(".mdsmith", "wordlists")))
1499+
1500+
files := []pack.File{{Path: filepath.Join(".mdsmith", "wordlists", "a.yaml"), Data: []byte("x")}}
1501+
err := writeScaffolds(files, io.Discard)
1502+
require.Error(t, err)
1503+
assert.Contains(t, err.Error(), "symlink")
1504+
_, statErr := os.Stat(filepath.Join(target, "a.yaml"))
1505+
assert.True(t, os.IsNotExist(statErr), "nothing written through the intermediate symlink")
1506+
}
1507+
1508+
func TestRefuseSymlinkedParents(t *testing.T) {
1509+
dir := t.TempDir()
1510+
t.Chdir(dir)
1511+
1512+
// A path whose parent is the cwd (dir == ".") has nothing to check.
1513+
require.NoError(t, refuseSymlinkedParents("a.yaml"))
1514+
1515+
// Parents that don't exist yet are fine — MkdirAll makes them real.
1516+
require.NoError(t, refuseSymlinkedParents(filepath.Join(".mdsmith", "wordlists", "a.yaml")))
1517+
1518+
// A fully real parent chain passes.
1519+
require.NoError(t, os.MkdirAll(filepath.Join(".mdsmith", "wordlists"), 0o755))
1520+
require.NoError(t, refuseSymlinkedParents(filepath.Join(".mdsmith", "wordlists", "a.yaml")))
1521+
1522+
// A symlinked component is refused.
1523+
require.NoError(t, os.Symlink(dir, filepath.Join(".mdsmith", "link")))
1524+
err := refuseSymlinkedParents(filepath.Join(".mdsmith", "link", "a.yaml"))
1525+
require.Error(t, err)
1526+
assert.Contains(t, err.Error(), "symlink")
1527+
1528+
// A non-ENOENT lstat error — a component is a file, so ENOTDIR — is
1529+
// surfaced as a checking error.
1530+
require.NoError(t, os.WriteFile(filepath.Join(".mdsmith", "afile"), []byte("x"), 0o644))
1531+
err = refuseSymlinkedParents(filepath.Join(".mdsmith", "afile", "child", "a.yaml"))
1532+
require.Error(t, err)
1533+
assert.Contains(t, err.Error(), "checking")
1534+
}
1535+
14851536
// --- runHelp ---
14861537

14871538
func TestRunHelp_NoArgs_ExitsZero(t *testing.T) {

0 commit comments

Comments
 (0)