feat(vault): enforce allowedAgents at deploy time instead of documenting it - #662
Merged
Conversation
…ing it SecretMetadata.allowedAgents was 'for visibility only'. That model assumes a human admin authors agent configs; create_sub_agent lets an LLM author one, so scoping a secret to one agent bought nothing. Enforced at deployment, not resolution, and that is the load-bearing decision: SecretResolver sees only a string and several call sites have no agent at all, while ChatModelRegistry caches models keyed on the UNRESOLVED parameters — so a check behind that cache runs for whichever agent built the model first and is skipped for every other one. That would be enforcement that looks real and is not. The binding lives in the agent's configuration, so it is checked there. deployIfGranted is the single deployment entry point; guarding checkDeployments alone left the 24h sweep to deploy what enforce had blocked. eddi.vault.grant-enforcement = off|warn|enforce, parsed strictly so a typo cannot silently disable it, defaulting to warn because the field has never been enforced and non-wildcard values in existing deployments are untested config. +15 tests.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
|
Warning Review limit reached
Next review available in: 30 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Aug 10, 2026
Closed
There was a problem hiding this comment.
Pull request overview
Adds deploy-time enforcement of vault secret allowedAgents grants with configurable rollout modes.
Changes:
- Adds vault-reference scanning across selected workflow extensions.
- Gates scheduled agent deployments using
off,warn, orenforce. - Adds grant-checking and configuration-parsing tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
VaultGrantChecker.java |
Scans configurations and validates secret grants. |
AgentDeploymentManagement.java |
Applies grant checks to scheduled deployments. |
VaultGrantCheckerTest.java |
Tests grant and extension scanning behavior. |
GrantEnforcementModeTest.java |
Tests strict enforcement-mode parsing. |
docs/changelog.md |
Documents the feature and rollout policy. |
Suppressed comments (1)
src/main/java/ai/labs/eddi/engine/runtime/internal/AgentDeploymentManagement.java:582
- This new security gate reports only through logs. The repository's observability guideline requires Micrometer metrics for new features; add counters for warned, blocked, and skipped checks so operators can validate rollout and alert on enforcement without log parsing.
if (grantEnforcement == GrantEnforcement.ENFORCE) {
LOGGER.error(message + " Deployment BLOCKED (eddi.vault.grant-enforcement=enforce).");
return false;
}
LOGGER.warn(message + " Deployment allowed (eddi.vault.grant-enforcement=warn); set it to 'enforce' to block.");
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+509
to
+513
| private boolean deployIfGranted(Environment environment, String agentId, Integer agentVersion) | ||
| throws ServiceException, IllegalAccessException { | ||
|
|
||
| if (!vaultGrantsSatisfied(agentId, agentVersion)) { | ||
| return false; |
Comment on lines
+143
to
+147
| private Set<String> collectVaultReferences(AgentConfiguration agentConfiguration) { | ||
| Set<String> references = new LinkedHashSet<>(); | ||
| if (agentConfiguration.getWorkflows() == null) { | ||
| return references; | ||
| } |
Comment on lines
+188
to
+193
| if (stepType.contains("ai.labs.httpcalls") || stepType.contains("ai.labs.apicalls")) { | ||
| return apiCallsStore.read(id.getId(), id.getVersion()); | ||
| } | ||
| if (stepType.contains("ai.labs.mcpcalls")) { | ||
| return mcpCallsStore.read(id.getId(), id.getVersion()); | ||
| } |
Comment on lines
+200
to
+204
| private static void scanForVaultReferences(Object config, Set<String> sink) { | ||
| String serialized; | ||
| try { | ||
| serialized = MAPPER.writeValueAsString(config); | ||
| } catch (Exception e) { |
|
|
||
| **Uncertainty never becomes a violation:** unreadable metadata, a disabled vault, an unreadable workflow, and absent/empty/wildcard grants all allow. Every wizard-vaulted key carries `["*"]`, so stock deployments see no change. **Not a revocation mechanism** — an agent deployed before its grant was narrowed keeps resolving until redeployed. | ||
|
|
||
| +15 tests, covering all three extension types. 109 green. |
…figs Copilot on #662, four findings, all valid: - The gate was NOT the deployment boundary. RestAgentAdministration and ConversationService call agentFactory.deployAgent directly, and create_sub_agent reaches the former through AgentSetupService — so an LLM-authored agent went live in enforce mode without ever being checked, which is the exact threat this PR exists to address. The gate now lives in AgentFactory.deployAgent, the one place all three funnel through, extracted into VaultGrantGate so the mode lives in one place rather than at each caller. - The agent document itself was never scanned, though UserMemoryConfig.DreamConfig.parameters explicitly supports vault references and DreamService hands them to ChatModelRegistry. - RAG extensions were omitted, though RagConfiguration carries embedding-model and vector-store credentials that SecretResolver resolves at runtime. - The changelog test count was wrong. +21 tests. The mode parse now fails bean construction, so an unusable value still fails startup.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…check logs CodeQL log-injection alerts 492-495. agentId reaches these paths from REST path params and from create_sub_agent, and the vault reference strings come from agent configuration an LLM can author — so both are user-influenced and must go through LogSanitizer before reaching a log line.
aisabella-ai
approved these changes
Aug 11, 2026
This was referenced Aug 11, 2026
pull Bot
pushed a commit
to Stars1233/EDDI
that referenced
this pull request
Aug 11, 2026
…doc that denies it allowedAgents became enforced (labsai#662) and enforcement became the default (labsai#664), but docs/secrets-vault.md never mentioned eddi.vault.grant-enforcement at all. The only description of the feature lived in the changelog, which operators do not read. Adds an "Agent Grants" section: the three modes, the two parsing rules (an unknown value fails startup, absent/blank resolves to enforce), what counts as granted, which configurations are scanned, and the upgrade step for the one deployment shape that is actually affected -- a master key plus a deliberately narrowed grant, where the first symptom is an agent refusing to deploy with no warning phase. The javadoc was worse than missing. Four places still told the reader the field is not enforced: SecretMetadata (flatly wrong), VaultSecretProvider and EncryptedSecret (true of those classes, but reading as "not enforced anywhere"), and the AgentSetupService comment arguing for "*" on the grounds that the enforcement does not exist. AgentSetupService still writes ["*"], which 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 209) and only ever receives agentName, so narrowing there would mean guessing an unassigned ID and blocking the agent the key was vaulted for. Documentation only -- no executable line changed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebuilt onto current
main.SecretMetadata.allowedAgentsis documented as "for visibility only — enforcement is via configuration authorship, not runtime resolution". That access model assumes a human admin authors agent configurations.create_sub_agentlets an LLM author one — so an operator who scoped a secret to a single agent got no enforcement at all.Why deployment time, and not resolution time
This is the load-bearing decision.
SecretResolvercannot do it soundly. It sees only a string — no agent identity — and its ~12 call sites spanApiCallExecutor,ChatModelRegistry,A2AToolProviderManager,McpToolProviderManager,EmbeddingModelFactory,EmbeddingStoreFactoryandChannelTargetRouter. Several legitimately run outside any conversation;AgentSigningServicebypasses the resolver entirely.The model cache would launder the check.
ChatModelRegistrycaches onModelCacheKey(type, unresolvedParams). Two agents sharing a config share a cache entry, so a check insideresolveSecretsruns for whichever agent builds the model first and is silently skipped for every other one — enforcement that looks real and is not.The agent↔secret binding is established in the agent's configuration, so that is where it can be checked completely, once, and with no cache in the way — beside the existing deploy-time
lintInertHitlConfig.What it does
VaultGrantCheckerwalks the agent's workflows →llm/apicalls/mcpcallsconfigs, serializes each and scans for${vault:...}references, then verifies each againstallowedAgents.The scan serializes rather than enumerating known credential fields — enumeration is how this kind of check rots when someone adds a new credential field. Pinned by a test planting a reference in a field named
someFutureCredentialField, plus cases for all three extension types.deployIfGrantedis the single deployment entry point. GuardingcheckDeploymentsalone leftmanageAgentDeployments' latest-version redeploy andmanageDeploymentOfOldAgentcalling the factory directly, soenforcewould have been bypassed a day later by the 24h sweep.Rollout
eddi.vault.grant-enforcement=off|warn(default) |enforce, parsed strictly:enforcedsilently meaningwarnwould turn one typo into a security control that is off while appearing on, so an unusable value fails startup with the valid values named.Warn is the default because the field has never been enforced — any non-wildcard value in an existing deployment is untested configuration, and blocking on it during an upgrade could take agents down for a policy nobody has yet verified.
Uncertainty never becomes a violation
Unreadable metadata, a disabled vault, an unreadable workflow, and absent / empty / wildcard grants all allow. Every
AgentSetupService-vaulted key carries["*"], so existing deployments see no change.Not a revocation mechanism: an agent deployed before its grant was narrowed keeps resolving until redeployed. Stated in the class Javadoc rather than left to be discovered.
Testing
+15 tests; 109 green across the vault and deployment suites.