fix(security): access control, A2A ownership, GDPR & audit ledger (wave 2a)

AGENTS.md

@@ -230,14 +230,14 @@ When tasks process templates (system prompts, HTTP call bodies, property instruc
 
 | Key                | Type                                         | Source                                                                     | Example Access                                   |
 | ------------------ | -------------------------------------------- | -------------------------------------------------------------------------- | ------------------------------------------------ |
-| `context`          | `Map<String, Object>`                        | Input context variables set per turn                                       | `{{context.language}}`                           |
+| `context`          | `Map<String, Object>`                        | Input context variables set per turn                                       | `{context.language}`                           |
 | `properties`       | `Map<String, Object>`                        | Conversation properties — raw values from `ConversationProperties.toMap()` | `{properties.preferred_language}`                |
 | `memory`           | `Map` with `current`, `last`, `past`         | Conversation step data from the pipeline                                   | `{memory.current.output}`, `{memory.last.input}` |
-| `snippets`         | `Map<String, Object>`                        | Prompt Snippets — auto-injected from `PromptSnippetService`                | `{{snippets.cautious_mode}}`                     |
-| `vars`             | `Map<String, Object>`                        | Global Variables — deployment-wide config from `GlobalVariableResolver`    | `{{vars.default-model}}`                         |
-| `userInfo`         | `Map` with `userId`                          | Authenticated user identity                                                | `{{userInfo.userId}}`                            |
-| `conversationInfo` | `Map` with `conversationId`, `agentId`, etc. | Conversation metadata                                                      | `{{conversationInfo.agentId}}`                   |
-| `conversationLog`  | `String`                                     | Formatted conversation history                                             | `{{conversationLog}}`                            |
+| `snippets`         | `Map<String, Object>`                        | Prompt Snippets — auto-injected from `PromptSnippetService`                | `{snippets.cautious_mode}`                     |
+| `vars`             | `Map<String, Object>`                        | Global Variables — deployment-wide config from `GlobalVariableResolver`    | `{vars.default-model}`                         |
+| `userInfo`         | `Map` with `userId`                          | Authenticated user identity                                                | `{userInfo.userId}`                            |
+| `conversationInfo` | `Map` with `conversationId`, `agentId`, etc. | Conversation metadata                                                      | `{conversationInfo.agentId}`                   |
+| `conversationLog`  | `String`                                     | Formatted conversation history                                             | `{conversationLog}`                            |
 
 > **Key insight**: `longTerm` properties are loaded into `conversationProperties` at conversation init and are immediately available via `{properties.key}` in any template. You do NOT need a separate template namespace for persistent data — properties IS the namespace.
 
@@ -251,7 +251,7 @@ Properties have a well-defined lifecycle managed by `Conversation.java`:
        └─→ IUserMemoryStore.getVisibleEntries(userId, agentId, groupIds, recallOrder, maxEntries)
        └─→ Visibility scoping: self + group + global entries are loaded
        └─→ Converted to Property objects with scope=longTerm
-       └─→ Available as {{properties.key}} in all templates
+       └─→ Available as {properties.key} in all templates
 
 2. Pipeline runs
    └─→ PropertySetterTask sets properties based on actions
@@ -340,6 +340,8 @@ A new `ILifecycleTask` requires ALL of:
 - [ ] `ExtensionDescriptor` (UI field definitions via `getExtensionDescriptor()`)
 - [ ] Unit test with Mockito
 
+> **Note on `@ConfigurationUpdate`:** it is declared in `IResourceStore` as an `@InterceptorBinding`, but **no `@Interceptor` class currently implements it** — today the annotation has no runtime behaviour and is purely a marker documenting "this method mutates stored configuration". Keep applying it for consistency with the existing stores, but do **not** rely on it to invalidate caches or fire events; caches such as `PromptSnippetService` use an explicit `invalidateCache()` plus a Caffeine TTL instead. Whether to implement the interceptor or drop the annotation is still open.
+
 All task implementations MUST implement: `getId()` (returns `TaskId`), `getType()`, `execute()`, `configure()`, `getExtensionDescriptor()`.
 
 ### 4.4 Code Patterns
@@ -808,7 +810,7 @@ Matcher:      "actions" : "ask_for_model"
 #### Qute template safety in HTTP call bodies
 
 When embedding `{properties.x}` in HTTP call body templates, be aware:
-- `quarkus.qute.strict-rendering=false` renders missing properties as empty strings (no error)
+- `quarkus.qute.strict-rendering=false` renders missing properties as empty strings (no error). This is set once in `application.properties` and applies to **every profile** — there is deliberately no `%prod` override, so dev, test and production all render leniently and fail identically. (Earlier releases turned strict rendering **on** in prod only, which meant a missing property rendered blank in dev but leaked the raw `{properties.x}` literal to the end user in production.)
 - Do NOT use `.orEmpty` on properties — it's for Qute iterables, not strings, and fails on `NOT_FOUND`
 - User-entered text containing `{` or `}` will be interpreted as Qute expressions, potentially eating content
 

docs/architecture.md

@@ -896,7 +896,7 @@ EDDI's memory model extends beyond single conversations. The `IUserMemoryStore`
 
 **How it integrates with the pipeline:**
 
-- At **conversation init**, visible user memories are loaded as `longTerm` properties and made available in all templates via `{{properties.key}}`
+- At **conversation init**, visible user memories are loaded as `longTerm` properties and made available in all templates via `{properties.key}`
 - During the pipeline, the LLM can autonomously store and recall facts using built-in memory tools (when enabled)
 - At **conversation teardown**, `longTerm` properties are persisted back to the user memory store
 - **Background consolidation** (the "Dream" service) performs scheduled maintenance: stale pruning, contradiction detection, and optional LLM-driven summarization

docs/attachments-guide.md

@@ -279,7 +279,7 @@ Use `contentTypeMatcher` to create different workflows based on attachment type:
 Attachments are available in templates via the memory namespace:
 
 ```
-Current step attachments: {{memory.current.attachments}}
+Current step attachments: {memory.current.attachments}
 ```
 
 This can be useful for logging, debugging, or constructing custom prompts that reference attachment metadata.

docs/behavior-rules.md

@@ -211,6 +211,13 @@ Inverts the overall outcome of the children conditions
 
 In some cases it is more relevant if a `condition` is `false` than if it is `true`, this is where the `negation` `condition` comes into play. The logical result of all children together (`AND` connected), will be _**inverted**_.
 
+This is exactly what the engine does: every child is evaluated in order, the first child that _fails_ makes the negation succeed (the `AND` is already false), and the negation only fails when _every_ child succeeded. Multi-child negations therefore behave as documented — earlier EDDI versions only looked at the first child, which made a negation with more than one child effectively always true.
+
+Two edge cases are worth knowing:
+
+- A `negation` **must** declare at least one nested condition. An empty `negation` is rejected at configuration validation time.
+- If a child reports `ERROR` or `NOT_EXECUTED` (e.g. a `sizematcher` where every bound is `-1`), there is nothing meaningful to invert — that state is propagated unchanged instead of being flipped.
+
 ### Example:
 
 ```bash
@@ -316,13 +323,27 @@ This condition type checks the size of arrays or collections in the conversation
 (...)
 ```
 
+The example above matches whenever the API call stored between 1 and 10 result elements. Collections, maps and arrays report their **element count** — earlier EDDI versions ran the resolved value through `Integer.parseInt`, so a real collection silently degraded to size `0` and a rule like this one could never match.
+
 | Config      | Type   | Description                              |
 | ----------- | ------ | ---------------------------------------- |
 | `valuePath` | string | Path to the array/collection to check    |
 | `min`       | int    | Minimum size required (-1 to skip check) |
 | `max`       | int    | Maximum size allowed (-1 to skip check)  |
 | `equal`     | int    | Exact size required (-1 to skip check)   |
 
+**How the size is determined** — the value the `valuePath` resolves to decides the rule:
+
+| Resolved value            | Size used                                                              |
+| ------------------------- | ---------------------------------------------------------------------- |
+| `null` / path not found   | `0`                                                                    |
+| Collection, Map, or array | Its element count                                                      |
+| Number                    | The number itself (the value _is_ the size)                            |
+| Numeric string            | The parsed number (kept for backwards compatibility)                   |
+| Any other value           | The length of its textual representation                               |
+
+If `min`, `max` and `equal` are all `-1`, the condition reports `NOT_EXECUTED` — it neither succeeds nor fails, and a wrapping `negation` propagates that state instead of inverting it.
+
 ## The Behavior Rule API Endpoints
 
 The API Endpoints below will allow you to manage the `Behavior Rule`s in your EDDI instance.

docs/capability-match-guide.md

@@ -86,14 +86,14 @@
 
 ## Template Variables
 
-Config values support **Jinja2 template expressions**, resolved against the conversation memory at evaluation time. This enables dynamic routing:
+Config values support **Qute template expressions**, resolved against the conversation memory at evaluation time. This enables dynamic routing:
 
 ```json
 {
   "type": "capabilityMatch",
   "configs": {
-    "skill": "{{properties.requiredSkill.valueString}}",
-    "strategy": "{{context.routingStrategy}}",
+    "skill": "{properties.requiredSkill}",
+    "strategy": "{context.routingStrategy}",
     "minResults": "1"
   }
 }
@@ -195,7 +195,7 @@ Use the discovered agents to dynamically compose a group conversation:
 **System prompt (LLM task triggered by `create_expert_group`):**
 ```
 The following agents have been identified as legal analysis experts:
-{{memory.current.capabilityMatch.results}}
+{memory.current.capabilityMatch.results}
 
 Use the createGroupConversation tool to assemble them into a discussion panel.
 ```
@@ -212,7 +212,7 @@ Use PropertySetter to capture the user's intent, then route dynamically:
     "actions": ["user_request_specialist"],
     "setProperties": [{
       "name": "requiredSkill",
-      "valueString": "{{memory.current.intent}}",
+      "valueString": "{memory.current.intent}",
       "scope": "conversation"
     }]
   }]
@@ -228,7 +228,7 @@ Use PropertySetter to capture the user's intent, then route dynamically:
     {
       "type": "capabilityMatch",
       "configs": {
-        "skill": "{{properties.requiredSkill.valueString}}",
+        "skill": "{properties.requiredSkill}",
         "strategy": "highest_confidence",
         "minResults": "1"
       }

docs/changelog.md

@@ -3,6 +3,48 @@
 > **Purpose:** Living document tracking all changes, decisions, and reasoning during implementation. Updated as work progresses for easy reference and review.
 
 
+---
+
+## 🔐 fix(security): code-review findings wave 2a — access control, A2A ownership, GDPR & audit ledger (2026-07-28)
+
+**Repo:** EDDI (`fix/code-review-access-control`)
+
+Second wave of the 124-finding external code review, split into two PRs so each stays under CodeRabbit's 100-file review limit (wave 1 at 151 files was skipped by it entirely). This half is the access-control and compliance surface.
+
+### The guard triad existed — it just wasn't called everywhere
+
+EDDI already has `OwnershipValidator`, `ConversationAccessGuard` and `HitlAccessGuard`, used correctly in `engine/internal`, `engine/hitl` and the MCP surface. Every finding here is a place that never called them.
+
+- **A1 (critical)** — The **SSE turn endpoint had no ownership check** while its non-streaming twin did. The turn then executed under the *target* conversation's `userId`, loading that user's long-term memories into the prompt and running tool calls in their context. Fixed in two layers: the guard moved **down into `ConversationService.say`/`sayStreaming`** so no future REST adapter can omit it, *before* the memory snapshot loads — plus a REST-layer check so denial is a plain 403 rather than an error event on an already-200 SSE stream.
+- **A2 (critical)** — Attachment endpoints **authorised the path parameter, not the caller**. `IAttachmentStore.load(ref, requestingConversationId)` checks that the *named* conversation owns the blob — and the caller supplies that name, so the check was self-satisfying. All five methods now require caller ownership, checked on the request thread *before* the async hop (`SecurityIdentity` is request-scoped).
+- **A3** — `?deleteOlderThanDays=0` permanently deleted **every ended conversation in the deployment**, from an endpoint with no role at all. Now admin-only with a minimum of 1 day.
+- **A4** — The tool control plane carried no `@RolesAllowed` at all: rate-limiter reset, cost-budget reset, and a history endpoint dumping **raw tool arguments and results** for any conversation.
+- **A5** — `/propertiesstore/properties/{userId}` had neither role nor ownership check over the same `IUserMemoryStore` that `RestUserMemoryStore` guards on all nine of its methods. Writes there land in the victim's next system prompt.
+- **A6** — A **conversation-id oracle**: it returned another user's live `conversationId`, which is the discovery half of A1 and A3.
+- **A7** — Template preview read **any** conversation's memory and returned a flattened dump of properties/context/memory — and since the caller supplies the template, it was effectively a query language over someone else's conversation.
+- **A8** — Config stores with full CRUD and no role. The review named one; **the actual inventory was six**, including two it missed (`IRestCapabilityRegistry`, `IRestWorkflowStepStore`). `IRestVersionInfo` was deliberately left alone — it has no `@Path` and is a mixin, reasoning recorded in the code.
+- **A9** — A2A sat entirely outside the ownership model: caches keyed on a caller-supplied id, `contextId` as an unauthenticated read+write conversation handle, conversations created with `userId = null` (which `OwnershipValidator` treats as "legacy — allow", so permanently unowned), and raw `e.getMessage()` returned to arbitrary peers.
+- **A12** — The global exception mapper returned the **raw driver message** as the 500 body — collection names, hostnames, replica-set topology. Now logged at ERROR with a correlation id.
+
+### GDPR & audit (G14–G20)
+
+- **G14** — Erasure was **served stale indefinitely** from a Caffeine cache with no TTL that erasure never invalidated.
+- **G15** — The erasure cascade missed three stores while `docs/gdpr-compliance.md` asserted it "covers all data stores": conversation **checkpoints** (which carry `propertiesCopy` including PII, behind a javadoc claiming it was "used during GDPR erasure" when its only caller was unreachable), **group transcripts**, and **schedules** that kept firing under an erased user's id.
+- **G16** — `AuditHmac.verifyHmac` had **zero production callers**; the docs told operators to "recompute the HMAC and compare it" and the product shipped no way to do so. Added admin verification endpoints.
+- **G17** — GDPR pseudonymisation did `updateMany($set userId)` with no HMAC recompute, and `userId` is a *signed* field — so **every routine erasure produced rows cryptographically indistinguishable from tampered ones**. The class javadoc claiming a write-once contract was literally true and substantively false.
+- **G18** — No hash chain: entries were independently signed, so **deletion and reordering were undetectable**. Added a per-conversation sequence inside the signed payload (chosen over a global chain, which would serialise all audit writes).
+  - **Follow-up (review):** the first cut shipped this on **MongoDB only**. `PostgresAuditStore` persisted no sequence and left `supportsSequence()` at its `false` default, so `AuditLedgerService` skipped assigning one entirely — **every PostgreSQL deployment silently had no deletion detection**, reported as `UNAVAILABLE`, which reads like "not applicable" rather than "unprotected". Added the column, an idempotent `ALTER TABLE` defaulting old rows to the `UNSEQUENCED` sentinel, the `(conversation_id, sequence)` index verification reads, and `supportsSequence() = true`. This is the second cross-backend gap in this PR (after schedule `userId`), both in compliance code, both invisible because the degraded answer looked benign — the case for the deferred D4/J3 conformance suite.
+- **G19** — `eddi.vault.master-key` ships empty, so entries were written **unsigned by default** while the docs present the ledger as evidence-grade. `ComplianceStartupChecks` had zero references to vault/HMAC/audit.
+- **G20** — Unbounded audit queue that **re-offered failed batches into itself** — an OOM loop under a slow store.
+
+### Documentation corrected (I7–I13)
+
+Several docs described behaviour that did not exist. `docs/semantic-parser.md` documented a **stemming extension that does not exist**, four times, including inside the flagship copy-paste config — copying it throws `UnrecognizedExtensionException` and the agent will not start. The same doc's expression table claimed `number(42)` and `time(15:00)` where the code emits `integer(42)` and epoch millis, so rules written from it never fired. `docs/conversation-memory.md` and `docs/properties.md` both contradicted AGENTS.md §5.1 on the template model (`{properties.X.valueString}` fails at runtime — `MemoryItemConverter` puts raw values).
+
+### Verification
+
+Full unit suite: 12,384 tests, **0 non-environmental failures** (308 listed failures/errors all carry a loopback/selector/event-loop signature — this machine cannot bind sockets; CI is the gate for those). Five high-stakes fixes were **mutation-checked** — revert the fix, confirm a test actually fails, restore: G2, G12, A1, A2 and F18 all bite, verified against whole test classes (a `-Dtest=Class#method` filter silently runs 0 tests and exits 0 when the method is in a `@Nested` class, which reads exactly like a pass).
+
 ---
 
 ## 🐞 fix(engine): code-review findings wave 1 — parser, rules, templating, apicalls, datastore, LLM infra, deployment (2026-07-28)