ingestion/internal/session/id_test.go

@@ -1,9 +1,12 @@
 package session
 
 import (
+	"errors"
 	"os"
 	"path/filepath"
+	"syscall"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -47,3 +50,43 @@ func TestReadRefusesSessionIDsThatAreNotPlainNames(t *testing.T) {
 	_, err := Read(filepath.Join(root, "sessions"), "../x")
 	assert.Error(t, err)
 }
+
+// A name that starts plainly and then climbs out with ".." is exactly what the
+// ban on "/" in the id's tail stops: filepath.Join cleans "a/../../x" to a
+// path beside the sessions dir without needing sessions/a to exist. The
+// assertion is on the refusal itself (ErrInvalidID), not on any error.
+func TestAppendRefusesAnIDThatClimbsOutAfterAPlainPrefix(t *testing.T) {
+	root := t.TempDir()
+	dir := filepath.Join(root, "sessions")
+	require.NoError(t, os.MkdirAll(dir, 0o755))
+	for _, id := range []string{"a/../../escape", "a/../../../escape", "a/../b"} {
+		err := Append(dir, id, Entry{SessionID: id})
+		assert.ErrorIs(t, err, ErrInvalidID, "%q", id)
+	}
+	_, err := os.Lstat(filepath.Join(root, "escape.jsonl"))
+	assert.True(t, errors.Is(err, os.ErrNotExist), "nothing created beside the sessions dir")
+	_, err = os.Lstat(filepath.Join(dir, "b.jsonl"))
+	assert.True(t, errors.Is(err, os.ErrNotExist), "no in-dir alias created either")
+}
+
+// Append does no Lstat type check, so O_NONBLOCK is the only thing that keeps
+// a FIFO named <id>.jsonl with no reader from blocking session_log forever.
+func TestAppendReturnsPromptlyOnAFIFOLogWithNoReader(t *testing.T) {
+	dir := filepath.Join(t.TempDir(), "sessions")
+	require.NoError(t, os.MkdirAll(dir, 0o755))
+	fifo := filepath.Join(dir, "s.jsonl")
+	require.NoError(t, syscall.Mkfifo(fifo, 0o644))
+	done := make(chan error, 1)
+	go func() { done <- Append(dir, "s", Entry{SessionID: "s"}) }()
+	select {
+	case err := <-done:
+		assert.Error(t, err, "a FIFO with no reader is refused, not written")
+	case <-time.After(3 * time.Second):
+		// Unblock the stuck open so the goroutine and TempDir cleanup finish.
+		if r, err := os.OpenFile(fifo, os.O_RDONLY|syscall.O_NONBLOCK, 0); err == nil {
+			defer r.Close() //nolint:errcheck
+		}
+		<-done
+		t.Fatal("Append blocked on a FIFO log with no reader")
+	}
+}

ingestion/internal/session/session.go

@@ -49,9 +49,12 @@ type Attempt struct {
 // write (hyperguild#169).
 var idPattern = regexp.MustCompile(`^[A-Za-z0-9_][A-Za-z0-9._-]{0,127}$`)
 
+// ErrInvalidID is returned when a session id is not a plain file name.
+var ErrInvalidID = errors.New("session id is not a plain name")
+
 func checkID(sessionID string) error {
 	if !idPattern.MatchString(sessionID) {
-		return fmt.Errorf("session id %q is not a plain name", sessionID)
+		return fmt.Errorf("%w: %q", ErrInvalidID, sessionID)
 	}
 	return nil
 }