Skip to content

refactor(proxy): unify inject target matching with rulesengine#201

Merged
SasSwart merged 2 commits into
sasswart/feat-proxy-inject-session-headersfrom
sasswart/refactor-unify-inject-target-matching
May 12, 2026
Merged

refactor(proxy): unify inject target matching with rulesengine#201
SasSwart merged 2 commits into
sasswart/feat-proxy-inject-session-headersfrom
sasswart/refactor-unify-inject-target-matching

Conversation

@SasSwart
Copy link
Copy Markdown
Contributor

PR Map

  1. feat(audit): session ID generation, sequence counter, and BoundaryLog wiring #196
  2. feat(config): session correlation header injection configuration #197
  3. feat(proxy): inject session ID and sequence number headers on matching requests #198
  4. 👉🏼 This PR
  5. test(proxy): integration tests for session correlation audit and header agreement #199

Addresses review feedback from @evgeniy-scherbina: --allow and --session-id-inject-target used different implementations for parsing and matching domains/paths, leading to divergent behavior.

This PR replaces the custom shouldInjectHeaders matching with the existing rulesengine.Engine. Inject targets now use the same domain=... path=... syntax and identical matching semantics as --allow rules.

Depends on #198.

Changes

  • config/session_correlation.go: Remove InjectTarget struct and ParseInjectTarget. SessionCorrelationConfig.InjectTargets is now []string (raw rule specs, same syntax as --allow). DefaultInjectTargetFromEnv returns a rule string. ValidateSessionCorrelation delegates parsing to rulesengine.ParseAllowSpecs.
  • config/config.go: Simplify buildSessionCorrelation to pass raw strings instead of parsing into structs.
  • proxy/proxy.go: Replace sessionCorrelation field with injectEngine *rulesengine.Engine. shouldInjectHeaders delegates to injectEngine.Evaluate instead of hand-rolled matching. Add InjectEngine to proxy.Config.
  • proxy/proxy_framework_test.go: Build inject engine from session correlation targets in test setup.
  • proxy/proxy_session_correlation_test.go: Update all tests to use raw rule strings.
  • config/session_correlation_test.go: Update all tests for the new []string type.

Behavioral differences resolved

Issue Before After
Domain case sensitivity inject: case-insensitive; allow: case-sensitive Both case-sensitive
Wildcard/subdomain support inject: none; allow: *.example.com Both support wildcards
Trailing * path semantics inject: path.Match (single segment); allow: greedy Both greedy
Comma-separated paths inject: unsupported; allow: supported Both supported
Trailing-slash normalization inject: none; allow: normalized Both normalized
Input validation inject: minimal; allow: RFC 3986 Both RFC 3986
Port in domain pattern inject: allowed, stripped at match; allow: rejected at parse Both rejected at parse

Note

This PR was authored by Coder Agents.

Replace the custom shouldInjectHeaders domain/path matching with the
existing rulesengine engine. Inject targets now use the same
"domain=... path=..." syntax and identical matching semantics as
--allow rules.

Changes:
- config: Remove InjectTarget struct and ParseInjectTarget. Change
  SessionCorrelationConfig.InjectTargets from []InjectTarget to
  []string (raw rule specs). DefaultInjectTargetFromEnv returns a
  string. ValidateSessionCorrelation delegates parsing to
  rulesengine.ParseAllowSpecs.
- config: Simplify buildSessionCorrelation to pass raw strings.
- proxy: Replace sessionCorrelation field with injectEngine
  (*rulesengine.Engine). shouldInjectHeaders now delegates to
  injectEngine.Evaluate instead of hand-rolled matching.
- proxy: Add InjectEngine to proxy.Config, built from
  SessionCorrelation.InjectTargets at construction time.
- tests: Update all session correlation tests to use raw rule strings.

This resolves the behavioral differences between inject targets and
allow rules: case sensitivity, wildcard/subdomain support, path
glob semantics, trailing-slash normalization, and input validation
are now identical.
The behavior is already documented on the Config.ForwardTransport
field. Addresses review feedback from johnstcn.
@SasSwart SasSwart marked this pull request as ready for review May 12, 2026 12:23
@SasSwart
Copy link
Copy Markdown
Contributor Author

This refactor has simplified the branch that is based on. To streamline review, I'm merging this directly into #198 now.

@SasSwart SasSwart merged commit 6b424f1 into sasswart/feat-proxy-inject-session-headers May 12, 2026
@SasSwart SasSwart deleted the sasswart/refactor-unify-inject-target-matching branch May 12, 2026 12:24
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