Skip to content

hardening(sidecar): Phase 1/2 transport, validation, and normalization fixes#35

Open
Ankit-Kotnala wants to merge 1 commit into
c2siorg:mainfrom
Ankit-Kotnala:hardening/phase1-2-sidecar
Open

hardening(sidecar): Phase 1/2 transport, validation, and normalization fixes#35
Ankit-Kotnala wants to merge 1 commit into
c2siorg:mainfrom
Ankit-Kotnala:hardening/phase1-2-sidecar

Conversation

@Ankit-Kotnala
Copy link
Copy Markdown

@Ankit-Kotnala Ankit-Kotnala commented Apr 9, 2026

Summary

This PR implements Phase 1/2 hardening improvements in the Go sidecar without introducing any architectural changes.

Changes

  • Transport: adds MaxPayloadSize guard and rejects oversized requests before unsafe allocation/read
  • Nonce: ensures safe fallback for zero or negative TTL values
  • Validation:
    • adds stricter checks for on_tool_call and on_memory
    • introduces new hard-block validation signals
  • Pipeline: preserves fail-closed behavior in non-strict mode (hard-block always results in BLOCK)
  • Normalization:
    • recursive, deterministic payload extraction
    • bounded chained decoding (URL/Base64/hex)
    • removal of invisible/bidi Unicode control characters
  • Configuration: adds signal weights for new validation signals
  • Tests: adds coverage for all of the above changes

Scope

  • No OPA changes
  • No telemetry changes
  • No Python SDK changes
  • No API or wire-format changes

Why

These changes improve the robustness and security of the sidecar by:

  • Preventing oversized payload attacks (DoS risk)
  • Ensuring safe handling of invalid TTL values
  • Strengthening validation for critical signals
  • Making normalization deterministic and bounded

Impact

  • No architectural changes
  • Backward compatible
  • Fail-closed behavior preserved

Notes

  • Prompt and context validation remain intentionally tolerant
  • Tool and memory hooks are validated more strictly where payload contracts are well-defined
  • Original payload is not mutated; only CanonicalText is derived

How to Test

  • Run existing tests: go test ./...
  • Send oversized payload → should be rejected
  • Test TTL = 0 or negative → fallback behavior applied
  • Validate normalization on encoded inputs (URL/Base64/hex)

@Ankit-Kotnala
Copy link
Copy Markdown
Author

@tharindupr
Hi! I've opened a PR for Phase 1 and 2 hardening on the Go sidecar (transport, validation, pipeline, and normalization).
Would really appreciate your review whenever you get a chance. Thanks!

@VibhorGautam
Copy link
Copy Markdown

nice scoping on this one, the transport guard and the non-strict-mode fail-closed behaviour are both things i was worrying about after reading phase 2. a few observations for your consideration:

on MaxPayloadSize in transport. what's the current default and does it land in sidecar.example.yaml? if it doesn't, a fresh clone running go run ./cmd/sidecar will either inherit a hidden constant or zero-default to block everything, and that'll surprise future contributors. worth documenting the chosen value next to the existing socket_path entry

on the nonce TTL fallback. good catch on the zero/negative case. is there a test that demonstrates the fallback actually kicks in? i saw nonce_test.go grew by 24 lines but didn't page through to see if one of them asserts the "negative TTL gets clamped to default" path specifically. if not, worth adding

on the normalise_test expansion. 238 insertions to normalise.go with 82 deletions is a big delta for a hardening pr. worth calling out in the pr body what changed in the leetspeak/zero-width logic vs what's pure refactor, since phase 2 just landed and the normalise path is sensitive. otherwise reviewers have to diff the diff to understand the behaviour delta

on validate.go growth. the new hard-block signals for on_tool_call and on_memory are the right direction. one question: are the new signal names in signal_weights in the default config? if a signal is emitted but has no weight, the aggregator treats it as zero. easy to miss on review and only surfaces as a silent fail-open

overall this is the kind of small-surface, tests-attached pr that lands cleanly. the style of grouping hardening into one commit makes it easy for phase-3 code to rebase on top. nothing blocking from my side, just the documentation + test-coverage questions above

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