fix: session security and correctness — path traversal, turn count restore, incognito quality leak
- store: validate session ID against store root to block path traversal in Load/Save - local: seed turnCount from LocalConfig.TurnCount so resumed sessions keep correct turn count - main: pass TurnCount from snapshot to LocalConfig on resume - main: suppress quality.json save when --incognito is active - main: handle UserConfigDir error in quality save defer instead of silently using wrong path - test: add TestSessionStore_Load/Save_RejectsPathTraversal
This commit is contained in:
@@ -196,14 +196,21 @@ func main() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Save QualityTracker data on exit (best-effort)
|
// Save QualityTracker data on exit (best-effort, suppressed in incognito)
|
||||||
defer func() {
|
defer func() {
|
||||||
|
if *incognito {
|
||||||
|
return
|
||||||
|
}
|
||||||
snap := rtr.QualityTracker().Snapshot()
|
snap := rtr.QualityTracker().Snapshot()
|
||||||
data, err := json.Marshal(snap)
|
data, err := json.Marshal(snap)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
userCfgDir, _ := os.UserConfigDir()
|
userCfgDir, err := os.UserConfigDir()
|
||||||
|
if err != nil {
|
||||||
|
logger.Warn("quality save skipped: no user config dir", "error", err)
|
||||||
|
return
|
||||||
|
}
|
||||||
dir := filepath.Join(userCfgDir, "gnoma")
|
dir := filepath.Join(userCfgDir, "gnoma")
|
||||||
os.MkdirAll(dir, 0o755)
|
os.MkdirAll(dir, 0o755)
|
||||||
os.WriteFile(filepath.Join(dir, "quality.json"), data, 0o644)
|
os.WriteFile(filepath.Join(dir, "quality.json"), data, 0o644)
|
||||||
@@ -409,6 +416,7 @@ func main() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Resume logic: --resume/-r flag
|
// Resume logic: --resume/-r flag
|
||||||
|
resumedTurnCount := 0
|
||||||
resumeRequested := isFlagSet("resume") || isFlagSet("r")
|
resumeRequested := isFlagSet("resume") || isFlagSet("r")
|
||||||
if resumeRequested {
|
if resumeRequested {
|
||||||
var snap session.Snapshot
|
var snap session.Snapshot
|
||||||
@@ -439,6 +447,7 @@ func main() {
|
|||||||
eng.SetHistory(snap.Messages)
|
eng.SetHistory(snap.Messages)
|
||||||
eng.SetUsage(snap.Metadata.Usage)
|
eng.SetUsage(snap.Metadata.Usage)
|
||||||
sessionID = snap.ID
|
sessionID = snap.ID
|
||||||
|
resumedTurnCount = snap.Metadata.TurnCount
|
||||||
logger.Info("session resumed", "id", snap.ID, "turns", snap.Metadata.TurnCount)
|
logger.Info("session resumed", "id", snap.ID, "turns", snap.Metadata.TurnCount)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -504,6 +513,7 @@ func main() {
|
|||||||
Provider: *providerName,
|
Provider: *providerName,
|
||||||
Model: armModel,
|
Model: armModel,
|
||||||
SessionID: sessionID,
|
SessionID: sessionID,
|
||||||
|
TurnCount: resumedTurnCount,
|
||||||
Store: sessStore,
|
Store: sessStore,
|
||||||
Incognito: fw.Incognito(),
|
Incognito: fw.Incognito(),
|
||||||
Logger: logger,
|
Logger: logger,
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ type LocalConfig struct {
|
|||||||
Provider string
|
Provider string
|
||||||
Model string
|
Model string
|
||||||
SessionID string // identifies this session on disk
|
SessionID string // identifies this session on disk
|
||||||
|
TurnCount int // seed from restored snapshot; 0 for new sessions
|
||||||
Store *SessionStore // nil = no persistence
|
Store *SessionStore // nil = no persistence
|
||||||
Incognito *security.IncognitoMode // nil = always persist
|
Incognito *security.IncognitoMode // nil = always persist
|
||||||
Logger *slog.Logger // nil = slog.Default()
|
Logger *slog.Logger // nil = slog.Default()
|
||||||
@@ -60,6 +61,7 @@ func NewLocal(cfg LocalConfig) *Local {
|
|||||||
state: StateIdle,
|
state: StateIdle,
|
||||||
provider: cfg.Provider,
|
provider: cfg.Provider,
|
||||||
model: cfg.Model,
|
model: cfg.Model,
|
||||||
|
turnCount: cfg.TurnCount,
|
||||||
sessionID: cfg.SessionID,
|
sessionID: cfg.SessionID,
|
||||||
store: cfg.Store,
|
store: cfg.Store,
|
||||||
incognito: cfg.Incognito,
|
incognito: cfg.Incognito,
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ import (
|
|||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"sort"
|
"sort"
|
||||||
|
"strings"
|
||||||
|
|
||||||
"somegit.dev/Owlibou/gnoma/internal/message"
|
"somegit.dev/Owlibou/gnoma/internal/message"
|
||||||
)
|
)
|
||||||
@@ -27,8 +28,25 @@ func NewSessionStore(projectRoot string, maxKeep int, logger *slog.Logger) *Sess
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// sessionDir validates a session ID and returns its absolute path within the store.
|
||||||
|
// Rejects empty IDs and path traversal attempts.
|
||||||
|
func (s *SessionStore) sessionDir(id string) (string, error) {
|
||||||
|
if id == "" {
|
||||||
|
return "", fmt.Errorf("session ID must not be empty")
|
||||||
|
}
|
||||||
|
dir := filepath.Join(s.dir, id)
|
||||||
|
storeRoot := filepath.Clean(s.dir) + string(os.PathSeparator)
|
||||||
|
if !strings.HasPrefix(dir+string(os.PathSeparator), storeRoot) {
|
||||||
|
return "", fmt.Errorf("invalid session ID %q", id)
|
||||||
|
}
|
||||||
|
return dir, nil
|
||||||
|
}
|
||||||
|
|
||||||
func (s *SessionStore) Save(snap Snapshot) error {
|
func (s *SessionStore) Save(snap Snapshot) error {
|
||||||
dir := filepath.Join(s.dir, snap.ID)
|
dir, err := s.sessionDir(snap.ID)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("session save: %w", err)
|
||||||
|
}
|
||||||
if err := os.MkdirAll(dir, 0o755); err != nil {
|
if err := os.MkdirAll(dir, 0o755); err != nil {
|
||||||
return fmt.Errorf("session %q: create dir: %w", snap.ID, err)
|
return fmt.Errorf("session %q: create dir: %w", snap.ID, err)
|
||||||
}
|
}
|
||||||
@@ -45,7 +63,10 @@ func (s *SessionStore) Save(snap Snapshot) error {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (s *SessionStore) Load(id string) (Snapshot, error) {
|
func (s *SessionStore) Load(id string) (Snapshot, error) {
|
||||||
dir := filepath.Join(s.dir, id)
|
dir, err := s.sessionDir(id)
|
||||||
|
if err != nil {
|
||||||
|
return Snapshot{}, fmt.Errorf("session load: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
metaBytes, err := os.ReadFile(filepath.Join(dir, "metadata.json"))
|
metaBytes, err := os.ReadFile(filepath.Join(dir, "metadata.json"))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -106,6 +106,25 @@ func TestSessionStore_List_SortedByUpdatedAt(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestSessionStore_Load_RejectsPathTraversal(t *testing.T) {
|
||||||
|
store := makeStore(t)
|
||||||
|
cases := []string{"../../etc/passwd", "../sibling", ""}
|
||||||
|
for _, id := range cases {
|
||||||
|
_, err := store.Load(id)
|
||||||
|
if err == nil {
|
||||||
|
t.Errorf("Load(%q): expected error for invalid ID", id)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSessionStore_Save_RejectsPathTraversal(t *testing.T) {
|
||||||
|
store := makeStore(t)
|
||||||
|
snap := makeSnap("../../evil", time.Now().UTC())
|
||||||
|
if err := store.Save(snap); err == nil {
|
||||||
|
t.Error("Save with traversal ID: expected error")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestSessionStore_Prune_RemovesOldest(t *testing.T) {
|
func TestSessionStore_Prune_RemovesOldest(t *testing.T) {
|
||||||
store := makeStore(t) // maxKeep = 3
|
store := makeStore(t) // maxKeep = 3
|
||||||
now := time.Now().UTC()
|
now := time.Now().UTC()
|
||||||
|
|||||||
Reference in New Issue
Block a user