Skip to content

Commit 5cbaa41

Browse files
committed
fix(cpex): address PR #493 review — multi-part redaction leak + hardening
Resolve Hai Huang's review findings on the CPEX plugin PR: Must-fix (blocker): - applyA2AResponseBodyMod: fail closed when >1 text-kind artifact parts exist, mirroring the inference response guard (cmf_inference.go:204). Previously only the first part was rewritten; parts[1..] forwarded the original unredacted content. Security/correctness: - Decision zero-value flipped from Allow to a fail-closed Unknown sentinel (DecisionUnknown = iota). An uninitialised Result never silently allows traffic. - FakeManager default changed from Allow to Deny for test fidelity. - Streaming (SSE) response gap detection: when the response body is non-empty but yields zero CMF parts (unparseable SSE), return DecisionError so fail_open governs rather than silently allowing. - Dockerfile: cosign verify-blob of the FFI .a asset at build time (sha256 + signature verification). Runtime ABI assertion already exists in the Go bindings. Nits: - Stale "not yet implemented" comment removed (manager_cpex.go:269). - IBM/... → contextforge-org/... import path comment fixed. - realm-export.json: _WARNING field flagging demo-only credentials. - 30-agent.yaml: model pinned from :latest to ollama/llama3.2:8b. - README + cpex-plugin.md: clarify authbridge-cpex is a build variant (not a second sidecar); document OPA dual presence. Verified: go test ./plugins/cpex/ passes (33 tests); go vet clean; full hr-cpex demo 9/9 scenarios pass on kind cluster. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
1 parent e0676fd commit 5cbaa41

13 files changed

Lines changed: 237 additions & 37 deletions

File tree

authbridge/authlib/plugins/cpex/README.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
# CPEX
22

3+
> **Deployment note:** `authbridge-cpex` is a **build variant** of the
4+
> AuthBridge proxy-sidecar (`authbridge-proxy`), deployed *in place of*
5+
> `authbridge-proxy` — not an additional sidecar. The operator selects
6+
> the image when CPEX policy enforcement is needed for a workload.
7+
38
The CPEX plugin routes AuthBridge pipeline hooks through the [CPEX](https://github.com/contextforge-org/cpex) framework,
49
so operators can define authorization flows inline and declaratively, and turn decisions into ordered effects (including invoking CPEX plugins).
510

@@ -140,6 +145,16 @@ policy:
140145
Local structural checks run fast and first; external PDPs run when the local
141146
checks pass; their decisions feed the same `on_deny` / `on_allow` effect lists.
142147

148+
> **OPA in two places.** AuthBridge ships a native `opa` pipeline plugin
149+
> (standalone, no CPEX dependency) for simple per-request OPA checks.
150+
> The CPEX `opa(...)` step shown above runs OPA as a CPEX sub-plugin —
151+
> within APL's effect framework, so its verdict feeds `on_deny`/`on_allow`
152+
> effects, session tainting, and ordered composition with other PDPs.
153+
> Use the native `opa` plugin for standalone binary OPA gates; use
154+
> CPEX's `opa(...)` when OPA participates in a multi-PDP orchestration
155+
> flow. The `authzen(...)` and `cedar` steps are doc-level BYOP examples
156+
> not exercised in this demo.
157+
143158
## CMF / extension mapping
144159

145160
The AuthBridge pipeline context is mapped into the CMF Message plus

authbridge/authlib/plugins/cpex/cmf_a2a.go

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -139,18 +139,18 @@ func applyA2ARequestBodyMod(pctx *pipeline.Context, newTexts []string) (mutated
139139
}
140140

141141
// applyA2AResponseBodyMod rewrites pctx.ResponseBody — a non-streaming
142-
// A2A JSON-RPC response — replacing the first artifact text part with
143-
// newArtifact (the single redacted text part CPEX returned on the
144-
// response phase).
142+
// A2A JSON-RPC response — replacing the single artifact text part with
143+
// newArtifact (the redacted text CPEX returned on the response phase).
145144
//
146-
// A2A responses are frequently SSE streams (message/stream); those won't
147-
// parse as one JSON object, so a requested redaction can't be applied and
148-
// we return an error to fail closed rather than forward unredacted output.
149-
// For the non-streaming JSON-RPC shape it rewrites the first text part
150-
// under result.artifacts[].parts[].
145+
// Because the write side receives only one redacted text while the read
146+
// side (a2aResponseParts) emits one text part per non-empty text-kind
147+
// artifact part, a response carrying more than one such part is an
148+
// ambiguous single-value rewrite — we fail closed rather than overwrite
149+
// only the first part and forward the rest unredacted. Streaming (SSE)
150+
// responses don't parse as one JSON object and also fail closed.
151151
//
152-
// Returns mutated=false (no error) when newArtifact is empty or there's no
153-
// artifact text part to replace.
152+
// Returns mutated=false (no error) when newArtifact is empty or there's
153+
// no artifact text part to replace.
154154
func applyA2AResponseBodyMod(pctx *pipeline.Context, newArtifact string) (mutated bool, err error) {
155155
if len(pctx.ResponseBody) == 0 || newArtifact == "" {
156156
return false, nil
@@ -168,6 +168,11 @@ func applyA2AResponseBodyMod(pctx *pipeline.Context, newArtifact string) (mutate
168168
return false, nil
169169
}
170170

171+
// Collect all non-empty text-kind artifact parts, matching what the
172+
// read side surfaced. We only hold one redacted text, so >1 such
173+
// part is ambiguous: fail closed instead of stamping the same value
174+
// over distinct parts or rewriting only the first.
175+
targets := make([]map[string]any, 0, 4)
171176
for _, a := range artifacts {
172177
artifact, ok := a.(map[string]any)
173178
if !ok {
@@ -185,21 +190,31 @@ func applyA2AResponseBodyMod(pctx *pipeline.Context, newArtifact string) (mutate
185190
if kind, _ := po["kind"].(string); kind != "text" {
186191
continue
187192
}
188-
// Skip empty text parts so we rewrite the same part the read
189-
// side surfaced (a2aResponseParts emits only non-empty text).
190-
if t, ok := po["text"].(string); !ok || t == "" {
191-
continue
192-
}
193-
po["text"] = newArtifact
194-
newBody, err := json.Marshal(envelope)
195-
if err != nil {
196-
return false, fmt.Errorf("re-serialize A2A response body: %w", err)
193+
if t, ok := po["text"].(string); ok && t != "" {
194+
targets = append(targets, po)
197195
}
198-
pctx.SetResponseBody(newBody)
199-
return true, nil
200196
}
201197
}
202-
return false, nil
198+
199+
if len(targets) == 0 {
200+
return false, nil
201+
}
202+
if len(targets) > 1 {
203+
return false, fmt.Errorf(
204+
"A2A response has %d text parts; single-value redaction rewrite is ambiguous",
205+
len(targets))
206+
}
207+
if targets[0]["text"] == newArtifact {
208+
return false, nil
209+
}
210+
targets[0]["text"] = newArtifact
211+
212+
newBody, err := json.Marshal(envelope)
213+
if err != nil {
214+
return false, fmt.Errorf("re-serialize A2A response body: %w", err)
215+
}
216+
pctx.SetResponseBody(newBody)
217+
return true, nil
203218
}
204219

205220
// a2aMessageParts navigates a decoded A2A JSON-RPC request envelope to

authbridge/authlib/plugins/cpex/cmf_a2a_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,34 @@ func TestA2AResponseBodyMod_RewritesArtifact(t *testing.T) {
122122
}
123123
}
124124

125+
func TestA2AResponseBodyMod_MultiPartFailsClosed(t *testing.T) {
126+
// Two text-kind artifact parts → ambiguous single-value rewrite → error.
127+
pctx := &pipeline.Context{
128+
ResponseBody: []byte(`{"result":{"artifacts":[{"parts":[{"kind":"text","text":"part one"},{"kind":"text","text":"part two"}]}]}}`),
129+
}
130+
mutated, err := applyA2AResponseBodyMod(pctx, "redacted")
131+
if err == nil {
132+
t.Fatal("expected fail-closed error for multi-part response, got nil")
133+
}
134+
if mutated {
135+
t.Fatal("mutated=true on multi-part fail-closed")
136+
}
137+
}
138+
139+
func TestA2AResponseBodyMod_MultiArtifactMultiPartFailsClosed(t *testing.T) {
140+
// Text parts spread across multiple artifacts still triggers the guard.
141+
pctx := &pipeline.Context{
142+
ResponseBody: []byte(`{"result":{"artifacts":[{"parts":[{"kind":"text","text":"first"}]},{"parts":[{"kind":"text","text":"second"}]}]}}`),
143+
}
144+
mutated, err := applyA2AResponseBodyMod(pctx, "redacted")
145+
if err == nil {
146+
t.Fatal("expected fail-closed error for multi-artifact text parts, got nil")
147+
}
148+
if mutated {
149+
t.Fatal("mutated=true on multi-artifact fail-closed")
150+
}
151+
}
152+
125153
func TestA2AResponseBodyMod_StreamingFailsClosed(t *testing.T) {
126154
pctx := &pipeline.Context{
127155
ResponseBody: []byte("data: {\"result\":{}}\n\n"),

authbridge/authlib/plugins/cpex/cmf_body.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,17 @@ import (
1010
"github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline"
1111
)
1212

13+
// isStreamingResponseGap reports whether a response-phase invocation has a
14+
// non-empty body that yielded zero structured content parts — i.e. a
15+
// streaming (SSE) body the policy engine can't inspect. Callers use this
16+
// to fail closed rather than silently allowing unredacted content through.
17+
func isStreamingResponseGap(pctx *pipeline.Context, isResponse bool, cmfPartCount int) bool {
18+
if !isResponse || cmfPartCount > 0 || len(pctx.ResponseBody) == 0 {
19+
return false
20+
}
21+
return pctx.Extensions.MCP != nil || pctx.Extensions.Inference != nil || pctx.Extensions.A2A != nil
22+
}
23+
1324
// sessionIDFromHeaders returns the X-Session-Id request header, the
1425
// session-correlation key AuthBridge threads into CPEX's session resolver
1526
// (tier-0 Agent.SessionID) for non-A2A traffic. MCP and inference requests

authbridge/authlib/plugins/cpex/cmf_body_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,3 +573,74 @@ func TestSessionIDFromHeaders(t *testing.T) {
573573
t.Fatalf("got %q, want taint-demo-42", got)
574574
}
575575
}
576+
577+
// --- isStreamingResponseGap ---
578+
579+
func TestIsStreamingResponseGap_SSEBodyWithProtocolExtension(t *testing.T) {
580+
pctx := &pipeline.Context{
581+
ResponseBody: []byte("data: {\"result\":{}}\n\n"),
582+
Extensions: pipeline.Extensions{
583+
A2A: &pipeline.A2AExtension{Method: "message/stream"},
584+
},
585+
}
586+
if !isStreamingResponseGap(pctx, true, 0) {
587+
t.Fatal("expected streaming gap: SSE body, A2A extension, zero parts")
588+
}
589+
}
590+
591+
func TestIsStreamingResponseGap_InferenceSSE(t *testing.T) {
592+
pctx := &pipeline.Context{
593+
ResponseBody: []byte("data: {\"choices\":[{\"delta\":{}}]}\n\n"),
594+
Extensions: pipeline.Extensions{
595+
Inference: &pipeline.InferenceExtension{Model: "gpt-4"},
596+
},
597+
}
598+
if !isStreamingResponseGap(pctx, true, 0) {
599+
t.Fatal("expected streaming gap for inference SSE")
600+
}
601+
}
602+
603+
func TestIsStreamingResponseGap_NotOnRequestPhase(t *testing.T) {
604+
pctx := &pipeline.Context{
605+
ResponseBody: []byte("data: something\n\n"),
606+
Extensions: pipeline.Extensions{
607+
A2A: &pipeline.A2AExtension{Method: "message/stream"},
608+
},
609+
}
610+
if isStreamingResponseGap(pctx, false, 0) {
611+
t.Fatal("request phase should never report a streaming gap")
612+
}
613+
}
614+
615+
func TestIsStreamingResponseGap_PartsPresent(t *testing.T) {
616+
pctx := &pipeline.Context{
617+
ResponseBody: []byte(`{"result":{"artifacts":[{"parts":[{"kind":"text","text":"ok"}]}]}}`),
618+
Extensions: pipeline.Extensions{
619+
A2A: &pipeline.A2AExtension{Method: "message/send"},
620+
},
621+
}
622+
if isStreamingResponseGap(pctx, true, 1) {
623+
t.Fatal("should not report gap when parts were successfully extracted")
624+
}
625+
}
626+
627+
func TestIsStreamingResponseGap_EmptyBodyNoGap(t *testing.T) {
628+
pctx := &pipeline.Context{
629+
ResponseBody: nil,
630+
Extensions: pipeline.Extensions{
631+
A2A: &pipeline.A2AExtension{Method: "message/send"},
632+
},
633+
}
634+
if isStreamingResponseGap(pctx, true, 0) {
635+
t.Fatal("empty body should not be a streaming gap")
636+
}
637+
}
638+
639+
func TestIsStreamingResponseGap_NoProtocolExtension(t *testing.T) {
640+
pctx := &pipeline.Context{
641+
ResponseBody: []byte("data: something\n\n"),
642+
}
643+
if isStreamingResponseGap(pctx, true, 0) {
644+
t.Fatal("no protocol extension → no gap (unrecognised traffic)")
645+
}
646+
}

authbridge/authlib/plugins/cpex/manager.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212
//
1313
// The real implementation lives in manager_cpex.go (//go:build cpex)
1414
// and wraps the cpex.PluginManager from the
15-
// github.com/IBM/contextforge-plugins-framework/go/cpex package. The
15+
// github.com/contextforge-org/cpex/go/cpex package. The
1616
// stub in manager_stub.go (//go:build !cpex) makes NewManager error
1717
// out with a clear "build the cpex binary" message if anyone tries to
1818
// configure the plugin in a binary that wasn't built with -tags cpex.
@@ -88,9 +88,15 @@ type ManagerOptions struct {
8888
type Decision int
8989

9090
const (
91+
// DecisionUnknown is the zero-value sentinel. A Result whose
92+
// Decision was never explicitly set fails closed in applyDecision
93+
// (the default arm), so an uninitialised Result never silently
94+
// allows traffic.
95+
DecisionUnknown Decision = iota
96+
9197
// DecisionAllow: all sub-plugins continued. Request proceeds
9298
// untouched.
93-
DecisionAllow Decision = iota
99+
DecisionAllow
94100

95101
// DecisionDeny: at least one sub-plugin returned a policy
96102
// violation. Plugin emits pipeline.Deny.
@@ -113,6 +119,8 @@ const (
113119
// reasons and log fields.
114120
func (d Decision) String() string {
115121
switch d {
122+
case DecisionUnknown:
123+
return "unknown"
116124
case DecisionAllow:
117125
return "allow"
118126
case DecisionDeny:

authbridge/authlib/plugins/cpex/manager_cpex.go

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,18 @@ func (c *cpexManager) Invoke(_ context.Context, hookName string, pctx *pipeline.
114114
isResponse := pctx.CurrentPhase() == pipeline.InvocationPhaseResponse
115115
payload, ext := buildCMF(pctx, isResponse)
116116

117+
// Streaming-response guard: when we're on the response phase, the
118+
// body is non-empty, a protocol parser claimed this traffic, yet the
119+
// CMF message has zero content parts — the body is an SSE stream
120+
// that the policy engine can't inspect. Rather than silently allowing
121+
// the unredacted stream through (the policy had nothing to evaluate),
122+
// surface a DecisionError so the fail_open knob governs: the default
123+
// fail-closed denies, fail_open=true logs and allows.
124+
if isStreamingResponseGap(pctx, isResponse, len(payload.Message.Content)) {
125+
return Result{Decision: DecisionError, Reason: "streaming response body not inspectable by policy"},
126+
fmt.Errorf("cpex: response body present (%d bytes) but yielded zero content parts — likely SSE stream; failing closed", len(pctx.ResponseBody))
127+
}
128+
117129
// Fused identity-resolve + hook invoke. cpex-core runs the identity
118130
// resolvers (jwt-user / jwt-client) ONLY on the identity.resolve hook,
119131
// never inside a tool/prompt/resource hook. An FFI host must therefore
@@ -265,8 +277,8 @@ func awaitBackground(hook, reqID string, bg *rcpex.BackgroundTasks) {
265277
}
266278

267279
// applyModificationsToPctx writes CPEX's modified Extensions and body
268-
// back onto pctx. MCP body modifications are re-serialized; inference /
269-
// A2A body rewriting is not yet implemented and fails closed (see
280+
// back onto pctx. MCP, inference, and A2A body modifications are each
281+
// re-serialized via format-aware write-back functions (see
270282
// applyBodyModFromCMF).
271283
//
272284
// Extension changes applied:

authbridge/authlib/plugins/cpex/manager_test.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ type FakeManager struct {
2525

2626
// Hooks maps hook name → canned Result. Invoke returns the
2727
// canned Result when the hook name matches; otherwise it
28-
// returns DecisionAllow with a "no hook configured" reason.
28+
// returns DecisionDeny (fail-closed default for test fidelity).
2929
Hooks map[string]Result
3030

3131
// InvokeErr, if non-nil, is returned from every Invoke before
@@ -107,7 +107,10 @@ func (f *FakeManager) HasHook(name string) bool {
107107
}
108108

109109
// Invoke records the call and returns InvokeErr (if set), the canned
110-
// Result for hookName (if in Hooks), or a default DecisionAllow.
110+
// Result for hookName (if in Hooks), or a default DecisionDeny. The
111+
// deny default mirrors production fail-closed semantics: tests must
112+
// explicitly wire Hooks with Allow/Modify results for the paths they
113+
// exercise, so an unconfigured hook is never silently permitted.
111114
func (f *FakeManager) Invoke(_ context.Context, hookName string, pctx *pipeline.Context) (Result, error) {
112115
f.mu.Lock()
113116
defer f.mu.Unlock()
@@ -118,5 +121,5 @@ func (f *FakeManager) Invoke(_ context.Context, hookName string, pctx *pipeline.
118121
if r, ok := f.Hooks[hookName]; ok {
119122
return r, nil
120123
}
121-
return Result{Decision: DecisionAllow, Reason: "fake: no hook configured"}, nil
124+
return Result{Decision: DecisionDeny, Reason: "fake: no hook configured (deny by default)"}, nil
122125
}

authbridge/authlib/plugins/cpex/plugin_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,26 @@ func TestDispatch_ErrorDecisionFailOpen(t *testing.T) {
529529
}
530530
}
531531

532+
func TestDispatch_UnknownDecisionFailsClosed(t *testing.T) {
533+
// A Decision value the switch doesn't recognise (including the
534+
// zero-value DecisionUnknown) must fail closed via the default arm.
535+
fake := &FakeManager{
536+
KnownHooks: []string{HookToolPreInvoke},
537+
Hooks: map[string]Result{
538+
HookToolPreInvoke: {Decision: DecisionUnknown, Reason: "zero-value sentinel"},
539+
},
540+
}
541+
cfg := `{"hooks":{"on_request":["cmf.tool_pre_invoke"]},"fail_open":false}`
542+
p := setupAndInit(t, fake, cfg)
543+
a := p.OnRequest(context.Background(), &pipeline.Context{})
544+
if a.Type != pipeline.Reject {
545+
t.Fatalf("Type = %d, want Reject (DecisionUnknown must fail closed)", a.Type)
546+
}
547+
if a.Violation == nil || a.Violation.Code != "cpex.error" {
548+
t.Fatalf("want Violation code=cpex.error, got %#v", a.Violation)
549+
}
550+
}
551+
532552
func TestSanitizeReason(t *testing.T) {
533553
cases := []struct {
534554
in string

0 commit comments

Comments
 (0)