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
45 changes: 41 additions & 4 deletions internal/obsidian/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,32 @@ func NewExporter(s StoreReader, cfg ExportConfig) *Exporter {
return &Exporter{store: s, config: cfg}
}

// sanitizePathComponent strips path separators and dot-dot sequences from a
// single path component (project name or observation type), preventing path
// traversal attacks when the value is used inside filepath.Join.
// Any slash, backslash, or ".." element is replaced with "_".
func sanitizePathComponent(s string) string {
// Replace OS separators with underscore.
s = strings.ReplaceAll(s, string(filepath.Separator), "_")
s = strings.ReplaceAll(s, "/", "_")
s = strings.ReplaceAll(s, "\\", "_")

// Replace ".." with "__" to neutralise traversal.
s = strings.ReplaceAll(s, "..", "__")

// filepath.Clean on a single component that contains no separators is a
// no-op, but run it anyway to normalise any remaining oddities (e.g. trailing
// dots on Windows). We re-apply the separator guard afterwards because Clean
// can introduce a leading "/" on absolute-looking inputs on Unix.
clean := filepath.Clean(s)
clean = strings.ReplaceAll(clean, string(filepath.Separator), "_")
clean = strings.ReplaceAll(clean, "/", "_")
if clean == "" || clean == "." {
clean = "_"
}
return clean
}

// SetGraphConfig sets the GraphConfig mode on this exporter's config.
// This is used by Watcher to force GraphConfigSkip on subsequent cycles (REQ-WATCH-06).
func (e *Exporter) SetGraphConfig(mode GraphConfigMode) {
Expand Down Expand Up @@ -189,16 +215,27 @@ func (e *Exporter) Export() (*ExportResult, error) {
}
}

// Determine target file path
// Determine target file path — sanitize project and type to prevent
// path traversal (issue #180): strip separators and dot-dot sequences,
// then verify the resolved absolute path stays inside engRoot.
project := "unknown"
if obs.Project != nil && *obs.Project != "" {
project = *obs.Project
project = sanitizePathComponent(*obs.Project)
}
obsType := sanitizePathComponent(obs.Type)
slug := Slugify(obs.Title, obs.ID)
relPath := filepath.Join(project, obs.Type, slug+".md")
absDir := filepath.Join(engRoot, project, obs.Type)
relPath := filepath.Join(project, obsType, slug+".md")
absDir := filepath.Join(engRoot, project, obsType)
absPath := filepath.Join(engRoot, relPath)

// Containment check: reject any path that escapes engRoot after cleaning.
cleanRoot := filepath.Clean(engRoot)
cleanPath := filepath.Clean(absPath)
if !strings.HasPrefix(cleanPath, cleanRoot+string(filepath.Separator)) {
result.Errors = append(result.Errors, fmt.Errorf("unsafe path rejected (would escape export root): %s", cleanPath))
Comment on lines +231 to +235
continue
}

// Create directory
if err := os.MkdirAll(absDir, 0755); err != nil {
result.Errors = append(result.Errors, fmt.Errorf("mkdir %s: %w", absDir, err))
Expand Down
116 changes: 116 additions & 0 deletions internal/obsidian/exporter_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package obsidian

import (
"path/filepath"
"strings"
"testing"

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

// ─── Security: TestPathTraversalPrevention (Issue #180) ──────────────────────

// TestPathTraversalPrevention asserts that malicious Project or Type values
// containing path traversal sequences (e.g. "../../etc") or absolute paths
// cannot produce output files that escape the {vault}/engram/ export root.
func TestPathTraversalPrevention(t *testing.T) {
traversalCases := []struct {
name string
project string
obsType string
}{
{
name: "dotdot in project",
project: "../../etc",
obsType: "bugfix",
},
{
name: "dotdot in type",
project: "myproject",
obsType: "../../etc",
},
{
name: "dotdot in both",
project: "../../tmp",
obsType: "../../../var",
},
{
name: "absolute path in project",
project: "/etc",
obsType: "bugfix",
},
{
name: "absolute path in type",
project: "myproject",
obsType: "/etc",
},
{
name: "nested traversal",
project: "foo/../../../secret",
obsType: "bugfix",
},
}

for _, tc := range traversalCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
dir := t.TempDir()

ms := &mockStore{
exportData: &store.ExportData{
Sessions: []store.Session{
{ID: "sess-1", Project: tc.project},
},
Observations: []store.Observation{
{
ID: 1,
SessionID: "sess-1",
Type: tc.obsType,
Title: "Traversal Test",
Content: "malicious content",
Scope: "project",
CreatedAt: "2026-01-01T10:00:00Z",
UpdatedAt: "2026-01-01T10:00:00Z",
Project: strPtr(tc.project),
},
},
Prompts: []store.Prompt{},
},
}

cfg := ExportConfig{VaultPath: dir}
exp := NewExporter(ms, cfg)
result, err := exp.Export()
if err != nil {
t.Fatalf("Export() returned unexpected error: %v", err)
}

// The observation must either be sanitized (created inside engRoot)
// or skipped due to invalid path — it must NOT escape the export root.
engRoot := dir + "/engram"

// Walk everything that was written and assert containment.
err = walkDir(dir, func(path string) {
// engRoot itself and state/hub files are fine — only check obs files.
if !isContainedIn(path, engRoot) {
t.Errorf("file written OUTSIDE export root: %s (engRoot=%s)", path, engRoot)
}
})
if err != nil {
t.Fatalf("walkDir: %v", err)
}

// If a file was created, verify its absolute clean path is under engRoot.
if result != nil && result.Created > 0 {
// Retrieve the state to find which relative path was recorded.
stateFile := engRoot + "/.engram-sync-state.json"
state, readErr := ReadState(stateFile)
if readErr != nil {
t.Fatalf("ReadState: %v", readErr)
}
for id, relPath := range state.Files {
absPath := filepath.Join(engRoot, relPath)
cleanAbs := filepath.Clean(absPath)
cleanRoot := filepath.Clean(engRoot)
if !strings.HasPrefix(cleanAbs, cleanRoot+string(filepath.Separator)) &&
cleanAbs != cleanRoot {
t.Errorf("obs %d: state path escapes export root: %s (engRoot=%s)", id, cleanAbs, cleanRoot)
}
}
}
})
}
}

// ─── Helpers ─────────────────────────────────────────────────────────────────

// buildPipelineFixtures creates a fixture ExportData with 3 projects, 5 sessions,
Expand Down
21 changes: 21 additions & 0 deletions internal/obsidian/testhelpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package obsidian
import (
"os"
"path/filepath"
"strings"
)

// mkdirAll creates the given directory path (all parents), used in tests.
Expand Down Expand Up @@ -48,3 +49,23 @@ func countFilesInDir(t interface {
}
return count
}

// walkDir walks all regular files under root and calls fn for each absolute path.
func walkDir(root string, fn func(path string)) error {
return filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if !info.IsDir() {
fn(path)
}
return nil
})
}

// isContainedIn reports whether path is inside (or equal to) root after cleaning.
func isContainedIn(path, root string) bool {
cleanPath := filepath.Clean(path)
cleanRoot := filepath.Clean(root)
return strings.HasPrefix(cleanPath, cleanRoot+string(filepath.Separator)) || cleanPath == cleanRoot
}