Skip to content

feat: add principals flag to cli for ssh dynamic secrets #238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
saifsmailbox98 wants to merge 1 commit into
base: main
Choose a base branch
from
+68 −25
Open

feat: add principals flag to cli for ssh dynamic secrets #238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
72 changes: 47 additions & 25 deletions packages/cmd/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@

const CACHE_TYPE_KUBERNETES = "kubernetes"

const DYNAMIC_SECRET_LEASE_TEMPLATE = "dynamic-secret-lease-%s-%s-%s-%s-%s"
const DYNAMIC_SECRET_LEASE_TEMPLATE = "dynamic-secret-lease-%s-%s-%s-%s-%s-%s"

Check failure on line 52 in packages/cmd/agent.go

View check run for this annotation

Claude / Claude Code Review

Cache key format change breaks backwards compatibility with existing cached leases 

The cache key template at `agent.go:52` changed from 5 to 6 placeholders, so on first agent run after upgrade any lease cached by the previous CLI version is unfindable: `ReadLeaseFromCache` now produces a key with a trailing dash (empty `principals`) that doesn't match the stored 5-segment key, causing a cache miss and a duplicate server-side lease. `DeleteUnusedLeasesFromCache` has the same problem — it reconstructs the delete key with the new 6-field format, so stale entries silently linger i
Comment thread
saifsmailbox98 marked this conversation as resolved.

// duration to reduce from expiry of dynamic leases so that it gets triggered before expiry
const DYNAMIC_SECRET_PRUNE_EXPIRE_BUFFER = -15
Expand Down Expand Up @@ -273,6 +273,7 @@
Data map[string]interface{}
TemplateIDs []int
RequestedLeaseTTL string
Principals string
}

func (c *CacheManager) WriteToCache(key string, value interface{}, ttl *time.Duration) error {
Expand Down Expand Up @@ -396,12 +397,13 @@
DYNAMIC_SECRET_LEASE_TEMPLATE,
lease.ProjectSlug,
lease.Environment,
lease.SecretPath,
lease.Slug,
requestedLeaseTTL,
lease.Principals,
)

ttl := time.Until(lease.ExpireAt)

Check warning on line 406 in packages/cmd/agent.go

View check run for this annotation

Claude / Claude Code Review

Principals cache key uses raw string instead of normalized form 

Nit: the new `principals` argument is stored raw in the cache key and in-memory match (e.g. `WriteLeaseToCache` at line 400, `ReadLeaseFromCache` at line 423, `GetLeaseUnsafe`/`AppendUnsafe`/`RegisterTemplateUnsafe`/`DeleteUnusedLeasesFromCache`), but the value sent to the API is normalized (split-by-comma → trim → drop empties at lines 990-1002). So `"alice,bob"`, `"alice, bob"`, and `"bob,alice"` produce identical API requests yet distinct cache keys/identity, causing cache misses and duplicat
Comment thread
saifsmailbox98 marked this conversation as resolved.

log.Info().Msgf("[cache]: writing dynamic secret lease to cache: [cache-key=%s] [entry-ttl=%s]", cacheKey, ttl.String())

Expand All @@ -412,13 +414,13 @@
}
}

func (d *DynamicSecretLeaseManager) ReadLeaseFromCache(projectSlug, environment, secretPath, slug string, requestedLeaseTTL string) *DynamicSecretLeaseWithTTL {
func (d *DynamicSecretLeaseManager) ReadLeaseFromCache(projectSlug, environment, secretPath, slug, requestedLeaseTTL, principals string) *DynamicSecretLeaseWithTTL {

if d.cacheManager == nil || !d.cacheManager.IsEnabled {
return nil
}

cacheKey := fmt.Sprintf(DYNAMIC_SECRET_LEASE_TEMPLATE, projectSlug, environment, secretPath, slug, requestedLeaseTTL)
cacheKey := fmt.Sprintf(DYNAMIC_SECRET_LEASE_TEMPLATE, projectSlug, environment, secretPath, slug, requestedLeaseTTL, principals)
var lease *DynamicSecretLeaseWithTTL
err := d.cacheManager.ReadFromCache(cacheKey, &lease)
if err != nil {
Expand All @@ -431,12 +433,12 @@
return lease
}

func (d *DynamicSecretLeaseManager) DeleteLeaseFromCache(projectSlug, environment, secretPath, slug, requestedLeaseTTL string) error {
func (d *DynamicSecretLeaseManager) DeleteLeaseFromCache(projectSlug, environment, secretPath, slug, requestedLeaseTTL, principals string) error {
if d.cacheManager == nil || !d.cacheManager.IsEnabled {
return nil
}

cacheKey := fmt.Sprintf(DYNAMIC_SECRET_LEASE_TEMPLATE, projectSlug, environment, secretPath, slug, requestedLeaseTTL)
cacheKey := fmt.Sprintf(DYNAMIC_SECRET_LEASE_TEMPLATE, projectSlug, environment, secretPath, slug, requestedLeaseTTL, principals)
err := d.cacheManager.DeleteFromCache(cacheKey)
if err != nil {
return fmt.Errorf("unable to delete lease from cache: %v", err)
Expand Down Expand Up @@ -503,7 +505,8 @@
s.Environment == cachedLease.Environment &&
s.SecretPath == cachedLease.SecretPath &&
s.Slug == cachedLease.Slug &&
s.RequestedLeaseTTL == cachedLease.RequestedLeaseTTL
s.RequestedLeaseTTL == cachedLease.RequestedLeaseTTL &&
s.Principals == cachedLease.Principals

if match {
log.Debug().Msgf("[cache]: found matching active lease: [project=%s], [env=%s], [path=%s], [slug=%s]",
Comment on lines 511 to 512
Copy link
Copy Markdown
Contributor

@adilsitos adilsitos May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we also need to add the principals here? isn't it good for debug?

Expand Down Expand Up @@ -532,6 +535,7 @@
cachedLease.SecretPath,
cachedLease.Slug,
cachedLease.RequestedLeaseTTL,
cachedLease.Principals,
); err != nil {
log.Warn().Msgf("[cache]: unable to delete lease from cache: %v", err)
}
Expand All @@ -551,7 +555,7 @@
shouldDelete := time.Now().After(s.ExpireAt.Add(DYNAMIC_SECRET_PRUNE_EXPIRE_BUFFER * time.Second))

if shouldDelete {
if err := d.DeleteLeaseFromCache(s.ProjectSlug, s.Environment, s.SecretPath, s.Slug, s.RequestedLeaseTTL); err != nil {
if err := d.DeleteLeaseFromCache(s.ProjectSlug, s.Environment, s.SecretPath, s.Slug, s.RequestedLeaseTTL, s.Principals); err != nil {
log.Warn().Msgf("[cache]: unable to delete lease from cache: %v", err)
}
}
Expand All @@ -563,9 +567,9 @@
func (d *DynamicSecretLeaseManager) AppendUnsafe(lease DynamicSecretLeaseWithTTL) {

index := slices.IndexFunc(d.leases, func(s DynamicSecretLeaseWithTTL) bool {
// match by configuration (project, env, path, slug, TTL) and same lease ID
// match by configuration (project, env, path, slug, TTL, principals) and same lease ID
// this allows merging template IDs when the same lease is added multiple times
if lease.SecretPath == s.SecretPath && lease.Environment == s.Environment && lease.ProjectSlug == s.ProjectSlug && lease.Slug == s.Slug && lease.LeaseID == s.LeaseID && lease.RequestedLeaseTTL == s.RequestedLeaseTTL {
if lease.SecretPath == s.SecretPath && lease.Environment == s.Environment && lease.ProjectSlug == s.ProjectSlug && lease.Slug == s.Slug && lease.LeaseID == s.LeaseID && lease.RequestedLeaseTTL == s.RequestedLeaseTTL && lease.Principals == s.Principals {
return true
}
return false
Expand All @@ -588,12 +592,12 @@
}

// Expects a lock to be held before invocation
func (d *DynamicSecretLeaseManager) RegisterTemplateUnsafe(projectSlug, environment, secretPath, slug string, templateId int, requestedLeaseTTL string) {
func (d *DynamicSecretLeaseManager) RegisterTemplateUnsafe(projectSlug, environment, secretPath, slug string, templateId int, requestedLeaseTTL, principals string) {

index := slices.IndexFunc(d.leases, func(lease DynamicSecretLeaseWithTTL) bool {
// find lease by configuration, not by template ID
// this allows us to register new template IDs to existing leases
return lease.SecretPath == secretPath && lease.Environment == environment && lease.ProjectSlug == projectSlug && lease.Slug == slug && lease.RequestedLeaseTTL == requestedLeaseTTL
return lease.SecretPath == secretPath && lease.Environment == environment && lease.ProjectSlug == projectSlug && lease.Slug == slug && lease.RequestedLeaseTTL == requestedLeaseTTL && lease.Principals == principals
})

log.Debug().Msgf("\n[cache]: registering template [template-id=%d] for lease [project=%s], [env=%s], [path=%s], [slug=%s]\nIndex: %d", templateId, projectSlug, environment, secretPath, slug, index)
Expand All @@ -616,21 +620,21 @@
}

// Expects a lock to be held before invocation
func (d *DynamicSecretLeaseManager) GetLeaseUnsafe(accessToken, projectSlug, environment, secretPath, slug string, templateId int, requestedLeaseTTL string) *DynamicSecretLeaseWithTTL {
func (d *DynamicSecretLeaseManager) GetLeaseUnsafe(accessToken, projectSlug, environment, secretPath, slug string, templateId int, requestedLeaseTTL, principals string) *DynamicSecretLeaseWithTTL {
// first try to get from in-memory storage

// find lease by configuration (project, env, path, slug, TTL) regardless of template IDs
// find lease by configuration (project, env, path, slug, TTL, principals) regardless of template IDs
// this allows multiple templates to share the same lease
for i := range d.leases {
lease := &d.leases[i]
if lease.SecretPath == secretPath && lease.Environment == environment && lease.ProjectSlug == projectSlug && lease.Slug == slug && lease.RequestedLeaseTTL == requestedLeaseTTL {
if lease.SecretPath == secretPath && lease.Environment == environment && lease.ProjectSlug == projectSlug && lease.Slug == slug && lease.RequestedLeaseTTL == requestedLeaseTTL && lease.Principals == principals {
log.Debug().Msgf("[cache]: lease found in in-memory storage: [project=%s], [env=%s], [path=%s], [slug=%s]", projectSlug, environment, secretPath, slug)
return lease
}
}

// if no lease is found in in-memory storage, try to get from cache
leaseFromCache := d.ReadLeaseFromCache(projectSlug, environment, secretPath, slug, requestedLeaseTTL)
leaseFromCache := d.ReadLeaseFromCache(projectSlug, environment, secretPath, slug, requestedLeaseTTL, principals)

if leaseFromCache == nil {
log.Info().Msgf("[cache]: cache miss, no lease found [template-id=%d]", templateId)
Expand All @@ -651,7 +655,7 @@
// lease not found in API, delete it from cache and return nil
if errors.Is(err, api.ErrNotFound) {
log.Warn().Msgf("dynamic secret lease does not exist, deleting from cache: [lease-id=%s]", leaseFromCache.LeaseID)
if err := d.DeleteLeaseFromCache(leaseFromCache.ProjectSlug, leaseFromCache.Environment, leaseFromCache.SecretPath, leaseFromCache.Slug, leaseFromCache.RequestedLeaseTTL); err != nil {
if err := d.DeleteLeaseFromCache(leaseFromCache.ProjectSlug, leaseFromCache.Environment, leaseFromCache.SecretPath, leaseFromCache.Slug, leaseFromCache.RequestedLeaseTTL, leaseFromCache.Principals); err != nil {
log.Warn().Msgf("[cache]: unable to delete lease from cache: %v", err)
}

Expand All @@ -661,7 +665,7 @@
// lease is found in cache but not in the the API, and the API returned a non 404-error. We should attempt to revoke it
// at this point we know that we should be able to reach the API because we've done authentication successfully
log.Warn().Msgf("unable to get dynamic secret lease from API. Revoking lease from cache: [lease-id=%s]", leaseFromCache.LeaseID)
if err := d.DeleteLeaseFromCache(leaseFromCache.ProjectSlug, leaseFromCache.Environment, leaseFromCache.SecretPath, leaseFromCache.Slug, leaseFromCache.RequestedLeaseTTL); err != nil {
if err := d.DeleteLeaseFromCache(leaseFromCache.ProjectSlug, leaseFromCache.Environment, leaseFromCache.SecretPath, leaseFromCache.Slug, leaseFromCache.RequestedLeaseTTL, leaseFromCache.Principals); err != nil {
log.Warn().Msgf("[cache]: unable to delete lease from cache: %v", err)
}

Expand All @@ -676,7 +680,7 @@
// lease is expired or about to expire, delete from cache and attempt to revoke it
if dynamicSecretLease.Lease.ExpireAt.Before(time.Now().Add(CACHE_LEASE_EXPIRE_BUFFER)) {
log.Warn().Msgf("dynamic secret lease is expired or about to expire, deleting from cache: [lease-id=%s]", leaseFromCache.LeaseID)
if err := d.DeleteLeaseFromCache(leaseFromCache.ProjectSlug, leaseFromCache.Environment, leaseFromCache.SecretPath, leaseFromCache.Slug, leaseFromCache.RequestedLeaseTTL); err != nil {
if err := d.DeleteLeaseFromCache(leaseFromCache.ProjectSlug, leaseFromCache.Environment, leaseFromCache.SecretPath, leaseFromCache.Slug, leaseFromCache.RequestedLeaseTTL, leaseFromCache.Principals); err != nil {
log.Warn().Msgf("[cache]: unable to delete lease from cache: %v", err)
}

Expand Down Expand Up @@ -942,22 +946,25 @@
defer dynamicSecretManager.mutex.Unlock()

argLength := len(args)
if argLength != 4 && argLength != 5 {
if argLength < 4 || argLength > 6 {
return nil, fmt.Errorf("invalid arguments found for dynamic-secret function. Check template %d", templateId)
}

projectSlug, envSlug, secretPath, slug, ttl := args[0], args[1], args[2], args[3], ""
if argLength == 5 {
projectSlug, envSlug, secretPath, slug, ttl, principals := args[0], args[1], args[2], args[3], "", ""
if argLength >= 5 {
ttl = args[4]
}
if argLength == 6 {
principals = args[5]
}

dynamicSecretData := dynamicSecretManager.GetLeaseUnsafe(accessToken, projectSlug, envSlug, secretPath, slug, templateId, ttl)
dynamicSecretData := dynamicSecretManager.GetLeaseUnsafe(accessToken, projectSlug, envSlug, secretPath, slug, templateId, ttl, principals)

// if a lease is found (either in memory or in cache), we register the template and return the data
if dynamicSecretData != nil {
dynamicSecretManager.RegisterTemplateUnsafe(projectSlug, envSlug, secretPath, slug, templateId, ttl)
dynamicSecretManager.RegisterTemplateUnsafe(projectSlug, envSlug, secretPath, slug, templateId, ttl, principals)

etagData := fmt.Sprintf("%s-%s-%s-%s-%s", projectSlug, envSlug, secretPath, slug, ttl)
etagData := fmt.Sprintf("%s-%s-%s-%s-%s-%s", projectSlug, envSlug, secretPath, slug, ttl, principals)
Copy link
Copy Markdown
Contributor

@adilsitos adilsitos May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for the cache we won't have a problem if the principals are different, but can't this be a problem on the etag as well? if they change the order of the principals, this would cause a difference an we would write on disk the change (which in case wouldn't really be a change, since only the order was changed)

Not sure if this is a problem, I just wanted to raise this so we can discuss.

dynamicSecretDataBytes, err := json.Marshal(dynamicSecretData.Data)
if err != nil {
return nil, err
Expand All @@ -980,19 +987,34 @@

// if there's no lease (either in memory or in cache), we create a new lease

leaseConfig := map[string]any{}
if principals != "" {
var parsedPrincipals []string
for _, p := range strings.Split(principals, ",") {
trimmed := strings.TrimSpace(p)
if trimmed != "" {
parsedPrincipals = append(parsedPrincipals, trimmed)
}
}
if len(parsedPrincipals) > 0 {
leaseConfig["principals"] = parsedPrincipals
}
}
Comment on lines +991 to +1002
Copy link
Copy Markdown
Contributor

@adilsitos adilsitos May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this could be a helper? if we have something similar in another command we might also need something similar.


leaseData, _, res, err := temporaryInfisicalClient.DynamicSecrets().Leases().Create(infisicalSdk.CreateDynamicSecretLeaseOptions{
DynamicSecretName: slug,
ProjectSlug: projectSlug,
EnvironmentSlug: envSlug,
SecretPath: secretPath,
TTL: ttl,
Config: leaseConfig,
})

if err != nil {
return nil, err
}

dynamicSecretManager.AppendUnsafe(DynamicSecretLeaseWithTTL{LeaseID: res.Id, ExpireAt: res.ExpireAt, Environment: envSlug, SecretPath: secretPath, Slug: slug, ProjectSlug: projectSlug, Data: leaseData, TemplateIDs: []int{templateId}, RequestedLeaseTTL: ttl})
dynamicSecretManager.AppendUnsafe(DynamicSecretLeaseWithTTL{LeaseID: res.Id, ExpireAt: res.ExpireAt, Environment: envSlug, SecretPath: secretPath, Slug: slug, ProjectSlug: projectSlug, Data: leaseData, TemplateIDs: []int{templateId}, RequestedLeaseTTL: ttl, Principals: principals})

return leaseData, nil
}
Expand Down
21 changes: 21 additions & 0 deletions packages/cmd/dynamic_secrets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package cmd

import (
"context"
"strings"

"github.com/Infisical/infisical-merge/packages/api"
"github.com/Infisical/infisical-merge/packages/config"
Expand Down Expand Up @@ -271,10 +272,27 @@ func createDynamicSecretLeaseByName(cmd *cobra.Command, args []string) {
util.HandleError(err, "Unable to parse flag")
}

principalsStr, err := cmd.Flags().GetString("principals")
if err != nil {
util.HandleError(err, "Unable to parse flag")
}

config := map[string]any{}
if kubernetesNamespace != "" {
config["namespace"] = kubernetesNamespace
}
if principalsStr != "" {
var principals []string
for _, p := range strings.Split(principalsStr, ",") {
trimmed := strings.TrimSpace(p)
if trimmed != "" {
principals = append(principals, trimmed)
}
}
if len(principals) > 0 {
config["principals"] = principals
}
}

leaseCredentials, _, leaseDetails, err := infisicalClient.DynamicSecrets().Leases().Create(infisicalSdk.CreateDynamicSecretLeaseOptions{
DynamicSecretName: dynamicSecretRootCredential.Name,
Expand Down Expand Up @@ -716,6 +734,9 @@ func init() {
// Kubernetes specific flags
dynamicSecretLeaseCreateCmd.Flags().String("kubernetes-namespace", "", "The namespace to create the lease in. Only used for Kubernetes dynamic secrets.")

// SSH specific flags
dynamicSecretLeaseCreateCmd.Flags().String("principals", "", "Comma-separated list of principals for SSH dynamic secret leases")

dynamicSecretLeaseCmd.AddCommand(dynamicSecretLeaseCreateCmd)

dynamicSecretLeaseListCmd.Flags().StringP("path", "p", "/", "The path from where dynamic secret should be leased from")
Expand Down
Loading