allthingssecurity commented on PR #28539:
URL: https://github.com/apache/camel/pull/28539#issuecomment-6435568414

   @davsclaus thank you. All points are in f3da6ae6505b, one commit on top of 
the rebased branch (a05bffc9e4ed).
   
   1. **Verdict for consumers that redeliver.** Your proposal is in "The 
report". Attempts are not tied to one message:
      each exchange is counted once, so a file that fails and then completes is 
one failed and one completed exchange.
      Tying them by a key is not in the first version. A failed exchange shows 
as "failed, kept by the consumer" or as
      "failed". The plugin reads the endpoint options through the generated 
configurer (`PropertyConfigurerGetter`), so
      options set on the component count too. A table lists each consumer. 
Without an expectation the verdict is PASS
      and nothing is "lost". Only an unmet expectation gives FAIL, and "lost" 
only comes from one. The Summary says the
      same. Three corrections to the list, from the code:
      - file/ftp: `noop=true` keeps the file too. 
`GenericFileOnCompletion.processStrategyRollback` removes it from the
        idempotent repository, so the next poll reads it again. Only 
`moveFailed` means "not kept".
      - Kafka: "without a commit" is `breakOnFirstError=true` (see its Javadoc 
in `KafkaConfiguration`). With the default
        `false` the consumer goes on to the next record, so the report shows 
"failed".
      - JMS `CLIENT_ACKNOWLEDGE` keeps the message only with 
`asyncConsumer=false` (the default).
        `EndpointMessageListener` rethrows only when it processes synchronously.
   2. **MCP and console.** I checked all of these on main: `RuntimeTools` is 
`@McpSecured`; `route_control`, `sql`,
      `send` and `receive` have `destructiveHint = true`; 
`McpSecurityConfig.AccessLevel.permits` refuses destructive
      tools at `read-write`; the `ToolRegistry` uses `.destructive(true)`. One 
caveat: that filter applies only with
      `camel.mcp.security.enabled=true`, which defaults to false (and the 
access level to `admin`). The hints per chaos
      tool are now in a table. We did take the console direction, with one 
change: the `chaos` console only lists and
      reports, so it is `@DevConsole(readOnly = true)`, not `false`. `/q/dev/*` 
takes GET and POST and never checks the
      flag (`ManagementHttpServer.setupDevConsole`). So add, change and remove 
are a `chaos` action of the CLI connector
      (`LocalCliConnector.dispatch`), and the MCP tools go through that action. 
On the TUI: it does not ask by the
      destructive flag today. Its AI tools ask only for the calls in 
`ToolConfirms` (CAMEL-25676), so an AI route stop
      is not asked. The doc proposes adding chaos add and change to 
`ToolConfirms`, and to "Developer tools" in the
      security model.
   3. **No profile.** Flipped as you suggest. The hooks are installed only with 
profile `dev` or
      `camel.chaos.enabled=true`, and never with `prod`. The default is `false` 
(`true` with `dev`), so with no profile
      nothing is installed. I verified both facts: 
`ProfileConfigurer.configureMain` returns early, and the `--profile`
      option in `Run.java` defaults to `dev`. Open question 2 is now marked as 
decided.
   4. **Where it lives.** A new "Getting it" section. One correction: `--stub` 
adds no dependency, it only sets the stub
      pattern. `--observe` is the closer precedent: it adds 
`camel:observability-services`, so `--chaos` adds
      `camel:chaos` the same way (`--dep camel:chaos` also works). On Spring 
Boot and Quarkus the plugin works with any
      `CamelContext`, but the options are camel-main only, so a starter or an 
extension can bind them later. "`camel
      run --chaos` only, no module" is now under Alternatives, with the reason.
   5. **`error status=`.** With `throwExceptionOnFailure=false`, chaos returns 
the response instead of throwing:
      `CamelHttpResponseCode`, the `header.*` values, and a new `body=` 
parameter.
   6. **DST.** Timed actions (the resume after a stall, rule expiry) run on an 
`ExecutorServiceManager` scheduler. The
      report reads its times through `TimeSource`, and event timestamps stay on 
`System`. The doc states the one-stack
      limit of `test-dst.adoc`, and that a world with lifecycle rules uses 
`DstCamel.asyncStep`.
   7. **`SecurityUtils`.** Confirmed: `SECURITY_OPTIONS` matches the bare name 
after the last dot, and
      `SchematronEndpoint` has `@UriParam rules`. The module PR limits the 
entry to keys under `camel.chaos.`.
   8. **Step hook JIRA.** The ticket is drafted but not filed yet. I will add 
its key to `see-also` once it is filed.
      Writing the draft turned up one fix to the doc: the `DisabledAware` check 
runs before the step's error handler
      (`DefaultChannel` runs its advices, then the error handler, then the 
processor). An exception set there would skip
      `onException`. So the doc now says the failure is raised where the error 
handler calls the step.
   9. **AGENTS.md.** The line now says "(proposed; ...)". No other line marks a 
status
      (`cross-cutting-binding.adoc` is `proposed` too, and its line is 
unmarked).
   
   The doc is about 23 KB.
   
   _Claude Code on behalf of allthingssecurity_
   


-- 
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]

Reply via email to