oscerd opened a new pull request, #26469:
URL: https://github.com/apache/camel/pull/26469
## What
`camel-opa` clears its decision headers on entry to `evaluate` instead of
relying on overwriting them at the end, so a verdict a message claimed for
itself cannot survive a failed evaluation.
## The gap
The component writes `CamelOpaDecisionAllow` (plus `CamelOpaDecision` and
`CamelOpaPolicyPath`) *after* reaching a verdict:
```java
boolean allowed = isAllowed(decision);
setDecisionHeaders(exchange, decision, allowed);
```
On the allow and deny paths that is enough — an inbound claim is
overwritten. `buildInput` already excludes the three headers
case-insensitively, so a claimed verdict never reaches OPA or its decision log
either. Both were deliberate.
**The failure paths never get there.** When the policy cannot be evaluated
at all — server unreachable, decision undefined, input not serializable —
`evaluate` throws and the headers are untouched:
```java
throw new OpaPolicyEvaluationException("Failed to evaluate policy " +
policyPath, exchange, e);
```
Whatever the message carried is still on it. `OpaSecurityPolicy` throws
`CamelAuthorizationException` through the same shared `evaluate`, so it has the
same gap.
## Why it matters
A route that lets the exception propagate is unaffected — the exchange is
failed. A route that **handles** it is not: `doTry`/`doCatch`, or
`onException(...).handled(true)` / `.continued(true)`, resumes routing with the
sender's own `CamelOpaDecisionAllow=true` still set. A downstream step reads
"allowed", attributed to a policy that never said so — and the component
documents that header as how a route acts on the verdict, with a filter or a
choice.
A route author setting the header is trusted and out of scope. The reachable
case is a consumer in front of the route that maps untrusted input into the
header map without a strict, case-insensitive `Camel*` `HeaderFilterStrategy` —
the header-injection class the security model puts explicitly in scope, and the
reason a security component should not depend on the consumer having got that
right.
## Fix
```java
// a verdict the message arrived with is a claim, not evidence. Clear it
before deciding anything, so that
// every way out of this method - allowed, denied, or a failure the route
goes on to handle - leaves only
// what this component decided. Overwriting at the end is not enough: the
paths that throw never get there
clearDecisionHeaders(exchange);
```
Clearing rather than overwriting also makes the invariant total and
one-line-checkable: these headers are only ever written by this component.
## Testing
Three cases, all in existing classes:
- `doesNotLeaveAVerdictClaimedByTheMessageBehindWhenEvaluationFails` — the
reproduction. Confirmed to **fail with the fix reverted**; the pre-existing
`failsClosedWhenThePolicyCannotBeEvaluated` does not catch it, because it never
sets the header inbound.
- `replacesAVerdictClaimedByTheMessageWithTheOneThePolicyGave` — pins the
allow/deny path, which was already correct.
- `doesNotForwardAVerdictClaimedByTheMessageToTheDecisionPoint` — pins the
`buildInput` exclusion. It passed first time: the behaviour was already right
but untested, so it was one refactor from being lost.
Full reactor build green.
## Scope
`main` only. `camel-opa` is new and unreleased in 4.23.0, so no released
behaviour changes and no advisory is warranted.
Related: **CAMEL-24738** proposes a `CamelOpaDecisionFailedOpen` header so a
route can tell "allowed by policy" from "allowed because the decision point was
down". If that lands, it needs clearing here too — otherwise a sender could
preload `FailedOpen=false` and make a fail-open look like a real allow.
_Claude Code on behalf of @oscerd_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
