Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,34 @@
> **Purpose:** Living document tracking all changes, decisions, and reasoning during implementation. Updated as work progresses for easy reference and review.


---

## 📖 docs(vault): document allowedAgents enforcement, and correct the javadoc that denies it (2026-08-11)

**Repo:** EDDI (`docs/vault-grant-enforcement`)

`allowedAgents` became enforced (#662) and enforcement became the default (#664), but `docs/secrets-vault.md` never mentioned `eddi.vault.grant-enforcement` at all — the only description of the feature lived in this changelog, which operators do not read. New **Agent Grants** section covering the three modes, the two parsing rules (unknown value fails startup, absent/blank resolves to `enforce`), what counts as granted, which configurations are scanned, and the upgrade step. The existing "Additional vault settings" properties block listed `cache-ttl-minutes` and `cache-max-size` but not `grant-enforcement`, so it now lists all three — an operator scanning that block for the available knobs would not have found the new one.

**The javadoc was worse than missing — it was wrong.** Four places still told the reader the field is not enforced:

| File | Said | Reality |
| ---- | ---- | ------- |
| `SecretMetadata` | "visibility only — enforcement is via configuration authorship, not runtime resolution" | Flatly wrong since #662 |
| `VaultSecretProvider` | "stored for visibility/documentation but NOT enforced at resolution time" | True of *that class*, reads as "not enforced anywhere" |
| `EncryptedSecret` | "for visibility/documentation only" (twice) | Same |
| `AgentSetupService` | "Narrowing this list would imply an enforcement that does not exist" | The enforcement now exists |
| `ISecretProvider`, `SecretReference`, `IRestSecretStore`, `SecretResolver` | "access control is via configuration authorship" | Found by grepping the phrase rather than fixing one file per review comment |

`VaultGrantChecker` and its test quote the old wording deliberately — "was documented as…" — and keep it; that is history, not a stale claim.

A review comment (CodeRabbit, #667) also caught that "a violation stops the agent coming up" is only true under `enforce`; `warn` logs and allows, `off` does not check. Every place asserting the blocking behavior now names `eddi.vault.grant-enforcement` as what decides it, including the doc's own lead paragraph.

This is the same text that, earlier in this review, caused a proposed narrowing of `allowedAgents` to be reverted as security theater — the documentation was accurate then and became false when the behavior changed under it. Left alone it now misleads in the opposite direction.

**`AgentSetupService` still writes `["*"]`, and that is still correct** — but for a different reason than the old comment gave. The wizard vaults the key *before* the agent exists (`vaultApiKey` at line 165; `agentId` extracted at line 209), and the method only ever receives `agentName`. Narrowing at that call site would mean guessing an ID that has not been assigned, and guessing wrong blocks the very agent the key was vaulted for. The comment now says that instead of citing an enforcement gap that has since closed.

Documentation only — no behavior change, no new tests.

---

## 🔒 chore(vault): default grant-enforcement to enforce (2026-08-11)
Expand Down
58 changes: 58 additions & 0 deletions docs/secrets-vault.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,60 @@ Vault references are resolved **at runtime** when the task executes, never store

**Caching:** Successfully resolved secrets are cached in a Caffeine cache (configurable TTL). Failed resolutions are **never cached**, ensuring newly created secrets resolve immediately without waiting for cache expiry.

## Agent Grants (`allowedAgents`)

Every stored secret carries an `allowedAgents` list — the agent IDs permitted to use it, or `["*"]` for all agents. It is checked **when an agent is deployed**, not when a secret is resolved. What a violation costs is set by [the enforcement mode](#modes) — blocked, logged, or not checked at all.

### Why deploy time, not resolution time

Blocking at resolution would fail in the middle of a live conversation, after the agent is already serving users, and the operator would learn about the misconfiguration from a broken turn. The deploy-time check runs once, before any user is affected: in `enforce` mode a misconfigured agent simply does not come up, and the reason is a single ERROR line.

The gate lives in `AgentFactory.deployAgent` — the one boundary every deployment path funnels through (REST administration, conversation-triggered deployment, the scheduled deployment poller). Placing it there rather than at each caller means it cannot be bypassed by reaching deployment through a different entry point.

### Modes

Configured with `eddi.vault.grant-enforcement`:

| Mode | Behavior |
| --------- | ------------------------------------------------------------------- |
| `off` | No check at all — the checker is never consulted |
| `warn` | Violations logged at WARN, deployment proceeds |
| `enforce` | **Default.** Violations logged at ERROR, deployment is **blocked** |

Two parsing rules, both deliberate:

- **An unrecognized value fails startup** rather than falling back to a default. `grant-enforcement=enforced` silently behaving as `warn` would turn one typo into a security control that is off while appearing on.
- **Absent or blank resolves to `enforce`**, the shipped default — never to something weaker. Turning enforcement down is always explicit.

### What counts as granted

The check answers "is this provably ungranted?", and anything short of proof is treated as granted. An agent is allowed when its `allowedAgents` list:

- contains the agent's ID, or
- contains the `*` wildcard, or
- is `null` or empty — an unset list means unrestricted, not "deny all"

Uncertainty likewise never becomes a violation: unreadable metadata, a disabled vault, or a secret that does not exist all resolve to *allowed*. A check that cannot run must not be able to take agents down.

### What is scanned

The agent document itself plus each workflow's LLM, HTTP-call, MCP-call, and RAG configurations are serialized and scanned for `${vault:...}` references. Each reference found is resolved to its secret metadata and tested against the deploying agent's ID.

### Upgrading to enforcement

On most deployments this control is inert, for two reasons worth confirming rather than assuming:

1. **No master key, no check.** With `eddi.vault.master-key` unset the vault is disabled and the checker reports no violations without looking at anything.
2. **Auto-vaulted keys are unrestricted.** Every key the setup wizard vaults is stored with `allowedAgents = ["*"]`.

The deployments that *are* affected have **both** a master key and a grant an operator has deliberately narrowed. There, enforcement is a behavior change with no warning phase — the first symptom is an agent refusing to deploy. Before enabling it, run once with `warn` and confirm the log is free of:

```text
references vault secret(s) it is not granted
```

Then set `enforce`. To widen a grant instead, add the agent ID to the secret's `allowedAgents` via the [REST API](#rest-api) — or remove the reference from the agent's configuration.

## Encryption

### Envelope Encryption
Expand Down Expand Up @@ -134,6 +188,10 @@ Additional vault settings in `application.properties`:
# Cache for resolved secrets (avoids repeated decryption)
eddi.vault.cache-ttl-minutes=5
eddi.vault.cache-max-size=1000

# Whether an agent referencing a secret it is not granted may deploy:
# off | warn | enforce (default). See "Agent Grants" above.
eddi.vault.grant-enforcement=enforce
```

> **⚠️ Important:** The vault master key encrypts all stored API keys. If the master key is lost, all encrypted secrets become **permanently unrecoverable**. Back up your `~/.eddi/.env` file.
Expand Down
18 changes: 8 additions & 10 deletions src/main/java/ai/labs/eddi/engine/setup/AgentSetupService.java
Original file line number Diff line number Diff line change
Expand Up @@ -913,17 +913,15 @@ private String vaultApiKey(String apiKey, String agentName, Map<String, Object>
createdResources.put(VAULTED_SECRET_KEY, keyName);
}
var ref = new SecretReference(SecretReference.DEFAULT_TENANT, keyName);
// "*" is deliberate and is NOT an access-control decision made here:
// VaultSecretProvider documents that allowedAgents is "stored for
// visibility/documentation but NOT enforced at resolution time". The
// vault's access model is "the admin who writes the agent config decides
// which references to include". Narrowing this list would imply an
// enforcement that does not exist.
// "*" is deliberate. allowedAgents IS enforced now (VaultGrantGate, at
// deploy time), so this list is a real access-control decision — but the
// agent being set up does not have an ID yet at this point, and the key is
// created for whichever agent this setup produces. Narrowing it here would
// have to guess that ID, and guessing wrong blocks the very agent the key
// was vaulted for.
//
// Worth flagging rather than silently narrowing: that access model assumes
// a human admin authors the config, and create_sub_agent lets an LLM author
// one. Making allowedAgents enforceable is a vault-level feature, not a
// one-line change at this call site.
// Operators who want a narrow grant set it after setup, via the secrets
// REST API; see docs/secrets-vault.md "Agent Grants".
secretProvider.store(ref, apiKey, "Auto-vaulted by AgentSetupService for agent: " + agentName,
List.of("*"));
LOGGER.infof("API key vaulted for agent '%s' (key: %s)", agentName, keyName);
Expand Down
9 changes: 5 additions & 4 deletions src/main/java/ai/labs/eddi/secrets/ISecretProvider.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@
* Service Provider Interface for secrets management. Implementations handle the
* actual storage and retrieval of encrypted secrets.
* <p>
* Secrets are scoped at the <b>tenant level</b>, identified by
* {@code (tenantId, keyName)}. There is no agent-level scoping — access control
* is via <b>configuration authorship</b>: the admin who writes the agent config
* decides which vault references ({@code ${vault:keyName}}) to include.
* Secrets are stored at the <b>tenant level</b>, identified by
* {@code (tenantId, keyName)}. Which agents may use a secret is governed by
* {@code SecretMetadata.allowedAgents}, checked when an agent is deployed (see
* {@link VaultGrantGate}) — not by this interface, whose implementations
* resolve any valid reference they are asked for.
* <p>
* All implementations MUST ensure:
* <ul>
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/ai/labs/eddi/secrets/SecretResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@
* workflow. It is called <b>after</b> Qute template processing and
* <b>before</b> the final API call (late-binding resolution).
* <p>
* <b>Access model:</b> Access control is via configuration authorship — the
* admin who writes the agent config decides which vault references to include.
* The resolver does NOT check agent permissions; it resolves any valid
* reference that exists in the vault.
* <b>Access model:</b> the resolver does NOT check agent permissions; it
* resolves any valid reference that exists in the vault. Which agents may use a
* secret is governed by {@code SecretMetadata.allowedAgents} and checked when
* the agent is deployed, by {@link VaultGrantGate}.
* <p>
* Includes a Caffeine cache with configurable TTL to avoid repeated
* decryption/vault calls. Cache is invalidated on secret rotation via
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,19 @@
* master key</li>
* <li>Both secrets and DEKs are persisted via {@link ISecretPersistence}
* (MongoDB or PostgreSQL)</li>
* <li>Secrets are scoped at the <b>tenant level</b> — access control is via
* configuration authorship</li>
* <li>Secrets are stored at the <b>tenant level</b>; which agents may use one
* is governed by {@code allowedAgents} (see below)</li>
* </ul>
* <p>
* <b>Key rotation:</b> Supports both DEK rotation (per-tenant, re-encrypts all
* secrets) and KEK rotation (re-encrypts all DEKs with a new master key).
* <p>
* <b>Access model:</b> The admin who writes the agent config decides which
* vault references to include. The {@code allowedAgents} field is stored for
* visibility/documentation but NOT enforced at resolution time.
* <b>Access model:</b> {@code allowedAgents} is not consulted here — this class
* resolves secrets and does not police who asked. It is checked one level up,
* when an agent is deployed, by {@link ai.labs.eddi.secrets.VaultGrantGate},
* which blocks or merely logs according to
* {@code eddi.vault.grant-enforcement}. "Not enforced at resolution time" is a
* statement about this class, not about the field.
* <p>
* The KEK (Master Key) is supplied via the {@code EDDI_VAULT_MASTER_KEY}
* environment variable. If not set, the provider is disabled and all operations
Expand Down
12 changes: 7 additions & 5 deletions src/main/java/ai/labs/eddi/secrets/model/EncryptedSecret.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
* Database entity for an encrypted secret stored via envelope encryption. The
* actual secret value is encrypted with the tenant's DEK (Data Encryption Key).
* <p>
* Secrets are scoped at the <b>tenant level</b> — identified by
* {@code (tenantId, keyName)}. The {@code allowedAgents} field is for
* visibility/documentation only (not enforced at resolution time).
* Secrets are stored at the <b>tenant level</b> — identified by
* {@code (tenantId, keyName)}. The {@code allowedAgents} field restricts which
* agents may use the secret, checked at deployment time by
* {@link ai.labs.eddi.secrets.VaultGrantGate}; whether a violation blocks the
* deployment depends on {@code eddi.vault.grant-enforcement}.
*
* @author ginccc
* @since 6.0.0
Expand All @@ -35,8 +37,8 @@ public class EncryptedSecret {
/** Human-readable description of what this secret is for */
private String description;
/**
* Agent IDs allowed to use this secret, or ["*"] for all agents. For
* visibility/documentation only — not enforced at resolution time.
* Agent IDs allowed to use this secret, or ["*"] for all agents. Null or empty
* means unrestricted, not "deny all". Enforced at deployment time.
*/
private List<String> allowedAgents;
private Instant createdAt;
Expand Down
19 changes: 12 additions & 7 deletions src/main/java/ai/labs/eddi/secrets/model/SecretMetadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,16 @@
* Non-sensitive metadata about a stored secret. Plaintext values are NEVER
* exposed through this record.
* <p>
* Secrets are scoped at the <b>tenant level</b>, not per-agent. Access control
* is via configuration authorship — the admin who writes the agent config
* decides which vault references to include. The {@code allowedAgents} field is
* for <b>visibility and documentation only</b>, helping admins track which
* agents use which secrets.
* Secrets are stored at the <b>tenant level</b>. Which agents may use one is
* governed by {@code allowedAgents}, checked when an agent is deployed (see
* {@link ai.labs.eddi.secrets.VaultGrantGate}) rather than when a secret is
* resolved, so a misconfiguration surfaces before the agent serves traffic
* rather than in the middle of a live conversation.
* <p>
* What a violation costs depends on {@code eddi.vault.grant-enforcement}: under
* {@code enforce} (the default) the deployment is blocked, under {@code warn}
* it is logged and allowed, and under {@code off} the check does not run at
* all.
*
* @param tenantId
* the owning tenant
Expand All @@ -37,8 +42,8 @@
* "OpenAI API key for production")
* @param allowedAgents
* list of agent IDs allowed to use this secret, or {@code ["*"]} for
* all agents. This is for <b>visibility only</b> — enforcement is
* via configuration authorship, not runtime resolution.
* all agents. {@code null} or empty means unrestricted, not "deny
* all". Enforced at deployment time, not at resolution time.
*
* @author ginccc
* @since 6.0.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
* means {@code docker.io/library/nginx}. Single-tenant deployments (the common
* case) use the cleaner short form.
* <p>
* <b>Access model:</b> Access control is via configuration authorship — the
* admin who writes the agent config decides which vault references to include.
* See {@link SecretMetadata#allowedAgents()} for visibility/documentation.
* <b>Access model:</b> which agents may use a secret is governed by
* {@link SecretMetadata#allowedAgents()}, checked when an agent is deployed
* rather than when a reference is resolved.
*
* @param tenantId
* the tenant namespace (default: "default" for single-tenant)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
* REST interface for managing secrets in the vault. Secrets are stored
* encrypted; plaintext values are NEVER returned by any endpoint.
* <p>
* Secrets are scoped at the <b>tenant level</b> — identified by
* {@code (tenantId, keyName)}. Access control is via configuration authorship
* (the admin writes vault references into agent configs).
* Secrets are stored at the <b>tenant level</b> — identified by
* {@code (tenantId, keyName)}. Which agents may use a secret is governed by its
* {@code allowedAgents} list, checked when an agent is deployed; these
* endpoints are where an operator widens or narrows that grant.
*
* @author ginccc
* @since 6.0.0
Expand Down
Loading