feat(vault): gate the secret store on a deployer input#183
Open
ItamarZand88 wants to merge 12 commits into
Open
feat(vault): gate the secret store on a deployer input#183ItamarZand88 wants to merge 12 commits into
ItamarZand88 wants to merge 12 commits into
Conversation
Extends enablement to Vault across AWS, GCP and Azure Terraform plus CloudFormation. On AWS and GCP the vault owns no block at all — those stores are name prefixes rather than resources — so its IAM grants are the only thing it can gate, and they had been left standing. That is not cosmetic. The grant is a data-plane read (ssm:GetParameter, roles/secretmanager.secretAccessor) over a name-prefix wildcard, and resource ids allow interior hyphens, so vaults `app` and `app-config` coexist happily. Declining `app` while leaving `<prefix>-app-*` granted leaves the runtime role able to read `app-config`'s secrets. Saying no has to withdraw the permission, not just the registration entry. On GCP the fix needed a second emitter. The service account's profile loop emits a byte-identical copy of the same grant, which the generator had been collapsing by body-identity dedup. Gating the vault alone made the two bodies differ, dedup stopped firing, and two ungated copies survived — holding exactly the access the deployer had declined. So the service account emitter is gated too, via gate_bindings. The shared project-wide custom roles are deliberately excluded: they already carry their own var.gcp_manage_custom_roles count, a second count is a hard conflict, and tying them to one vault's gate would delete roles that vault's siblings still hold. Azure is the one cloud where the vault is a real resource, and every role assignment names it twice — once as `scope`, once inside the uuidv5 seed that derives the assignment's name. The seed is a raw interpolation, so it is the reference most likely to be left behind. The tests assert the property rather than the mechanism. Checking that the vault's own policy carries the gate is the assertion that missed the duplicate grant: it passes straight over an ungated twin. So they resolve the template with the gate off and ask what access is still standing.
…mespace Vault and postgres data grants are name-prefix scoped, so a grant on 'app' covers every secret under 'app-*', including a sibling 'app-config'. Declining 'app-config' withdraws its own grants but not the sibling's, leaving the declined namespace readable and writable. Refuse the gate unless the sibling follows the same input, or the ids are renamed apart.
The fixture gated 'secrets', the reserved deployment secrets vault the preflight refuses. Rename it to the id the grant-scoped tests already use.
…space A '*'-scoped vault or postgres data grant binds by secret-name prefix over the whole stack, whichever type wrote the secret, so on GCP a postgres grant keeps reading a declined vault's namespace. The wildcard rule now flags either family member's grant when the gated resource is in the family.
strip_declined_resources removed a deployer-declined gated resource from the desired stack but left its resource-scoped entry in every permission profile. On GCP the service-account controller derives project-level Vertex bindings straight from the profile (IAM can't scope to a sub-resource), so a declined resource's grant was still applied at deploy time — a runtime fail-open of the .enabled() gate. Prune the entry from every profile so no runtime consumer re-grants it; the "*" wildcard, which is not resource-scoped, stays.
The strip test only proved the "*" wildcard survived; add a kept sibling resource with its own resource-scoped grant and assert it stays, so the test proves the removal is exact-key and never sweeps a sibling. Also trim the vault supports_enabled_when docstrings and the queue/vault test docs to lead with the prefix-wildcard footgun instead of restating that the vault owns no block, and point the strip comment at the preflight that makes exact-key removal safe.
Greptile SummaryAdds deployer-input gating for Vault resources and their IAM grants across CloudFormation and Terraform.
Confidence Score: 2/5This PR should not merge until management-profile grants are stripped with declined resources and the new namespace checks are scoped to platforms where the overlap exists. Initial setup can preserve a declined resource's management grant, while the platform-independent preflight logic also blocks valid AWS and Azure stack configurations. crates/alien-deployment/src/pending.rs; crates/alien-preflights/src/compile_time/resource_enabled_valid.rs
|
| Filename | Overview |
|---|---|
| crates/alien-deployment/src/pending.rs | Removes declined-resource grants from named profiles but omits the separately stored management profile. |
| crates/alien-preflights/src/compile_time/resource_enabled_valid.rs | Adds useful namespace protections, but applies GCP-specific vault/postgres overlap rules to AWS and Azure. |
| crates/alien-terraform/src/emitters/aws/vault.rs | Gates emitted AWS vault IAM policy blocks while preserving prefix-based import and binding data. |
| crates/alien-terraform/src/emitters/gcp/vault.rs | Gates GCP vault IAM bindings without incorrectly gating shared custom roles. |
| crates/alien-terraform/src/emitters/azure/vault.rs | Counts Azure Key Vault and role-assignment blocks and indexes reachable resource references. |
| crates/alien-cloudformation/src/emitters/aws/vault.rs | Opts the prefix-only AWS vault emitter into generator-applied CloudFormation conditions. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Input[Deployer gate input] --> Render[CFN / Terraform gating]
Input --> Strip[Strip declined resource]
Render --> Cloud[Cloud resource and IAM grants]
Strip --> Named[Named permission profiles]
Strip -. omitted .-> Mgmt[Management permission profile]
Mgmt --> Runtime[Runtime permission consumers]
Runtime --> IAM[Management IAM grant]
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
crates/alien-deployment/src/pending.rs:169-171
**Management grants survive decline**
When an initial setup declines a gated resource with a resource-scoped management grant, this loop cleans only named profiles and leaves the separate management profile unchanged, causing downstream emitters and permission controllers to retain management access to the declined resource's namespace.
### Issue 2 of 2
crates/alien-preflights/src/compile_time/resource_enabled_valid.rs:112-124
**Namespace family ignores platform**
When an AWS or Azure stack combines a gated vault with a postgres wildcard grant or prefix-nested sibling, this platform-independent family check rejects it even though the cross-type namespace overlap exists only on GCP, blocking valid AWS and Azure manifests before rendering.
Reviews (1): Last reviewed commit: "test(deployment): assert kept grants sur..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+169
to
+171
| for profile in stack.permissions.profiles.values_mut() { | ||
| profile.0.shift_remove(resource_id.as_str()); | ||
| } |
There was a problem hiding this comment.
When an initial setup declines a gated resource with a resource-scoped management grant, this loop cleans only named profiles and leaves the separate management profile unchanged, causing downstream emitters and permission controllers to retain management access to the declined resource's namespace.
Knowledge Base Used: Deployment Engine (alien-deployment)
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/alien-deployment/src/pending.rs
Line: 169-171
Comment:
**Management grants survive decline**
When an initial setup declines a gated resource with a resource-scoped management grant, this loop cleans only named profiles and leaves the separate management profile unchanged, causing downstream emitters and permission controllers to retain management access to the declined resource's namespace.
**Knowledge Base Used:** [Deployment Engine (alien-deployment)](https://app.greptile.com/alien/-/custom-context/knowledge-base/alienplatform/alien/-/docs/deployment-engine.md)
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+112
to
+124
| let flagged_prefixes: Vec<String> = | ||
| if NAME_PREFIX_GRANTED_TYPES.contains(&resource_type.as_ref()) { | ||
| NAME_PREFIX_GRANTED_TYPES | ||
| .iter() | ||
| .map(|family_type| { | ||
| format!( | ||
| "{}/", | ||
| crate::mutations::management_permission_profile::permission_resource_type( | ||
| family_type, | ||
| ) | ||
| ) | ||
| }) | ||
| .collect() |
There was a problem hiding this comment.
When an AWS or Azure stack combines a gated vault with a postgres wildcard grant or prefix-nested sibling, this platform-independent family check rejects it even though the cross-type namespace overlap exists only on GCP, blocking valid AWS and Azure manifests before rendering.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/alien-preflights/src/compile_time/resource_enabled_valid.rs
Line: 112-124
Comment:
**Namespace family ignores platform**
When an AWS or Azure stack combines a gated vault with a postgres wildcard grant or prefix-nested sibling, this platform-independent family check rejects it even though the cross-type namespace overlap exists only on GCP, blocking valid AWS and Azure manifests before rendering.
How can I resolve this? If you propose a fix, please make it concise.
Contributor
Author
There was a problem hiding this comment.
Intentional. The check is deliberately platform-independent: a stack is portable across clouds and resource ids cannot be renamed after deploy, so a config whose vault/postgres namespaces would overlap on GCP is rejected everywhere rather than allowed to reach a GCP deploy where it leaks.
The declined-resource strip cleaned every named permission profile but not the separate management profile. An Extend/Override management profile is a second resource-scoped grant store the management role reads from, so a declined resource's entry there kept the management role a namespace grant the resource no longer backs. Strip it from the management profile too; Auto needs nothing, it is re-derived from the stripped resource set.
Adds a distribution test app with a worker plus a matched on/off pair of every gated resource type (KV, storage, queue, vault) behind .enabled(input). The test answers the four *-on inputs true and the four *-off inputs false, provisions to AWS via Terraform, then asserts each enabled resource and its grant reach the imported stack_state and the account while each declined resource is absent. Wires TestApp::EnabledDemo through the harness (input_values -> tfvars) and adds the terraform-aws matrix entry to e2e-cloud.yml behind an enabled-demo app filter.
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
1 participant
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.
Summary
Gate a
Vault(secret store) on a boolean deployer input — the same.enabled(input)gate the other stores already have — across AWSCloudFormation and AWS/GCP/Azure Terraform. Includes a small fix so a
declined resource doesn't leave a stale grant behind in the permission
profiles.
When a stack renders, for each gated resource:
that targets it — a Terraform
count/ CloudFormationConditionforstores that own a cloud block; the vault is a Parameter Store / Secret
Manager name prefix with no block, so its gate rides the IAM grant blocks
and the registration entry instead.
drop out: no cloud resource, no registration entry, and — the small fix —
no leftover resource-scoped grant in any permission profile, so nothing
re-applies that grant at runtime as a project-level binding.
*-familygrant over a gated secret namespace, or a vault/postgres nested inside a
sibling's name prefix.
What I did
cloud block (they're a name prefix), so the gate rides the IAM grant blocks
plus the dropped registration entry rather than a resource
count; Azure'sKey Vault is a real block, gated by
countwith every reference indexed.strip_declined_resourcesnow also drops a declinedresource's resource-scoped entry from every permission profile — and from an
Extend/Override management profile — not just the resource and its
registration entry. Otherwise a runtime consumer (the GCP service-account
controller, or the management role) reads that entry straight from the
profile and re-grants it after handoff.
over a gated vault/postgres, and reject a gated resource nested in a
sibling's name prefix (where a prefix wildcard would still cover it).
the gating shape); all prior queue coverage is reproduced.
Files touched
crates/alien-terraform/src/emitters/{aws,gcp,azure}/vault.rs,crates/alien-cloudformation/src/emitters/aws/vault.rs— vault gating.crates/alien-deployment/src/pending.rs— the grant-drop fix.crates/alien-preflights/src/compile_time/resource_enabled_valid.rs—secret-namespace preflight rules.
enabled_vault_queue_tests).How I tested
cargo nextest run -p alien-deployment— 89 pass, including the strip teststhat assert a declined resource's grant is gone from a named profile (while a
kept sibling's grant and the
"*"wildcard survive — the removal isexact-key, not a prefix sweep) and from an Extend/Override management profile.
cargo nextest run -p alien-cloudformation enabled_vault_queue(9) and-p alien-terraform enabled_vault_queue(15, incl.terraform validate) —both green. These resolve the whole template with the gate off and assert no
grant survives over the vault's namespace, so the check is blind to which
emitter produced a grant.
cargo nextest run -p alien-preflightssecret-namespace rules — green.terraform_aws_push_enabled_demo, wired intoe2e-cloud.ymlbehind anenabled-demoapp filter): a demo app with amatched on/off pair of every gated type provisioned to AWS via Terraform. It
asserts each
*-onresource and its grant reach the imported stack_state andthe account, and each
*-offis absent. Passed end to end (provision →assert → destroy); cleanup verified.
Security review (this PR touches IAM grants and secret access):
runtime → fails: the grant leaves every profile, so the service-account
controller has nothing to re-apply (strip test + the emitter-agnostic
"no ungated grant over the namespace" property tests).
appstill reads a live siblingapp-configthrough the<prefix>-app-*wildcard → fails: preflight rejects a gated resource nestedin a sibling's name prefix, and rejects a
*-family grant over the gatednamespace.
strip_declined_resourcesdrops the declined id from an Extend/Overridemanagement profile (test
a_declined_resource_loses_its_management_grant),the emitters gate the per-resource management grants, and the generator
hard-errors if a gated emitter doesn't opt into gating.