oscerd opened a new pull request, #27073:
URL: https://github.com/apache/camel/pull/27073
## What
With `failOpen=true`, camel-opa let an exchange through on **every** failure
to evaluate the policy. The option promises less: it should apply only when
*"the policy cannot be evaluated at all, for example because the OPA server is
unreachable"*.
The OPA Java SDK (`com.styra:opa` 2.1.1, checked in the bytecode) reports
these cases through `OPAClient.evaluate`:
| OPA's answer | how the SDK reports it | before, under `failOpen` |
|---|---|---|
| undefined decision (rule has no default and does not match) |
`OPAException` with no cause | **allow** |
| `400` | `OPAException` ← `ClientError` | **allow** |
| `401` / `403` (wrong or expired `bearerToken`), `404`, other `4xx` |
`OPAException` ← `SDKError(code)` | **allow** |
| `500`, how OPA reports an error evaluating the policy against this input |
`OPAException` ← `ServerError` | **allow** |
| connection refused / timeout | `OPAException` ← `IOException` | allow |
| `502` / `503` / `504` / `429` from a gateway | `OPAException` ←
`SDKError(code)` | allow |
The undefined case is the sharpest. It is how Rego behaves for
`opa:authz/allow` against a policy without `default allow := false`, so with
`failOpen` every input the rule does *not* match was allowed. The WASM engine
throws on an undefined rule the same way, deliberately, to match REST, so it
had the same gap. Input documents are built from the message, so a sender can
shape most of these answers.
`failOpen` is `insecure:dev` and documented as not for production, and
camel-opa is not released yet (4.23.0). So this is a bug fix before release,
not a vulnerability. It is the same class as the finding in the camel-openfga
review (#26974), where a `4xx` was also read as "no verdict" under `failOpen`.
## How
`failOpen` now applies only when the decision point was **unavailable**.
Everything else fails closed even with `failOpen` set.
- `OpaPolicyEvaluator.isDecisionPointUnavailable(Exception)`: the default is
`false`, so an engine fails closed unless it recognises its own unavailability.
It is used by single evaluation, whole-batch failure and per-element batch
failure. The fail-closed path is unchanged: every failure still fails closed
without `failOpen`.
- `OpaRestEvaluator` walks the cause chain. It returns `true` for an
`IOException` (including connect and request timeouts) and for `SDKError`
429/502/503/504. It returns `false` for `ClientError`, `ServerError`,
`AuthException`, any other `SDKError`, a cause-less `OPAException` (an
undefined decision), and a Jackson exception, which is an `IOException` of its
own and would otherwise pass for a transport failure.
- `OpaWasmEvaluator` returns `true` only for a pool `TimeoutException` (busy
past `borrowTimeout`). An undefined rule, a trap and a serialization failure
fail closed.
- The docs are updated in the option javadocs (`failOpen`, `batch`;
regenerated metadata) and in the component's *Failure handling* section, which
now lists what `failOpen` covers and what it never covers.
`CamelOpaDecisionFailedOpen` still marks every exchange that proceeds
through `failOpen`.
## Tests
- `OpaSdkFailures` (new test helper) builds each failure exactly the way the
SDK does. The existing tests stubbed "unreachable" as `new
OPAException("connection refused")`, which is the SDK's shape for an *undefined
decision*. They now use the real one.
- `OpaProducerTest` (unit tests):
- fails open on a timeout and on 429/502/503/504;
- fails closed under `failOpen` on an undefined decision, a 400, a
401/403/404/413, a 500 evaluation error, and an input the SDK cannot serialize.
- `OpaBatchEvaluationTest`:
- an element that OPA evaluated with an error, or that came back
undefined, is denied under `failOpen`;
- an element whose server was unreachable (the SDK's per-element fallback)
is still allowed;
- a whole batch rejected with 401 fails closed.
- `OpaSecurityPolicyTest`: the fail-open policy refuses an undefined
decision.
- ITs against a real OPA server and real WASM bundles:
- `OpaIT`: an undefined decision under `failOpen` fails closed, and an
unreachable server still fails open through the real SDK's wrapping;
- `OpaWasmIT`: an undefined rule under `failOpen` fails closed;
- `OpaBearerTokenIT`: a wrong token under `failOpen` fails closed.
- Revert check: with the old evaluators and the new tests, all 14
fail-closed-under-`failOpen` tests fail, including the three real-server ITs,
and the fail-open tests still pass.
- `mvn clean install` in `components/camel-opa`: 108 unit tests and 30 ITs
green. Full reactor `mvn clean install -DskipTests -DskipITs`: green, with no
generated-file drift beyond the opa catalog and DSL entries this change
regenerates.
## Scope
Main only, since camel-opa is new in 4.23.0. No upgrade-guide entry: there
is no earlier release to migrate from.
JIRA: https://issues.apache.org/jira/browse/CAMEL-25139
_Claude Code on behalf of @oscerd_
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
