Skip to content

fix: close session ownership gaps on metrics, tools, latency, screenshot, events, stats (#1399)#1607

Merged
OneStepAt4time merged 1 commit intodevelopfrom
fix/1399-session-ownership-v2
Apr 10, 2026
Merged

fix: close session ownership gaps on metrics, tools, latency, screenshot, events, stats (#1399)#1607
OneStepAt4time merged 1 commit intodevelopfrom
fix/1399-session-ownership-v2

Conversation

@OneStepAt4time
Copy link
Copy Markdown
Owner

Summary

Add ownership checks to 6 per-session routes that were missing them:

  • GET /v1/sessions/:id/metrics
  • GET /v1/sessions/:id/tools
  • GET /v1/sessions/:id/latency
  • POST /v1/sessions/:id/screenshot
  • GET /v1/sessions/:id/events
  • GET /v1/sessions/stats (scoped by ownerKeyId for non-master callers)

Changes

  • src/server.ts: +17/-11 lines

Security

Without these checks, any API key could read metrics, tool usage, latency, screenshots, and event streams for sessions they don't own.

PR Stats

  • 1 file changed, 17 insertions(+), 11 deletions(-)
  • Well within 500 line diff limit ✅

…hot, events, stats (#1399)

- Add requireOwnership() to /v1/sessions/:id/metrics
- Add requireOwnership() to /v1/sessions/:id/tools
- Add requireOwnership() to /v1/sessions/:id/latency
- Add requireOwnership() to /v1/sessions/:id/screenshot
- Add requireOwnership() to /v1/sessions/:id/events
- Scope /v1/sessions/stats by caller ownership for non-master keys
- Consensus route removed (already removed in #1583)
@OneStepAt4time OneStepAt4time force-pushed the fix/1399-session-ownership-v2 branch from 1dda0e4 to d72ba07 Compare April 10, 2026 12:51
Copy link
Copy Markdown
Contributor

@aegis-gh-agent aegis-gh-agent bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved.

Fixes #1399 — Session ownership gaps closed on 6 endpoints.

Endpoints now guarded:

  • GET /v1/sessions/:id/metrics — requireOwnership
  • GET /v1/sessions/:id/tools — requireOwnership
  • GET /v1/sessions/:id/latency — requireOwnership
  • POST /sessions/:id/screenshot — requireOwnership
  • GET /v1/sessions/:id/events (SSE) — requireOwnership
  • GET /v1/sessions/stats — scoped by ownerKeyId for non-master keys

Notes:

  • Uses existing requireOwnership() pattern — consistent with codebase
  • Stats endpoint correctly scopes: master sees all, non-master filtered by ownerKeyId, null/unset = legacy pass-through
  • Missing from original #1597: consensus handler ownership + ownerKeyId propagation on child sessions. Recommend a follow-up PR for those.

1 file, 17 lines. CI green. Clean rebase.

@OneStepAt4time OneStepAt4time merged commit 2ddb57d into develop Apr 10, 2026
11 checks passed
@OneStepAt4time OneStepAt4time deleted the fix/1399-session-ownership-v2 branch April 10, 2026 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant