Skip to content

fix: dynamic secret env scoping - #1024

Open
rohan-chaturvedi wants to merge 7 commits into
mainfrom
fix--dynamic-secret-env-scoping
Open

rohan-chaturvedi wants to merge 7 commits into
mainfrom
fix--dynamic-secret-env-scoping

Conversation

@rohan-chaturvedi

Copy link
Copy Markdown
Member

🔍 Overview

Misc fixes to permission enforcement and lifeccycle checks for dynamic secrets

💡 Proposed Changes

Environment scoping

  • Lease listing over REST is bound to the token's environment.
  • dynamicSecrets (app and org level) only returns secrets in environments the caller has access to. The nested leases field applies the same check.
  • deleteDynamicSecret and deleteEnvironment require access to the environment.
  • Shared helpers accessible_environment_ids() and request_accessible_env_ids() in api/utils/access/permissions.py.

Lease permissions

  • Generating a lease requires DynamicSecretLeases:create for the member or service account that will hold it. This applies to the REST secrets endpoints (only when a lease would actually be created) and to the GraphQL leaseDynamicSecret mutation.
  • leaseDynamicSecret now checks Secrets:read + DynamicSecretLeases:create, instead of Secrets:create.
  • Lease holders using a personal access token can renew and revoke their own leases over REST. The ownership check compares members with members and service accounts with service accounts.
  • GraphQL renew and revoke use active memberships and return one error for leases that can't be found.

Lease lifecycle

  • Deleting an environment, app or folder now revokes its active leases, through a pre_delete receiver on DynamicSecretLease. Revocation is best-effort and bounded:
    • short AWS client timeouts for revocations that run inside a request
    • an overall time budget per delete
    • no further attempts against a credential that proved unreachable
  • Leases that can't be revoked are logged with enough detail to clean them up manually.
  • Scheduled revocation jobs are cancelled only after the delete commits.
  • Renewal now reschedules revocation correctly, so renewed leases stay valid until their new expiry. Renewal rejects invalid TTLs, revoked leases and deleted secrets. Cancelled scheduler jobs are also removed from Redis.
  • Account deletion reuses the shared revocation helper and still fails closed if revocation fails.

Frontend

  • The lease dialogs handle an empty result without crashing.

🖼️ Screenshots or Demo

N/A. Backend permission and lifecycle changes, plus a small frontend guard.

📝 Release Notes

  • Improved permission checks and environment scoping for dynamic secrets and leases.
  • Action may be required for custom roles: generating dynamic secret leases now requires DynamicSecretLeases:create. All default roles already include it. Update custom roles and team role overrides that use dynamic secrets. This affects:
    • the CLI (phase run, phase shell, phase secrets get/export) in environments with dynamic secrets at the requested path
    • the REST API with lease=true
    • the console
    • As a workaround, the CLI can skip lease generation with --generate-leases=false.
  • Generating a lease in the console no longer requires Secrets:create. Secrets:read + DynamicSecretLeases:create is enough.
  • Deleting an environment, app or folder now immediately revokes its active dynamic secret leases. Workloads using those credentials lose access.
  • Renewed leases now stay valid until their renewed expiry. Previously they could be revoked at the original expiry.
  • Personal access token holders can renew and revoke their own leases through the API.
  • Dynamic secrets in environments you don't have access to are no longer listed.

🧪 Testing

  • New tests:
    • in backend/tests/ee/integrations/secrets/dynamic/: test_lease_view_scoping.py, test_dynamic_secret_env_scoping.py, test_lease_cascade_revocation.py, test_lease_ownership.py, test_lease_renewal.py, test_lease_create_permission.py
    • backend/tests/api/utils/test_env_access_helpers.py
    • backend/tests/graphene/mutations/test_environment_delete_access.py
  • Updated: backend/tests/api/mutations/test_delete_account.py.
  • The full backend suite passes (1,702 tests), and each commit passes on its own.
  • The key checks were mutation-tested, so the tests fail if a check is removed or weakened.
  • Verified end to end against a local stack (cloned database, AWS and scheduler mocked), comparing before and after:
    • REST and GraphQL lease flows
    • environment, app and folder deletes, including rollback and retry
    • renewal scheduling
    • account deletion

🎯 Reviewer Focus

  • backend/api/signals.py, _dynamic_secret_lease_pre_delete:
    • the savepoint around each revocation
    • the per-delete budget, keyed on the delete's origin and its outermost transaction
    • unreachable-credential handling
  • backend/ee/integrations/secrets/dynamic/utils.py:
    • revoke_lease_immediately
    • cancel_scheduled_lease_job (deferred to commit)
    • renew_dynamic_secret_lease
    • can_create_dynamic_secret_lease
  • backend/api/views/secrets.py: the lease permission check only runs when a lease would be created.
  • backend/ee/integrations/secrets/dynamic/rest/views.py and graphene/mutations.py: ownership checks.

➕ Additional Context

  • No migrations and no GraphQL schema changes.
  • Commits are split by concern to make review easier.

✨ How to Test the Changes Locally

Setup:

  • An Enterprise plan org with an AWS integration pointing at a sandbox account.
  • An SSE-enabled app with dev and prod environments, and a dynamic secret in each.
  • Members: an Owner, and a Developer with access to dev only.
  • A custom role with Secrets:read but without DynamicSecretLeases:create, assigned to a member and a service account.
  1. Scoping: as the dev-only Developer, prod dynamic secrets and leases are not listed. Deleting the prod dynamic secret through the API is refused.
  2. Lease permission:
    • With a Developer PAT or a default Service role token, phase run in an environment with dynamic secrets creates leases.
    • With the custom role, it fails with a permission error and no lease is created. --generate-leases=false still works, as do environments without dynamic secrets.
    • Granting DynamicSecretLeases:create restores access.
  3. Delete cleanup:
    • Create leases in a custom environment and confirm the IAM users exist, then delete the environment. Repeat for a folder and an app. The IAM users are removed and the delete completes quickly.
    • With the AWS integration broken, the delete still succeeds and the backend logs which leases need manual cleanup.
  4. Renewal:
    • Create a short-lived lease and renew it. The IAM user survives past the original expiry and is revoked at the new one.
    • With a PAT, renewing or revoking your own lease is allowed, and another member's lease is denied.
  5. Tests:
    docker compose -f dev-docker-compose.yml exec backend pytest tests/ee/integrations/secrets/dynamic tests/graphene/mutations/test_environment_delete_access.py tests/api/mutations/test_delete_account.py tests/api/utils/test_env_access_helpers.py

💚 Did You...

  • Ensure linting passes (code style checks)?
  • Update dependencies and lockfiles (if required)
  • Update migrations (if required)
  • Regenerate graphql schema and types (if required)
  • Verify the app builds locally?
  • Manually test the changes on different browsers/devices?

Comment thread backend/api/signals.py Dismissed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants