fix(update,session): brain_update and session_log confine caller paths (#169)
Review round 1 on PR #170 found two more handlers with the #169 class of bug (verified by execution): - brain_update resolved its caller path with resolveWithin, a lexical containment check: any brain file could be superseded (classification.yaml, raw/, sessions/), and a final symlink was written through. UpdateNote now resolves like brain_edit (notes.Resolve, ScopeNotes: knowledge/ and wiki/ .md notes), and reads and rewrites through brainfs (ReadRegular, WriteRegular). - session_log joined the caller's session_id onto the sessions dir and opened it O_CREATE|O_APPEND: an id with path segments created or appended a .jsonl file anywhere the pod could write. The id must now be a plain name (^[A-Za-z0-9_][A-Za-z0-9._-]{0,127}$, which every live session file matches), and the open adds O_NOFOLLOW|O_NONBLOCK. Read checks the id too. - Tunnel tests: the .txt case now names an existing file (it failed on "missing" before), the symlink test adds an in-brain target, and the FIFO comment no longer claims the handle check is the only guard. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,49 @@
|
||||
package session
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// session_log passed the caller's session_id straight into
|
||||
// filepath.Join(sessionsDir, id+".jsonl") and opened it O_CREATE|O_APPEND:
|
||||
// path segments in the id created or appended files anywhere the pod could
|
||||
// write. The id is now a plain name, and the open does not follow a final
|
||||
// symlink.
|
||||
func TestAppendRefusesSessionIDsThatAreNotPlainNames(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
dir := filepath.Join(root, "sessions")
|
||||
for _, id := range []string{"../x", "a/b", "/abs", ".", "..", ".hidden", "", "a\\b", "a\x00b"} {
|
||||
assert.Error(t, Append(dir, id, Entry{SessionID: id}), "%q", id)
|
||||
}
|
||||
ents, _ := os.ReadDir(root)
|
||||
for _, e := range ents {
|
||||
assert.Equal(t, "sessions", e.Name(), "nothing created beside the sessions dir")
|
||||
}
|
||||
for _, id := range []string{"1557b2e1-bf0d-5c4f-bbe3-0754d087e356", "_routing", "2026-05-05-infra.plan", "0"} {
|
||||
require.NoError(t, Append(dir, id, Entry{SessionID: id}), "positive control: %q", id)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAppendDoesNotWriteThroughASymlinkedLog(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
dir := filepath.Join(root, "sessions")
|
||||
require.NoError(t, os.MkdirAll(dir, 0o755))
|
||||
victim := filepath.Join(root, "victim.md")
|
||||
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
|
||||
require.NoError(t, os.Symlink(filepath.Join("..", "victim.md"), filepath.Join(dir, "s.jsonl")))
|
||||
assert.Error(t, Append(dir, "s", Entry{SessionID: "s"}))
|
||||
got, _ := os.ReadFile(victim)
|
||||
assert.Equal(t, "untouched\n", string(got))
|
||||
}
|
||||
|
||||
func TestReadRefusesSessionIDsThatAreNotPlainNames(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
require.NoError(t, os.WriteFile(filepath.Join(root, "x.jsonl"), []byte(`{"session_id":"x"}`+"\n"), 0o644))
|
||||
_, err := Read(filepath.Join(root, "sessions"), "../x")
|
||||
assert.Error(t, err)
|
||||
}
|
||||
@@ -9,6 +9,8 @@ import (
|
||||
"io/fs"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"syscall"
|
||||
"time"
|
||||
)
|
||||
|
||||
@@ -42,13 +44,29 @@ type Attempt struct {
|
||||
RunnerOutput string `json:"runner_output,omitempty"`
|
||||
}
|
||||
|
||||
// idPattern is a plain file name: session_log takes the id from the caller,
|
||||
// and a path segment in it created or appended files anywhere the pod could
|
||||
// write (hyperguild#169).
|
||||
var idPattern = regexp.MustCompile(`^[A-Za-z0-9_][A-Za-z0-9._-]{0,127}$`)
|
||||
|
||||
func checkID(sessionID string) error {
|
||||
if !idPattern.MatchString(sessionID) {
|
||||
return fmt.Errorf("session id %q is not a plain name", sessionID)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// Append writes entry as a single JSON line to sessionsDir/{sessionID}.jsonl.
|
||||
// The open does not follow a final symlink.
|
||||
func Append(sessionsDir, sessionID string, entry Entry) error {
|
||||
if err := checkID(sessionID); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := os.MkdirAll(sessionsDir, 0o755); err != nil {
|
||||
return fmt.Errorf("create sessions dir: %w", err)
|
||||
}
|
||||
path := filepath.Join(sessionsDir, sessionID+".jsonl")
|
||||
f, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644)
|
||||
f, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0o644)
|
||||
if err != nil {
|
||||
return fmt.Errorf("open session log: %w", err)
|
||||
}
|
||||
@@ -70,6 +88,9 @@ func Append(sessionsDir, sessionID string, entry Entry) error {
|
||||
|
||||
// Read returns all entries for sessionID. Returns empty slice if no log exists.
|
||||
func Read(sessionsDir, sessionID string) ([]Entry, error) {
|
||||
if err := checkID(sessionID); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
path := filepath.Join(sessionsDir, sessionID+".jsonl")
|
||||
f, err := os.Open(path)
|
||||
if errors.Is(err, fs.ErrNotExist) {
|
||||
|
||||
Reference in New Issue
Block a user