feat(pam): real-time session log sync via incremental batch uploads#173
Conversation
Replaces end-of-session bulk upload with incremental 10-second batch uploads to enable live monitoring and session intervention. - Add CallUploadPamSessionEventBatch to post raw event bytes to the new batch endpoint - Add sessionUploadState with per-session file offset tracking and mutex - Add RegisterSession/UnregisterSession for active session management - Add readFromOffset to read and decrypt new records from a byte offset - Persist upload progress to a .offset file alongside the .enc recording for crash recovery; resume from saved offset on restart - Flush all active sessions every 10 seconds via a new ticker - Re-register all on-disk session files at startup to resume after crash - Fall back to legacy bulk upload if the batch endpoint returns 404 - Remove end-of-session bulk upload call from CleanupPAMSession
Greptile SummaryThis PR replaces the end-of-session bulk upload for PAM session events with an incremental 10-second batch upload, with automatic fallback to the legacy bulk upload when the platform's
Confidence Score: 3/5Not safe to merge as-is: the legacyMode bug causes continuous 404 flood and log spam for every session on older platforms. Two P1 findings need to be addressed: the missing legacyMode early-return in flushSession (causes repeated wasted HTTP calls and log spam on every tick) and the use of native regexp instead of re2 against the repo policy. packages/pam/session/uploader.go — legacyMode guard and regexp; packages/api/api.go — URL path escaping.
|
| Filename | Overview |
|---|---|
| packages/pam/session/uploader.go | New incremental batch upload logic; contains a P1 bug where legacyMode is never checked before re-attempting uploads, causing repeated 404 calls and log spam every 10 seconds, plus native regexp used contrary to repo re2 policy. |
| packages/api/api.go | Adds CallUploadPamSessionEventBatch function; sessionId is interpolated into the URL path without url.PathEscape, which could be exploited with a crafted filename in the recording directory. |
| packages/pam/pam-proxy.go | Minor wiring change to call RegisterSession after the session logger is created; logic is straightforward and correct. |
Comments Outside Diff (1)
-
packages/pam/session/uploader.go, line 71-96 (link)Native
regexpused instead ofre2— ReDoS risk per repo policyBoth
newFormatRegexandlegacyFormatRegexare compiled with the standard libraryregexppackage inside the function body (recompiled on every call). The repository rule requires all new regex usage to use there2package to prevent ReDoS. The pattern(.+)combined with the resource-type alternation can exhibit catastrophic backtracking on a crafted input.Additionally, compiling inside a hot function is inefficient; these should be package-level
varexpressions.Consider migrating to a re2-compatible package and hoisting the compiled patterns to package scope:
var ( newFormatRegex = re2.MustCompile(`^pam_session_(.+)_(postgres|mysql|mssql|redis|ssh|kubernetes)_expires_(\d+)\.enc$`) legacyFormatRegex = re2.MustCompile(`^pam_session_(.+)_expires_(\d+)\.enc$`) )
Verify the current patterns at https://devina.io/redos-checker.
Reviews (1): Last reviewed commit: "fix(pam): restore debug logs accidentall..." | Re-trigger Greptile
|
@claude review this |
Description 📣
Instead of uploading session logs in a single bulk request at the end of a session, the gateway now uploads events incrementally every 10 seconds. This unblocks live session monitoring and session intervention, which previously required waiting until the session ended to see any activity.
The gateway reads new records from the local recording file since the last uploaded byte offset, encrypts them as a JSON array, and POSTs them to the new
/event-batchesendpoint on the platform. The byte offset is persisted to disk alongside the recording file so uploads resume correctly after a crash or restart. Re-uploads of the same byte range are idempotent. If the platform does not support the new endpoint (404), the gateway automatically falls back to the original bulk upload behavior at session end.Depends on: Infisical/infisical#5965 — should be merged first.
Type ✨
Tests 🛠️
pam_session_event_batchesevery ~10 seconds with advancing offsets/event-batchesroute — verify the warning log appears and logs are uploaded via the legacy/logsendpoint at session end