Skip to content

feat: introduce strongly-typed RequestContext#8497

Draft
probelabs[bot] wants to merge 2 commits into
masterfrom
draft-pr-request-context
Draft

feat: introduce strongly-typed RequestContext#8497
probelabs[bot] wants to merge 2 commits into
masterfrom
draft-pr-request-context

Conversation

@probelabs

@probelabs probelabs Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Problem / Task

Currently, request state management in the Gateway relies heavily on context.WithValue via a family of ctxSet... and ctxGet... functions. This approach lacks cohesion, requires type assertions, and makes it hard to track state. This PR introduces a strongly-typed RequestContext struct to hold the mutable state of the request, starting with URL rewrite state.

Changes

  • Created RequestContext struct in ctx/request_context.go with RewriteUrl, Reset, and IsRewritten methods.
  • Updated GetRequestContext to work on a 'get or create' basis, injecting a new RequestContext into the request context if it doesn't exist.
  • Added RequestContextKey to ctx/ctx.go.
  • Updated DummyProxyHandler.ServeHTTP to use rc.RewriteUrl.
  • Updated MCPVEMContinuationMiddleware to use rc.Reset.
  • Updated SanitizeProxyPaths to check rc.IsRewritten().
  • Added unit tests in gateway/api_definition_test.go.

Testing

  • Added unit test strip=true but skipped because UrlRewritten is true in gateway/api_definition_test.go.
  • Ran go test ./gateway/... -run TestAPISpec_SanitizeProxyPaths which passed.

Requested by: U08QWRUM11Q
Slack thread: https://slack.com/archives/D0AF7G82CSV/p1784188011906799

@probelabs

probelabs Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

This PR introduces a strongly-typed RequestContext struct to centralize the management of request-scoped state, moving away from a scattered use of context.WithValue. This change improves type safety, discoverability, and makes the flow of state through middleware easier to track. The initial implementation encapsulates URL rewrite state.

Files Changed Analysis

The changes introduce a new core component and integrate it into the request lifecycle:

  • ctx/request_context.go (New File): Defines the central RequestContext struct, which tracks URL rewrite status in a thread-safe manner. It includes the GetRequestContext function that ensures a RequestContext is always available in the request's context, creating it if it doesn't exist.
  • ctx/ctx.go: Adds a new context key, RequestContextKey, for storing and retrieving the RequestContext instance.
  • gateway/proxy_muxer.go: At the entry point of request handling (handleWrapper.ServeHTTP), a new RequestContext is instantiated and injected into every incoming request's context.
  • gateway/api_loader.go: The DummyProxyHandler, which handles URL rewrites, is updated to record the rewrite action in the RequestContext.
  • gateway/api_definition.go: The SanitizeProxyPaths function now checks RequestContext.IsRewritten() to conditionally skip stripping the listen path, preventing unintended modifications on already rewritten URLs.
  • gateway/mw_mcp_vem_continuation.go: This middleware now calls RequestContext.Reset() to clear the rewrite state when it restores the original URL path.
  • gateway/api_definition_test.go: A new unit test verifies that SanitizeProxyPaths correctly skips path stripping when the URL has been marked as rewritten.

Architecture & Impact Assessment

  • What this PR accomplishes: It refactors request state management from a loosely-typed, key-value approach to a cohesive, strongly-typed RequestContext struct. This establishes a more robust and maintainable pattern for handling request-scoped data.

  • Key technical changes introduced:

    • Centralized State: Introduction of ctx.RequestContext as a single source of truth for mutable request data.
    • Lifecycle Management: The RequestContext is created at the beginning of the request lifecycle in the proxy muxer and is accessible to all subsequent middleware and handlers.
    • Conditional Logic: System components like path sanitization now rely on the state within RequestContext to alter their behavior.
  • Affected system components:

    • Request Entrypoint (gateway/proxy_muxer.go): Responsible for initializing and injecting the RequestContext.
    • URL Rewriting Middleware (gateway/api_loader.go): Acts as a writer to the RequestContext.
    • Path Processing (gateway/api_definition.go): Acts as a reader of the RequestContext to make decisions.
    • Specialized Middleware (gateway/mw_mcp_vem_continuation.go): Interacts with the RequestContext to reset state during complex routing flows.

Request Context Lifecycle

sequenceDiagram
    participant Client
    participant Muxer as "proxy_muxer.go"
    participant Rewriter as "DummyProxyHandler"
    participant Sanitizer as "SanitizeProxyPaths"

    Client->>+Muxer: HTTP Request
    Muxer->>Muxer: Create & Inject RequestContext
    Muxer->>+Rewriter: Process Request
    Note over Rewriter: URL rewrite occurs
    Rewriter->>Rewriter: rc.RewriteUrl(old, new)
    Rewriter-->>-Muxer: Continue
    Muxer->>+Sanitizer: Process Request
    Sanitizer->>Sanitizer: rc.IsRewritten()? -> true
    Note over Sanitizer: Skips path stripping
    Sanitizer-->>-Muxer: Continue
    Muxer-->>-Client: HTTP Response
Loading

Scope Discovery & Context Expansion

This change establishes a new architectural pattern for state management. While the current scope is limited to URL rewriting, the introduction of RequestContext paves the way for migrating other request-scoped data currently managed via context.WithValue.

The other keys defined in ctx/ctx.go (e.g., SessionData, AuthToken) are prime candidates for being moved into the RequestContext struct. Future work will likely involve refactoring other middleware and handlers that use ctxGet... and ctxSet... functions to adopt this new, centralized pattern, which will make the data flow through the request lifecycle easier to trace and debug.

Metadata
  • Review Effort: 3 / 5
  • Primary Label: feature

Powered by Visor from Probelabs

Last updated: 2026-07-20T10:28:30.767Z | Triggered by: pr_updated | Commit: 9c2fbe1

💡 TIP: You can chat with Visor using /visor ask <your question>

@probelabs

probelabs Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Security Issues (2)

Severity Location Issue
🟠 Error ctx/request_context.go:48-54
A race condition can occur in `GetRequestContext` if it's called concurrently on the same request before a `RequestContext` has been set. Multiple goroutines could simultaneously check for the context value, find it missing, and then proceed to create and assign a new request context. This leads to a data race on the `*r = *h` assignment, where the request struct is being written to by multiple goroutines without synchronization. This can cause unpredictable behavior, including panics or corrupted request state.
💡 SuggestionProtect the 'get or create' logic with a lock to ensure that only one `RequestContext` is created and assigned per request. A `sync.Mutex` associated with the request (e.g., via a map keyed by request pointer) or a double-checked locking pattern with atomic operations could be used. However, a simpler approach is to ensure `RequestContext` is always initialized at the start of the request lifecycle, making the creation logic in `GetRequestContext` a fallback that should ideally not be hit in concurrent scenarios. Given the current usage, where `proxy_muxer.go` initializes the context, this race is less likely but still possible if other code paths call `GetRequestContext` before initialization. A defense-in-depth approach would be to make the creation logic thread-safe.
🟡 Warning gateway/proxy_muxer.go:72
The `RequestContext` is initialized with an empty struct `&ctx.RequestContext{}`. This is immediately followed by calls to other middleware that might call `ctx.GetRequestContext(r)`. The `GetRequestContext` function contains logic to create a `RequestContext` if one doesn't exist. This creates two places where `RequestContext` can be initialized, which can be confusing and error-prone. The initialization should happen in one designated place. The `GetRequestContext` function should be the single source of truth for getting (and creating if necessary) the context.
💡 SuggestionRemove the explicit initialization in `gateway/proxy_muxer.go`. Let the first component that needs the `RequestContext` trigger its creation via `ctx.GetRequestContext(r)`. This centralizes the creation logic and avoids redundant initializations. The race condition in `GetRequestContext` should be fixed to support this pattern safely.

Security Issues (2)

Severity Location Issue
🟠 Error ctx/request_context.go:48-54
A race condition can occur in `GetRequestContext` if it's called concurrently on the same request before a `RequestContext` has been set. Multiple goroutines could simultaneously check for the context value, find it missing, and then proceed to create and assign a new request context. This leads to a data race on the `*r = *h` assignment, where the request struct is being written to by multiple goroutines without synchronization. This can cause unpredictable behavior, including panics or corrupted request state.
💡 SuggestionProtect the 'get or create' logic with a lock to ensure that only one `RequestContext` is created and assigned per request. A `sync.Mutex` associated with the request (e.g., via a map keyed by request pointer) or a double-checked locking pattern with atomic operations could be used. However, a simpler approach is to ensure `RequestContext` is always initialized at the start of the request lifecycle, making the creation logic in `GetRequestContext` a fallback that should ideally not be hit in concurrent scenarios. Given the current usage, where `proxy_muxer.go` initializes the context, this race is less likely but still possible if other code paths call `GetRequestContext` before initialization. A defense-in-depth approach would be to make the creation logic thread-safe.
🟡 Warning gateway/proxy_muxer.go:72
The `RequestContext` is initialized with an empty struct `&ctx.RequestContext{}`. This is immediately followed by calls to other middleware that might call `ctx.GetRequestContext(r)`. The `GetRequestContext` function contains logic to create a `RequestContext` if one doesn't exist. This creates two places where `RequestContext` can be initialized, which can be confusing and error-prone. The initialization should happen in one designated place. The `GetRequestContext` function should be the single source of truth for getting (and creating if necessary) the context.
💡 SuggestionRemove the explicit initialization in `gateway/proxy_muxer.go`. Let the first component that needs the `RequestContext` trigger its creation via `ctx.GetRequestContext(r)`. This centralizes the creation logic and avoids redundant initializations. The race condition in `GetRequestContext` should be fixed to support this pattern safely.
\n\n ### Architecture Issues (3)
Severity Location Issue
🟠 Error ctx/request_context.go:50-52
The logic for updating the request's context in-place (`*r = *r.WithContext(...)`) is duplicated here. A similar private helper `setCtxValue` exists in the `gateway` package. This duplication is necessary because of package boundaries but indicates a missing shared utility function for this common operation.
💡 SuggestionCreate a public helper function in a shared package (e.g., `internal/httpctx` or the `ctx` package itself) to encapsulate the logic of setting a value in the request context and updating the request pointer. Both this function and the helpers in the `gateway` package should then use this new shared utility to avoid code duplication and improve maintainability.
🟡 Warning ctx/request_context.go:46-52
The `GetRequestContext` function includes "create on-demand" logic, but the `RequestContext` is already explicitly created for every request in `gateway/proxy_muxer.go`. This creates two competing initialization strategies. The design should commit to either eager initialization at the request entry point or lazy initialization on first access, but not both.
💡 SuggestionTo simplify the design, either remove the explicit initialization from `gateway/proxy_muxer.go` and rely solely on this function's lazy initialization, or remove the creation logic from this function and have it assume the context has already been initialized. The latter would make the contract clearer and the system's behavior easier to reason about.
🟡 Warning ctx/request_context.go:46-52
The function `GetRequestContext` has a significant side effect: it modifies the `http.Request` object passed to it (`*r = *h`). Functions with a "Get" prefix are expected to be read-only. This behavior can be surprising and violates the principle of least astonishment.
💡 SuggestionRename the function to better reflect its behavior, such as `EnsureRequestContext`. Alternatively, change its signature to return the modified request object explicitly, e.g., `func GetRequestContext(r *http.Request) (*RequestContext, *http.Request)`, forcing the caller to acknowledge the change.

Performance Issues (1)

Severity Location Issue
🟡 Warning gateway/proxy_muxer.go:72
A new `RequestContext` object is allocated for every incoming request. In a high-throughput gateway, this can lead to significant garbage collector pressure. The `RequestContext` is a good candidate for pooling to reduce allocations on the hot path.
💡 SuggestionUse a `sync.Pool` to recycle `RequestContext` objects. Get an object from the pool at the beginning of `handleWrapper.ServeHTTP`, and use a `defer` to reset it and put it back into the pool after the request is handled. The existing `Reset()` method is well-suited for this pattern.

Example:

// At package level
var requestContextPool = sync.Pool{
    New: func() interface{} { return new(ctx.RequestContext) },
}

// In ServeHTTP
rc := requestContextPool.Get().(*ctx.RequestContext)
defer func() {
    rc.Reset()
    requestContextPool.Put(rc)
}()

// Replace the existing line with this:
ctxSetRequestContext(r, rc)

Powered by Visor from Probelabs

Last updated: 2026-07-20T10:28:12.122Z | Triggered by: pr_updated | Commit: 9c2fbe1

💡 TIP: You can chat with Visor using /visor ask <your question>

@sentinelone-cnapp-eu1

Copy link
Copy Markdown

SentinelOne CNS Hardcoded Secret Detector
✅ Congratulations, your code is safe

SentinelOne CNS is a cloud-agnostic, agentless CSPM & CWPP solution that continuously detects and prevents vulnerabilities that have the highest probability of being exploited in Azure, AWS, Google Cloud, and Kubernetes.

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