harden: address PR review — Windows volume + glob-metachar session IDs · Entire
harden: address PR review — Windows volume + glob-metachar session IDs
fdff08c→main·
Soph·1mo ago·4 files·+81 added/-10 removed
Three issues from PR #1365 review:
ValidateSessionID accepted Windows drive-relative paths ("C:foo"): separator-free and not reported absolute by filepath.IsAbs, yet filepath.Join drops the base dir on a volume name. Reject ":" (the volume separator) — portable and testable; no legitimate session ID contains a colon.
ValidateSessionID accepted glob metacharacters ("", "?", "["). Session IDs are interpolated into filepath.Glob patterns (agent transcript lookup in gemini/pi/codex, session-state cleanup), so a "" could match unrelated files. Reject them at the validator choke point.
StateStore.Clear and ClearSessionState globbed "
." to find files to DELETE. With the permissive validator a session ID of "" would have matched and removed every session's state (DoS/data-loss within the state dir). Replace globbing with literal prefix matching so deletion can never depend on pattern interpretation, even if a future caller skips validation.
Adds regression tests for the drive-relative and glob-metacharacter cases.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Sessions
ed740e10abe5View transcript
Changes
- cmd/entire/cli
- session
- Mstate.go+27/-5
- strategy
- Msession_state.go+15/-5
- validation
- Mvalidators.go+13
- Mvalidators_test.go+26
532 unmodified lines
// matchSessionFiles returns the names (not paths) of files in dir that belong to
// the given session ID — i.e. "<sessionID>.<ext>". It uses literal prefix
// matching, never glob patterns, so a session ID containing glob metacharacters
// cannot match unrelated files.
func matchSessionFiles(dir, sessionID string) []string {
entries, err := os.ReadDir(dir)
if err != nil {
return nil // missing/unreadable dir => nothing to clear
}
prefix := sessionID + "."
var matched []string
for _, e := range entries {
if name := e.Name(); strings.HasPrefix(name, prefix) {
matched = append(matched, name)
}
}
return matched
}
// RemoveAll removes the entire session state directory.
// This is used during uninstall to completely remove all session state.
func (s *StateStore) RemoveAll() error {
// Remove all files for this session (state .json, .model hint, any future hint files).
// filepath.Glob finds matches; os.Root ensures traversal-resistant removal.
matches, _ := filepath.Glob(filepath.Join(s.stateDir, sessionID + ".*")) //nolint:errcheck // pattern is always valid
for _, f := range matches {
_ = osroot.Remove(root, filepath.Base(f)) //nolint:errcheck // best-effort cleanup
}
// Reject glob metacharacters. Session IDs are interpolated into
// filepath.Glob patterns in several places (agent transcript lookup,
// session-state cleanup); "*"/"?"/"[" could match and act on unrelated files.
if strings.ContainsAny(id, "*?[") {
return fmt.Errorf("invalid session ID %q: contains glob metacharacters", id)
}
89 unmodified lines
for _, tt := range tests {