Skip to content

Commit 01bfc9c

Browse files
committed
fix: session ID entropy + auth tokens; restrict send_message callbacks
Finding #9 — Predictable session IDs: - Generate 128-bit random session IDs (YYYYMMDD-<32 hex chars>). - Add a 256-bit session-scoped AuthToken stored in the session JSON. - Require the token via X-Session-Token header, session_token cookie, or auth_token WebSocket field for GET/DELETE/POST /api/sessions/<id> and WebSocket session resume. - Add per-IP rate limiting (60/min) on session detail lookups. - Redact AuthToken from the /api/sessions list endpoint. - Update the Web UI to store and send tokens. Finding #10 — send_message callback injection: - Reject callback_data values that start with reserved internal prefixes (apr:, den:, trs:, clarify:, skill_save:, skill_skip:) in the send_message tool. - Add defense-in-depth validation in the Telegram sender closure. - Only user-facing cb: callbacks are allowed. Tests and docs updated.
1 parent d360a82 commit 01bfc9c

11 files changed

Lines changed: 581 additions & 46 deletions

File tree

AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
137137
- **Schedule atomic-write hardening** (`internal/schedule/store.go` + `internal/fsatomic`) — schedule file writes now use `fsatomic.WriteFile`, which creates a random temp file with `O_EXCL`, fsyncs data and directory, and renames over the target. A swapped-in symlink is replaced rather than followed, closing the symlink-override attack on `schedules.json` / `schedule-state.json`.
138138
- **Telegram singleton flock lock** (`cmd/odek/telegram.go` + `internal/flock`) — the Telegram bot now uses an advisory `flock` on `~/.odek/telegram.lock` instead of a PID file probed with signals. This removes the non-Linux path where a planted PID could cause odek to kill an arbitrary process.
139139
- **Telegram photo caption wrapping** (`cmd/odek/telegram.go`) — photo captions cross the Telegram trust boundary, so they are wrapped as untrusted content both when passed to the local vision model and when injected into the main agent's user message.
140+
- **`send_message` callback prefix restriction** (`internal/tool/send_message.go` + `cmd/odek/telegram.go`) — the `send_message` tool rejects any button whose `callback_data` starts with a reserved internal prefix (`apr:`, `den:`, `trs:`, `clarify:`, `skill_save:`, `skill_skip:`); only user-facing `cb:` callbacks are allowed. The Telegram sender closure validates again as defense-in-depth, preventing a forged approval or skill button.
141+
- **Session ID entropy + session-scoped auth tokens** (`internal/session/session.go`, `cmd/odek/serve.go`) — session IDs now carry 128 bits of randomness (16 bytes / 32 hex chars); each session stores a 256-bit `AuthToken` required by `GET/DELETE/POST /api/sessions/<id>` and WebSocket session-resume messages via `X-Session-Token` header, `session_token` cookie, or `auth_token` WS field. Per-IP rate limiting (60/min) on session lookups adds a brute-force backstop.
140142
- **Secret redaction** (`internal/redact/redact.go`) — 20+ patterns: OpenAI, Anthropic, GitHub PAT, AWS, PEM, JWT, Vault, Google OAuth, SendGrid, Discord, DB URLs, etc.
141143

142144
### Platform Support

cmd/odek/serve.go

Lines changed: 163 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,59 @@ var wsConns sync.Map // map[*golangws.Conn]struct{}
5050
// after closing all connections to ensure cleanup completes.
5151
var wsHandlerWG sync.WaitGroup
5252

53+
// sessionLookupLimiter provides basic per-IP rate limiting for session detail
54+
// lookups, raising the cost of brute-force enumeration of session IDs.
55+
var sessionLookupLimiter = newRateLimiter(60, time.Minute)
56+
57+
// rateLimiter is a tiny per-key sliding-window rate limiter.
58+
type rateLimiter struct {
59+
mu sync.Mutex
60+
windows map[string][]time.Time
61+
max int
62+
window time.Duration
63+
}
64+
65+
func newRateLimiter(max int, window time.Duration) *rateLimiter {
66+
return &rateLimiter{
67+
windows: make(map[string][]time.Time),
68+
max: max,
69+
window: window,
70+
}
71+
}
72+
73+
// allow returns true if the key has not exceeded max requests in the sliding
74+
// window. It prunes stale entries on each call.
75+
func (rl *rateLimiter) allow(key string) bool {
76+
if rl == nil || rl.max <= 0 {
77+
return true
78+
}
79+
rl.mu.Lock()
80+
defer rl.mu.Unlock()
81+
82+
now := time.Now().UTC()
83+
cutoff := now.Add(-rl.window)
84+
var times []time.Time
85+
for _, t := range rl.windows[key] {
86+
if t.After(cutoff) {
87+
times = append(times, t)
88+
}
89+
}
90+
if len(times) >= rl.max {
91+
rl.windows[key] = times
92+
return false
93+
}
94+
times = append(times, now)
95+
rl.windows[key] = times
96+
return true
97+
}
98+
99+
// reset clears all tracked windows (useful in tests).
100+
func (rl *rateLimiter) reset() {
101+
rl.mu.Lock()
102+
defer rl.mu.Unlock()
103+
rl.windows = make(map[string][]time.Time)
104+
}
105+
53106
// ── Serve Command ───────────────────────────────────────────────────────
54107

55108
func serveCmd(args []string) error {
@@ -444,6 +497,7 @@ type wsClientMsg struct {
444497
Type string `json:"type"`
445498
Content string `json:"content"`
446499
SessionID string `json:"session_id"`
500+
AuthToken string `json:"auth_token,omitempty"`
447501
Model string `json:"model,omitempty"`
448502
Thinking string `json:"thinking,omitempty"` // "enabled" | "" — per-query toggle
449503
}
@@ -633,12 +687,18 @@ func handleWS(store *session.Store, resources *resource.Registry, resolved confi
633687
// Handle session switch mid-connection (new conversation)
634688
if msg.SessionID != "" && (currentSession == nil || currentSession.ID != msg.SessionID) {
635689
sess, err := store.Load(msg.SessionID)
636-
if err == nil {
637-
currentSession = sess
638-
// Restore buffer from the resumed session
639-
if mm := agent.Memory(); mm != nil && len(sess.Buffer) > 0 {
640-
mm.RestoreBuffer(sess.Buffer)
641-
}
690+
if err != nil {
691+
writeWSError(conn, "session not found")
692+
continue
693+
}
694+
if _, ok := validateSessionToken(store, sess, msg.AuthToken); !ok {
695+
writeWSError(conn, "invalid session token")
696+
continue
697+
}
698+
currentSession = sess
699+
// Restore buffer from the resumed session
700+
if mm := agent.Memory(); mm != nil && len(sess.Buffer) > 0 {
701+
mm.RestoreBuffer(sess.Buffer)
642702
}
643703
}
644704

@@ -735,10 +795,12 @@ func handlePrompt(
735795

736796
// Send session info
737797
sid := ""
798+
authToken := ""
738799
if sess != nil {
739800
sid = sess.ID
801+
authToken = sess.AuthToken
740802
}
741-
writeWSJSON(conn, map[string]any{"type": "session", "session_id": sid, "model": resolved.Model, "sandbox": resolved.Sandbox})
803+
writeWSJSON(conn, map[string]any{"type": "session", "session_id": sid, "auth_token": authToken, "model": resolved.Model, "sandbox": resolved.Sandbox})
742804

743805
// Append user input to buffer (AppendBuffer summarizes raw text).
744806
if mm := agent.Memory(); mm != nil {
@@ -986,6 +1048,41 @@ func isStateChangingMethod(method string) bool {
9861048
return true
9871049
}
9881050

1051+
// sessionTokenFromRequest returns the session auth token from the
1052+
// X-Session-Token header or the session_token cookie, in that order.
1053+
func sessionTokenFromRequest(r *http.Request) string {
1054+
if t := r.Header.Get("X-Session-Token"); t != "" {
1055+
return t
1056+
}
1057+
if c, err := r.Cookie("session_token"); err == nil && c.Value != "" {
1058+
return c.Value
1059+
}
1060+
return ""
1061+
}
1062+
1063+
// validateSessionToken checks the provided token against the session. If the
1064+
// session has no token (legacy session created before this defense), a token is
1065+
// generated and the session is persisted. The returned string is the effective
1066+
// token (empty only when validation failed). The bool indicates success.
1067+
func validateSessionToken(store *session.Store, sess *session.Session, token string) (string, bool) {
1068+
if sess == nil {
1069+
return "", false
1070+
}
1071+
if sess.AuthToken == "" {
1072+
sess.AuthToken = session.GenerateAuthToken()
1073+
if err := store.Save(sess); err != nil {
1074+
// If we cannot persist the token, still allow this request but do not
1075+
// leak a transient token to the client.
1076+
return "", true
1077+
}
1078+
return sess.AuthToken, true
1079+
}
1080+
if token == sess.AuthToken {
1081+
return sess.AuthToken, true
1082+
}
1083+
return "", false
1084+
}
1085+
9891086
func writeWSJSON(conn *golangws.Conn, data any) {
9901087
payload, err := json.Marshal(data)
9911088
if err != nil {
@@ -1032,6 +1129,12 @@ func handleSessionList(store *session.Store) http.HandlerFunc {
10321129
sessions = []session.Session{}
10331130
}
10341131

1132+
// Never leak session-scoped auth tokens in the list endpoint. Tokens are
1133+
// only returned (in the X-Session-Token header) after a valid detail lookup.
1134+
for i := range sessions {
1135+
sessions[i].AuthToken = ""
1136+
}
1137+
10351138
w.Header().Set("Content-Type", "application/json")
10361139
json.NewEncoder(w).Encode(sessions)
10371140
}
@@ -1040,24 +1143,45 @@ func handleSessionList(store *session.Store) http.HandlerFunc {
10401143
func handleSessionByID(store *session.Store) http.HandlerFunc {
10411144
return func(w http.ResponseWriter, r *http.Request) {
10421145
id := strings.TrimPrefix(r.URL.Path, "/api/sessions/")
1146+
if id == "" {
1147+
http.Error(w, "missing session id", http.StatusBadRequest)
1148+
return
1149+
}
10431150

10441151
switch r.Method {
10451152
case http.MethodGet:
1046-
if id == "" {
1047-
http.Error(w, "missing session id", http.StatusBadRequest)
1153+
// Rate-limit session detail lookups per IP to slow brute-force
1154+
// enumeration of the 128-bit ID space.
1155+
if !sessionLookupLimiter.allow(clientIP(r)) {
1156+
http.Error(w, "rate limit exceeded", http.StatusTooManyRequests)
10481157
return
10491158
}
10501159
sess, err := store.Load(id)
10511160
if err != nil {
10521161
http.Error(w, "session not found", http.StatusNotFound)
10531162
return
10541163
}
1164+
token := sessionTokenFromRequest(r)
1165+
effectiveToken, ok := validateSessionToken(store, sess, token)
1166+
if !ok {
1167+
http.Error(w, "invalid session token", http.StatusUnauthorized)
1168+
return
1169+
}
10551170
w.Header().Set("Content-Type", "application/json")
1171+
if effectiveToken != "" {
1172+
w.Header().Set("X-Session-Token", effectiveToken)
1173+
}
10561174
json.NewEncoder(w).Encode(sess)
10571175

10581176
case http.MethodDelete:
1059-
if id == "" {
1060-
http.Error(w, "missing session id", http.StatusBadRequest)
1177+
sess, err := store.Load(id)
1178+
if err != nil {
1179+
http.Error(w, "session not found", http.StatusNotFound)
1180+
return
1181+
}
1182+
token := sessionTokenFromRequest(r)
1183+
if _, ok := validateSessionToken(store, sess, token); !ok {
1184+
http.Error(w, "invalid session token", http.StatusUnauthorized)
10611185
return
10621186
}
10631187
if err := store.Delete(id); err != nil {
@@ -1068,10 +1192,6 @@ func handleSessionByID(store *session.Store) http.HandlerFunc {
10681192

10691193
case http.MethodPost:
10701194
// Rename session
1071-
if id == "" {
1072-
http.Error(w, "missing session id", http.StatusBadRequest)
1073-
return
1074-
}
10751195
var body struct {
10761196
Name string `json:"name"`
10771197
}
@@ -1084,6 +1204,11 @@ func handleSessionByID(store *session.Store) http.HandlerFunc {
10841204
http.Error(w, "session not found", http.StatusNotFound)
10851205
return
10861206
}
1207+
token := sessionTokenFromRequest(r)
1208+
if _, ok := validateSessionToken(store, sess, token); !ok {
1209+
http.Error(w, "invalid session token", http.StatusUnauthorized)
1210+
return
1211+
}
10871212
sess.Task = body.Name
10881213
store.Save(sess)
10891214
w.Header().Set("Content-Type", "application/json")
@@ -1095,6 +1220,29 @@ func handleSessionByID(store *session.Store) http.HandlerFunc {
10951220
}
10961221
}
10971222

1223+
// clientIP returns a best-effort client identifier for rate limiting. It prefers
1224+
// X-Forwarded-For / X-Real-Ip only when the direct remote address is a loopback
1225+
// proxy, otherwise uses RemoteAddr. This avoids trusting spoofed headers from
1226+
// arbitrary clients while still working behind a local reverse proxy.
1227+
func clientIP(r *http.Request) string {
1228+
host, _, err := net.SplitHostPort(r.RemoteAddr)
1229+
if err != nil {
1230+
host = r.RemoteAddr
1231+
}
1232+
if host == "127.0.0.1" || host == "::1" || host == "localhost" {
1233+
if fwd := r.Header.Get("X-Forwarded-For"); fwd != "" {
1234+
if i := strings.Index(fwd, ","); i > 0 {
1235+
return strings.TrimSpace(fwd[:i])
1236+
}
1237+
return strings.TrimSpace(fwd)
1238+
}
1239+
if real := r.Header.Get("X-Real-Ip"); real != "" {
1240+
return real
1241+
}
1242+
}
1243+
return host
1244+
}
1245+
10981246
func handleModelList(configuredModel string) http.HandlerFunc {
10991247
return func(w http.ResponseWriter, r *http.Request) {
11001248
if r.Method != http.MethodGet {

0 commit comments

Comments
 (0)