Skip to content

refactor(proxy): extract sessionKeyFor helper for session-key construction#643

Merged
luckyPipewrench merged 2 commits into
mainfrom
refactor/proxy-deny-helpers
May 31, 2026
Merged

refactor(proxy): extract sessionKeyFor helper for session-key construction#643
luckyPipewrench merged 2 commits into
mainfrom
refactor/proxy-deny-helpers

Conversation

@luckyPipewrench
Copy link
Copy Markdown
Owner

@luckyPipewrench luckyPipewrench commented May 31, 2026

Dedups the per-session adaptive-enforcement key construction. The pattern (agent-namespaced client IP, falling back to client IP for anonymous or unnamed agents) was rebuilt inline across the fetch, forward, CONNECT, WebSocket, TLS-interception, CEE, and admin/session paths. All sites now route through one sessionKeyFor helper, so adaptive escalation, de-escalation, and audit correlation share a single source of truth. Mechanical extraction, no behavioral change. 9 files, +112/-160.

Summary by CodeRabbit

  • Refactor

    • Consolidated session key construction logic across multiple internal modules to centralize adaptive enforcement tracking, improving code consistency and maintainability.
  • Tests

    • Added unit tests to validate session key construction behavior.

…ction

The per-session key (agent + "|" + clientIP, falling back to clientIP for
anonymous or unnamed agents) was rebuilt inline across the fetch, forward,
CONNECT, WebSocket, TLS-intercept, CEE, and admin/session paths. Route all of
them through one sessionKeyFor helper so adaptive escalation, de-escalation,
and audit correlation share a single source of truth.

Mechanical extraction, no behavioral change.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 31, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2490d332-55cd-409a-b0a6-43c35bf7221d

📥 Commits

Reviewing files that changed from the base of the PR and between cde0ce1 and 5bdeecb.

📒 Files selected for processing (9)
  • internal/proxy/cee.go
  • internal/proxy/forward.go
  • internal/proxy/intercept.go
  • internal/proxy/proxy.go
  • internal/proxy/proxy_test.go
  • internal/proxy/session.go
  • internal/proxy/sessionkey.go
  • internal/proxy/sessionkey_test.go
  • internal/proxy/websocket.go

📝 Walkthrough

Walkthrough

This PR refactors session key construction across the proxy package by centralizing repeated conditional logic into a shared sessionKeyFor(agent, clientIP) helper function. The helper returns agent|clientIP for named agents, or clientIP for empty/anonymous agents, and is consistently applied across adaptive enforcement, escalation, and signal-recording paths in CONNECT, forward-proxy, intercepted-CONNECT, and WebSocket handlers.

Changes

Session Key Construction Refactoring

Layer / File(s) Summary
Session key helper and unit tests
internal/proxy/sessionkey.go, internal/proxy/sessionkey_test.go
Introduces sessionKeyFor(agent, clientIP) helper returning agent|clientIP for named agents or clientIP for empty/anonymous agents, with table-driven unit tests validating named-agent precedence, empty-agent handling, and edge cases.
Foundational session key refactoring
internal/proxy/cee.go, internal/proxy/session.go, internal/proxy/proxy_test.go
Applies helper to CeeSessionKey, SessionManager.AdaptiveWhoami lookup, and test utility escalateSession for consistent key derivation at the session manager level.
Core proxy handler refactoring
internal/proxy/proxy.go
Refactors session key derivation in recordSessionActivityWithUserAgent, recordShieldIntervention, handleFetch (recorder selection and escalation checks), and filterAndActOnResponseScan adaptive signal recording.
Forward proxy handler refactoring
internal/proxy/forward.go
Refactors session key derivation across handleConnect and handleForwardHTTP escalation/block-all paths, TLS-interception live session recorder initialization, and adaptive enforcement across request-body and response-injection scanning and SignalStrip scoring.
Intercepted CONNECT refactoring
internal/proxy/intercept.go
Refactors session key derivation in interceptRecordSignal, URL-scan audit escalation, request-body and response-injection adaptive upgrades, session recovery de-escalation, block-all enforcement, and SignalStrip recording.
WebSocket handler refactoring
internal/proxy/websocket.go
Refactors session key derivation in wsRelay.recordSignal, handshake escalation/block-all, header DLP and address-protection escalations, session recorder lookup, request-body upgrades, relay de-escalation/block-all enforcement, and response-scan escalation/SignalStrip recording.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • luckyPipewrench/pipelock#304: Both PRs touch session key construction logic—the retrieved PR adds on-entry block-all recovery paths that build session keys, while the main PR refactors all session-key call sites to use the new centralized sessionKeyFor helper.
  • luckyPipewrench/pipelock#550: Both PRs directly affect adaptive enforcement session identity—the main PR refactors the exact session-key derivation used by SessionManager.AdaptiveWhoami and adaptive lookups, while the retrieved PR introduces the AdaptiveWhoami endpoint that relies on consistent (clientIP, agent) key construction.

Suggested labels

pr:verified, contributor:verified

Suggested reviewers

  • DeathCamel58
  • BlueAceFrost

Poem

🐰 A rabbit hops through proxy streams,
With session keys no longer split,
One helper tames the scattered schemes—
sessionKeyFor makes logic fit.
From CONNECT to WebSocket's dance,
Consistency leads by the hand!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: extracting a shared sessionKeyFor helper for session-key construction across multiple proxy files.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/proxy-deny-helpers

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@luckyPipewrench luckyPipewrench self-assigned this May 31, 2026
@cla-assistant
Copy link
Copy Markdown

cla-assistant Bot commented May 31, 2026

CLA assistant check
All committers have signed the CLA.

@sentry
Copy link
Copy Markdown

sentry Bot commented May 31, 2026

Codecov Report

❌ Patch coverage is 78.57143% with 9 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/proxy/websocket.go 50.00% 7 Missing ⚠️
internal/proxy/forward.go 87.50% 1 Missing ⚠️
internal/proxy/intercept.go 85.71% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@luckyPipewrench luckyPipewrench enabled auto-merge (squash) May 31, 2026 13:47
@luckyPipewrench luckyPipewrench merged commit cfa0ae1 into main May 31, 2026
19 checks passed
@luckyPipewrench luckyPipewrench deleted the refactor/proxy-deny-helpers branch May 31, 2026 13:59
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