Skip to content

fix(api,web): X-TCDI-Poll marker stops background polls sliding the portal idle window (FIX-IDLE) - #133

Open
vanlongme wants to merge 8 commits into
mainfrom
v10/fix-idle
Open

vanlongme wants to merge 8 commits into
mainfrom
v10/fix-idle

Conversation

@vanlongme

@vanlongme vanlongme commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Portal sessions carry a sliding idle timeout, but the SPA polls GET /v1/workspaces, GET /v1/workspaces/{id}, GET /v1/workspaces/{id}/events (and the data lists) every 8–10 s on a visible tab — and those routes were mounted behind sliding RequireAuth, so an open but unattended tab kept the session alive forever.

Design (server-side rule — no client marker)

The server decides what counts as activity; the client never does:

  • Every cookie-authenticated GET mounts RequireAuthPassive (Peek, no slide): workspaces list/detail/events, templates, data list/detail, /v1/me, /v1/quota, and the admin reads. /v1/session and /v1/workspaces/{id}/connection were already passive. Interval polls can therefore never hold a session open.
  • Only mutations slide — including the new POST /v1/session:touch activity beat (passive auth + CSRF; the handler slides explicitly with Get only after the token check, so a cookie-only POST never earns a slide; bodiless 204; audited session.touch; wrapped in the login-family limiter keyed on the session digest). The SPA fires it from useActivityTouch on pointerdown, keydown and route changes — throttled to one beat per 60 s, never from timer polls or background fetches. Forging it needs cookie + CSRF — the same bar as any mutation.
  • Desktop activity is credited broker-side on real RFB input events only (existing InputHook → session store touch, 1/min throttle). Lease renew deliberately does not slide — a sliding renew would let a connected-but-idle stream pin the session open, reintroducing the same bug through the desktop path. Net idle semantics: no user interaction AND no desktop input for the whole window.

SR-1-F2 (lease layer honours the idle window)

WithSessionIdle(cfg.SessionIdle) gives the broker the portal idle window, and the shared portalSessionLiveSQL predicate (epoch + absolute expiry + last_seen_at inside the window) now backs both loadLease's CASE and RedeemTicket's session re-check. Redeem, renew and rehydrate all fail closed on an idle-dead session — the lease is revoked on the spot (reason=invalid), so the stream dies within one renew cycle instead of outliving its session. NULL-digest legacy leases stay exempt.

SR-1-F3 (input touch scoped to the bound session)

Desktop input used to slide every session of the principal. loadLease now selects the bound portal_session_digest and the input hook carries it to the new TouchSessionDigest store method (single-row, pkey-keyed, same never-revive guards). NULL-digest legacy leases keep the principal-wide fallback so pre-upgrade streams survive a rolling deploy.

Tests

Command Result
go test -race ./internal/api/ -count=1 PASS
go test ./internal/broker/ -count=1 PASS (real PG) — TestLeaseSession_IdleDeadSessionRevokesOnRenew, RedeemRejectsIdleDeadSession, IdleCheckDisabledWithoutOption
go test -tags integration ./tests/integration/ -run TestTouch PASS — TestTouchSessionDigest_ScopesToBoundSession, pkey-index EXPLAIN pins
go test ./internal/... PASS (envtest-dependent internal/operator tests fail locally for missing bin/k8s binaries — pre-existing env gap; CI has them)
npm run test:unit PASS 308 — new activity.test.tsx: polls never touch; pointer/key/route beats once per 60 s
npm run typecheck / npm run build / lint:strings PASS; npm run generate committed (schema.d.ts)
Regression vs origin/main TestPolledReads_DoNotSlideIdleWindow + TestMeRead_DoesNotSlideIdleWindow FAIL on the unfixed mounts, PASS here

Also pinned: TestSessionTouch_SlidesIdleWindow (touch slides; 403 without CSRF; 401 anonymous), TestMutation_StillSlidesIdleWindow, TestInputActivity_SlidesOnlyBoundSession, TestInputActivity_NullDigestFallsBackToPrincipal. Docs updated: openapi.yaml idle-timeout section, docs/lifecycle-reasons.md portal-idle note, docs/security/threat-model.md.

git diff --stat origin/main...HEAD:

docs/lifecycle-reasons.md                |   9 ++
 docs/security/threat-model.md            |  38 +++++--
 internal/api/adminquota.go               |   6 +-
 internal/api/adminuserlimit.go           |   4 +-
 internal/api/auditroutes.go              |   3 +
 internal/api/auth.go                     |  70 +++++++++----
 internal/api/auth_test.go                |   3 +-
 internal/api/data.go                     |   7 +-
 internal/api/idlepoll_test.go            | 163 +++++++++++++++++++++++++++++++
 internal/api/inputhook_throttle_test.go  |   6 +-
 internal/api/me.go                       |   7 +-
 internal/api/me_test.go                  |  76 +++++++++++++-
 internal/api/middleware_test.go          |   3 +
 internal/api/openapi.yaml                |  51 ++++++++++
 internal/api/quota.go                    |   4 +-
 internal/api/session_probe.go            |  51 +++++++++-
 internal/api/workspaces.go               |   7 +-
 internal/backend/wire.go                 |  12 ++-
 internal/broker/activity.go              |  10 +-
 internal/broker/connection_state.go      |  11 ++-
 internal/broker/connection_state_test.go |   2 +-
 internal/broker/lease_session_test.go    |  96 ++++++++++++++++++
 internal/broker/leases.go                |  45 +++++++--
 internal/broker/tickets.go               |  37 +++++--
 internal/observability/metrics.go        |   4 +-
 internal/store/sessions.go               |  39 ++++++++
 tests/integration/api_admission_test.go  |   4 +
 tests/integration/session_touch_test.go  |  74 ++++++++++++--
 web/src/api/generated/schema.d.ts        |  58 +++++++++++
 web/src/app/shell.tsx                    |   5 +
 web/src/session/activity.ts              |  55 +++++++++++
 web/tests/mock-api/auth.ts               |   5 +
 web/tests/unit/session/activity.test.tsx |  94 ++++++++++++++++++
 33 files changed, 975 insertions(+), 84 deletions(-)

vanlongme and others added 2 commits October 6, 2026 12:17
…X-IDLE)

Interval polls on RequireAuth routes (workspaces list/detail/events, data
list/detail, the session page's "starting" poll, /v1/me backoff retries)
used to keep a visible-but-unattended portal tab's session alive forever.
The SPA now marks timer-driven reads with X-TCDI-Poll: background and
requireAuth reads marked requests with Peek instead of Get; navigation,
manual refresh, return-to-visible reloads and mutations still slide. The
marker is honoured on every authenticated route and can only withhold an
idle slide, never earn one.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Literal "X-TCDI-Poll: background" instead of the new constants keeps the
regression test compiling on v0.5.0, where it then fails at the "marked
poll past idle" step (204, want 401) — proving the pre-fix slide.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 6, 2026 05:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

vanlongme and others added 6 commits October 6, 2026 12:33
…ow (FIX-IDLE)

Advisor-steered redesign (supersedes the client marker at c5745e6): the
server decides what is activity — every cookie-authenticated GET mounts
RequireAuthPassive, so the portal's interval polls can no longer hold a
visible-but-unattended session open; only mutations and server-measured
desktop input slide the idle window. The SPA needs no marker.

SR-1-F2: the lease layer now consults the same liveness rule as
RequireAuth — WithSessionIdle plumbs cfg.SessionIdle into the broker and
the shared portalSessionLiveSQL predicate (epoch + absolute expiry +
last_seen_at inside the window) is applied by loadLease's CASE and
RedeemTicket's session re-check, so redeem, renew and rehydrate all fail
closed on an idled-out session and the lease is revoked on the spot.
Renew deliberately only consults, never slides: a sliding renew would
let a connected-but-idle stream pin the session open — the same bug
through the desktop path.

SR-1-F3: desktop input now credits exactly the session the stream's
lease was minted under — loadLease selects the bound
portal_session_digest and the input hook carries it to the new
digest-scoped store method TouchSessionDigest, instead of sliding every
session of the principal. A NULL-digest legacy lease keeps the
principal-wide fallback so pre-upgrade streams survive a rolling deploy.

Tests: idlepoll_test.go pins that polled GETs never slide (fails on the
old sliding mounts) while mutations still do; lease_session_test gains
idle-dead renew/redeem/rehydrate cases on real Postgres; me_test pins
digest-scoped input vs a same-principal sibling; session_touch_test pins
the pkey index and the scoping/no-revival behaviour.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
With every cookie-authenticated GET now passive, real user interaction
needed a way to slide the idle window — read-only portal use is activity.
POST /v1/session:touch is a bodiless mutation: RequireAuth's sliding read
extends the deadline, RequireCSRF guards it like any other write, the
login-family limiter keys it on the session digest, and the audited
wrapper emits session.touch. The SPA sends it on pointerdown, keydown and
route changes only — throttled to one call per minute in
useActivityTouch (web/src/session/activity.ts), never from timer polls.

Server tests pin the semantics: polls alone let the session expire, one
touch slides it, CSRF/session are both required (403/401). Portal tests
pin that pointer/key/route events beat while useResource's timer ticks
never do. OpenAPI + generated types updated.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Reviewer finding: the beat's RequireAuth ran its sliding read before
RequireCSRF, so a cookie-only POST that failed the token check still
extended the idle window — a slide the caller never earned. The chain is
now RequireAuthPassive + RequireCSRF and the handler slides explicitly
with Get only after both gates pass. Denied requests change nothing:
the test proves a 403'd touch leaves the window to lapse on schedule.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

This branch has not been deployed

No deployments
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.

2 participants