Skip to content

Commit d0e7e21

Browse files
fix(obsidian): prevent path traversal in exporter file path construction (#423)
Sanitize project and type path components before filepath.Join to strip separators and dot-dot sequences, and add a post-join containment check that rejects any resolved path escaping the vault/engram root (issue #180).
1 parent e840ea2 commit d0e7e21

3 files changed

Lines changed: 178 additions & 4 deletions

File tree

internal/obsidian/exporter.go

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,32 @@ func NewExporter(s StoreReader, cfg ExportConfig) *Exporter {
3838
return &Exporter{store: s, config: cfg}
3939
}
4040

41+
// sanitizePathComponent strips path separators and dot-dot sequences from a
42+
// single path component (project name or observation type), preventing path
43+
// traversal attacks when the value is used inside filepath.Join.
44+
// Any slash, backslash, or ".." element is replaced with "_".
45+
func sanitizePathComponent(s string) string {
46+
// Replace OS separators with underscore.
47+
s = strings.ReplaceAll(s, string(filepath.Separator), "_")
48+
s = strings.ReplaceAll(s, "/", "_")
49+
s = strings.ReplaceAll(s, "\\", "_")
50+
51+
// Replace ".." with "__" to neutralise traversal.
52+
s = strings.ReplaceAll(s, "..", "__")
53+
54+
// filepath.Clean on a single component that contains no separators is a
55+
// no-op, but run it anyway to normalise any remaining oddities (e.g. trailing
56+
// dots on Windows). We re-apply the separator guard afterwards because Clean
57+
// can introduce a leading "/" on absolute-looking inputs on Unix.
58+
clean := filepath.Clean(s)
59+
clean = strings.ReplaceAll(clean, string(filepath.Separator), "_")
60+
clean = strings.ReplaceAll(clean, "/", "_")
61+
if clean == "" || clean == "." {
62+
clean = "_"
63+
}
64+
return clean
65+
}
66+
4167
// SetGraphConfig sets the GraphConfig mode on this exporter's config.
4268
// This is used by Watcher to force GraphConfigSkip on subsequent cycles (REQ-WATCH-06).
4369
func (e *Exporter) SetGraphConfig(mode GraphConfigMode) {
@@ -189,16 +215,27 @@ func (e *Exporter) Export() (*ExportResult, error) {
189215
}
190216
}
191217

192-
// Determine target file path
218+
// Determine target file path — sanitize project and type to prevent
219+
// path traversal (issue #180): strip separators and dot-dot sequences,
220+
// then verify the resolved absolute path stays inside engRoot.
193221
project := "unknown"
194222
if obs.Project != nil && *obs.Project != "" {
195-
project = *obs.Project
223+
project = sanitizePathComponent(*obs.Project)
196224
}
225+
obsType := sanitizePathComponent(obs.Type)
197226
slug := Slugify(obs.Title, obs.ID)
198-
relPath := filepath.Join(project, obs.Type, slug+".md")
199-
absDir := filepath.Join(engRoot, project, obs.Type)
227+
relPath := filepath.Join(project, obsType, slug+".md")
228+
absDir := filepath.Join(engRoot, project, obsType)
200229
absPath := filepath.Join(engRoot, relPath)
201230

231+
// Containment check: reject any path that escapes engRoot after cleaning.
232+
cleanRoot := filepath.Clean(engRoot)
233+
cleanPath := filepath.Clean(absPath)
234+
if !strings.HasPrefix(cleanPath, cleanRoot+string(filepath.Separator)) {
235+
result.Errors = append(result.Errors, fmt.Errorf("unsafe path rejected (would escape export root): %s", cleanPath))
236+
continue
237+
}
238+
202239
// Create directory
203240
if err := os.MkdirAll(absDir, 0755); err != nil {
204241
result.Errors = append(result.Errors, fmt.Errorf("mkdir %s: %w", absDir, err))

internal/obsidian/exporter_test.go

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package obsidian
22

33
import (
4+
"path/filepath"
5+
"strings"
46
"testing"
57

68
"github.com/Gentleman-Programming/engram/internal/store"
@@ -546,6 +548,120 @@ func TestExporterCallsGraphConfig(t *testing.T) {
546548
})
547549
}
548550

551+
// ─── Security: TestPathTraversalPrevention (Issue #180) ──────────────────────
552+
553+
// TestPathTraversalPrevention asserts that malicious Project or Type values
554+
// containing path traversal sequences (e.g. "../../etc") or absolute paths
555+
// cannot produce output files that escape the {vault}/engram/ export root.
556+
func TestPathTraversalPrevention(t *testing.T) {
557+
traversalCases := []struct {
558+
name string
559+
project string
560+
obsType string
561+
}{
562+
{
563+
name: "dotdot in project",
564+
project: "../../etc",
565+
obsType: "bugfix",
566+
},
567+
{
568+
name: "dotdot in type",
569+
project: "myproject",
570+
obsType: "../../etc",
571+
},
572+
{
573+
name: "dotdot in both",
574+
project: "../../tmp",
575+
obsType: "../../../var",
576+
},
577+
{
578+
name: "absolute path in project",
579+
project: "/etc",
580+
obsType: "bugfix",
581+
},
582+
{
583+
name: "absolute path in type",
584+
project: "myproject",
585+
obsType: "/etc",
586+
},
587+
{
588+
name: "nested traversal",
589+
project: "foo/../../../secret",
590+
obsType: "bugfix",
591+
},
592+
}
593+
594+
for _, tc := range traversalCases {
595+
tc := tc
596+
t.Run(tc.name, func(t *testing.T) {
597+
dir := t.TempDir()
598+
599+
ms := &mockStore{
600+
exportData: &store.ExportData{
601+
Sessions: []store.Session{
602+
{ID: "sess-1", Project: tc.project},
603+
},
604+
Observations: []store.Observation{
605+
{
606+
ID: 1,
607+
SessionID: "sess-1",
608+
Type: tc.obsType,
609+
Title: "Traversal Test",
610+
Content: "malicious content",
611+
Scope: "project",
612+
CreatedAt: "2026-01-01T10:00:00Z",
613+
UpdatedAt: "2026-01-01T10:00:00Z",
614+
Project: strPtr(tc.project),
615+
},
616+
},
617+
Prompts: []store.Prompt{},
618+
},
619+
}
620+
621+
cfg := ExportConfig{VaultPath: dir}
622+
exp := NewExporter(ms, cfg)
623+
result, err := exp.Export()
624+
if err != nil {
625+
t.Fatalf("Export() returned unexpected error: %v", err)
626+
}
627+
628+
// The observation must either be sanitized (created inside engRoot)
629+
// or skipped due to invalid path — it must NOT escape the export root.
630+
engRoot := dir + "/engram"
631+
632+
// Walk everything that was written and assert containment.
633+
err = walkDir(dir, func(path string) {
634+
// engRoot itself and state/hub files are fine — only check obs files.
635+
if !isContainedIn(path, engRoot) {
636+
t.Errorf("file written OUTSIDE export root: %s (engRoot=%s)", path, engRoot)
637+
}
638+
})
639+
if err != nil {
640+
t.Fatalf("walkDir: %v", err)
641+
}
642+
643+
// If a file was created, verify its absolute clean path is under engRoot.
644+
if result != nil && result.Created > 0 {
645+
// Retrieve the state to find which relative path was recorded.
646+
stateFile := engRoot + "/.engram-sync-state.json"
647+
state, readErr := ReadState(stateFile)
648+
if readErr != nil {
649+
t.Fatalf("ReadState: %v", readErr)
650+
}
651+
for id, relPath := range state.Files {
652+
absPath := filepath.Join(engRoot, relPath)
653+
cleanAbs := filepath.Clean(absPath)
654+
cleanRoot := filepath.Clean(engRoot)
655+
if !strings.HasPrefix(cleanAbs, cleanRoot+string(filepath.Separator)) &&
656+
cleanAbs != cleanRoot {
657+
t.Errorf("obs %d: state path escapes export root: %s (engRoot=%s)", id, cleanAbs, cleanRoot)
658+
}
659+
}
660+
}
661+
})
662+
}
663+
}
664+
549665
// ─── Helpers ─────────────────────────────────────────────────────────────────
550666

551667
// buildPipelineFixtures creates a fixture ExportData with 3 projects, 5 sessions,

internal/obsidian/testhelpers_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package obsidian
33
import (
44
"os"
55
"path/filepath"
6+
"strings"
67
)
78

89
// mkdirAll creates the given directory path (all parents), used in tests.
@@ -48,3 +49,23 @@ func countFilesInDir(t interface {
4849
}
4950
return count
5051
}
52+
53+
// walkDir walks all regular files under root and calls fn for each absolute path.
54+
func walkDir(root string, fn func(path string)) error {
55+
return filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
56+
if err != nil {
57+
return err
58+
}
59+
if !info.IsDir() {
60+
fn(path)
61+
}
62+
return nil
63+
})
64+
}
65+
66+
// isContainedIn reports whether path is inside (or equal to) root after cleaning.
67+
func isContainedIn(path, root string) bool {
68+
cleanPath := filepath.Clean(path)
69+
cleanRoot := filepath.Clean(root)
70+
return strings.HasPrefix(cleanPath, cleanRoot+string(filepath.Separator)) || cleanPath == cleanRoot
71+
}

0 commit comments

Comments
 (0)