WIP: Secret Providers POC

[aluzzardi]

• Change how secrets are managed so they are requested just in time by
  the Engine from the CLI whenever they're needed, rather than stored in plaintext from the get go.
• New (still ugly) MapSecret API
  • Similar to AddSecret but instead of providing the plaintext value
    we map the URI of an external secret (e.g. vault://...) to a dagger.Secret
  • Need re-thinking
    • 1. we may only want the CLI to map a secret (e.g. avoid a module "jail breaking" by mapping more external secrets)
    • 2. MapSecret was shoved into the current API, some bits don't make sense
• CLI implements 2 quick & dirty POC secret providers
  • env: like before
  • op: 1Password

Example:

# Env
BAR=baz dagger core container from --address alpine with-mounted-secret --path /tmp/secret.txt --source env://BAR terminal

# 1Password
dagger core container from --address alpine with-mounted-secret --path /tmp/secret.txt --source "op://Infra/ITEM/credential" terminal

[aluzzardi]

PS: @sipsma @vito I have no idea what I'm doing ... I followed the socket pattern where it looks like we're "proxying" the BK session (bk -> engine -> cli). There's a lot of concepts in there however: bk session, nesting, clients, metadata, IDs ... and I don't know how much I missed

[sipsma]

@aluzzardi Noice 😎

I have no idea what I'm doing ... I followed the socket pattern where it looks like we're "proxying" the BK session (bk -> engine -> cli). There's a lot of concepts in there however: bk session, nesting, clients, metadata, IDs ... and I don't know how much I missed

On a first quick glance this looks good so far!

I really really don't like that proxying part either, but it's necessary to coerce buildkit into working with our dagger session that consists of multiple clients. I'd prefer it was just engine -> cli but 🤷‍♂️ for now.

we may only want the CLI to map a secret (e.g. avoid a module "jail breaking" by mapping more external secrets)

Fortunately, I think that jail breaking is already protected against in the code you have so far. Worth an integ test to confirm nonetheless, once you're at that point.

There's more details in the PR that added support for this, but basically each client in a session gets its own isolated secret store. The CLI is one client and every function call is its own client, so the secrets they can access are isolated from each other even though they are in the same session.

The end result is that if a module called mapSecret as you wrote it, it wouldn't just magically be mapping secrets from the CLI. It would actually be mapping the secrets to itself (which is weird, but not a jailbreak).

That being said, I would probably still call this function in mapSecret to only allow the CLI to call it, but just for correctness. Modules run in ephemeral containers so if they called it they may not be around any more to give the secret on demand, which would just be a confusing error.

---

(longer term thoughts)

In follow-ups, it would be cool if mapSecret could additionally point to a Function that gets called on demand to retrieve the secret plaintext.

If we have that, I think we could+should deprecate SetSecret. If all secrets were obtained on demand rather than being mutable things stored in memory so so so much would be simplified (we could probably just flip on function call caching by default, for example)

[jedevc]

General question for @sipsma - do we still need this "accessor" property? Now we've got the secret separation via the SecretsStore is this still something that we need?

Or is it still required to ensure we still get actually unique values per module?

[jedevc]

I might be missing something - but this isn't defined anywhere? I'm guessing this is where the actual env/op logic should be?

[aluzzardi]

Forgot to git add :)

[jedevc]

CLI implements 2 quick & dirty POC secret providers

Bit of an open question (and not really to-be-tackled in this PR), but how do we want to handle all of these? Are we gonna maintain all of these in-tree, or would we want to consider a plugin mechanism?

However we do it, I think we should be prepared for a lot of providers, and make it easy for users/contributors to add new ones - some ones that I would love:

• OnePassword/Bitwarden/Infiscal/Vault/etc
• pass, keepass
• Kubernetes

Maybe we should keep the exec hook so we can easily allow extending with custom secret providers without needing source changes?

• Similar to AddSecret but instead of providing the plaintext value
  we map the URI of an external secret (e.g. vault://...) to a dagger.Secret

How would this work if there are multiple potential services for a single provider, e.g.:

• logged into two one password sessions at once (maybe not possible)
• have multiple kubeconfigs
• need to connect to a remote vault, etc.

Would each provider have a similar scheme? Or would it be up to each provider to determine how to address a service, and then a secret within that service?

(longer term thoughts)

In follow-ups, it would be cool if mapSecret could additionally point to a Function that gets called on demand to retrieve the secret plaintext.

Cool - I like this 🎉 Don't wanna derail too far, but just curious about this!

If we have that, I think we could+should deprecate SetSecret. If all secrets were obtained on demand rather than being mutable things stored in memory so so so much would be simplified (we could probably just flip on function call caching by default, for example)

Maybe 🤔 There's a use-case where I think it's still useful, even if we have all of this. I can take a secret like a password, but I need to "manipulate" the password into a config file (or a header, or an encoded env var) - see a couple of examples in our ci:

dagger/.dagger/sdk_typescript.go

Lines 202 to 211 in 006c2ff

	if !dryRun {
	plaintext, err := npmToken.Plaintext(ctx)
	if err != nil {
	return err
	}
	npmrc := fmt.Sprintf(`//registry.npmjs.org/:_authToken=%s
	registry=https://registry.npmjs.org/
	always-auth=true`, plaintext)
	build = build.WithMountedSecret(".npmrc", dag.SetSecret("npmrc", npmrc))
	}

dagger/.dagger/sdk.go

Lines 133 to 143 in 006c2ff

	if !opts.dryRun {
	githubTokenRaw, err := opts.githubToken.Plaintext(ctx)
	if err != nil {
	return err
	}
	encodedPAT := base64.URLEncoding.EncodeToString([]byte("pat:" + githubTokenRaw))
	git = git.
	WithEnvVariable("GIT_CONFIG_COUNT", "1").
	WithEnvVariable("GIT_CONFIG_KEY_0", "http.https://github.com/.extraheader").
	WithSecretVariable("GIT_CONFIG_VALUE_0", dag.SetSecret("GITHUB_HEADER", fmt.Sprintf("AUTHORIZATION: Basic %s", encodedPAT)))
	}

These do feel like a bit of a special case - they're "derived" secrets - I wonder if we could somehow support these in the API as the well? Maybe mapSecret with functions and self-calls would be enough to handle this case?

[kpenfound]

@sipsma

If we have that, I think we could+should deprecate SetSecret. If all secrets were obtained on demand rather than being mutable things stored in memory so so so much would be simplified (we could probably just flip on function call caching by default, for example)

+1000

@jedevc

Bit of an open question (and not really to-be-tackled in this PR), but how do we want to handle all of these? Are we gonna maintain all of these in-tree, or would we want to consider a plugin mechanism?

IMO, a plugin mechanism would be ideal but I don't know what kind of complexity that adds. Without it, we'd be able to maintain support for a good number of providers in-tree, especially with community support, but there will always be a huge list of requests for more providers

[marcosnils]

Bit of an open question (and not really to-be-tackled in this PR), but how do we want to handle all of these? Are we gonna maintain all of these in-tree, or would we want to consider a plugin mechanism?

👋 chiming in here. The original idea in my head was if these plugins could be also modules so they could both serve as secret providers as well as stand-alone modules that could be also used for other types of operations. Still haven't through about all the details, just sharing some random ideas that came across my head while thinking about this 🤝

[shykes]

@aluzzardi since the current API was designed with this extra feature in mind (#4426) couldn't you simply use the human-readable secret name as the url, and no longer need mapsecret()?

[shykes]

1. we may only want the CLI to map a secret (e.g. avoid a module "jail breaking" by mapping more external secrets)

Personally I like the idea of leaving the CLI in charge of the mapping. It's not just a matter of sandboxing (I saw @sipsma 's reassuring comment on that dimension) but also of portability. I don't think we want modules hardcoding op:// urls, that's guaranteed to not be reusable anywhere.

I like the idea of using "virtual environments" as the interface for CLI-controlled mapping, wdyt @sipsma @aluzzardi @kpenfound ?

• We standardize on a standard unix-style env API, key/val strings.
• API can request specific env key. We could even use them as defaults for secret arguments (// +defaultKey=API_TOKEN)
• CLI is in charge of mapping. mapping would be configurable for the entire env, instead of secret-by-secret
• Fine-grained per-secret URL would still be supported

[jedevc]

We standardize on a standard unix-style env API, key/val strings

API can request specific env key

How would we present a 1-password vault in this style? I'm not too sure.

Maybe something like this:

func (*Module) Function(
	// +defaultKey="API_TOKEN"
	password *dagger.Secret,
) error {

Then:

$ dagger config map-secret API_TOKEN op://...
$ dagger call function

Or maybe even an interactive session to do all the secrets at once?

It feels like we still need some manual config and setup on the part of a user, because how would we map API_TOKEN into an op:// automagically?

[aluzzardi]

I like the idea of using "virtual environments" as the interface for CLI-controlled mapping, wdyt @sipsma @aluzzardi @kpenfound ?

I think that's interesting but not specific to secrets. Arguments do change based on the environment, but IMO it affects all types, nothing "special" about secrets. For instance:DoIt(bucket string, awsCreds dagger.Secret, production bool). In this example, awsCreds are 100% co-dependant on bucket (changing one requires changing the other -- it's the creds FOR the bucket).

The current solution to avoid passing a bunch of arguments has been through default values. I think it might make sense exploring a different interface for passing arguments that can be environment specific. Something like, in order of preference: 1) In-code default value 2) Environment specific file 3) flag

[shykes]

We standardize on a standard unix-style env API, key/val strings

API can request specific env key

How would we present a 1-password vault in this style? I'm not too sure.

The most important question :)

My thinking is that to use this "virtual env" feature in your vault or password manager, you would have to adapt the layout of your vault. Specifically, you would have to create a single item containing all the key-values for your env, using custom fields.

I believe this is a known pattern, although perhaps not ubiquitous. I think, if it passes the gut check of @kpenfound, we should consider "blessing" that pattern as the correct one. It would make the problem of mapping go away completely, by moving it entirely in the password manager. In that way it's much simpler: Dagger doesn't need to implement any custom configuration for mapping: you simply use the tool you already have, to organize your environment as you see fit. Then you just pass that item to dagger. For example:

$ dagger --env=op://personal/acme/shykes-dev-env call deploy --token TOKEN

Under the hood, the CLI would read the secret value from op://personal/acme/shykes-dev-env/TOKEN

It feels like we still need some manual config and setup on the part of a user, because how would we map API_TOKEN into an op:// automagically?

Yes sometimes manual configuration will still be needed. In those cases, you would do as in this POC: pass op:// directly as value for one particular argument. Eg.

$ dagger --env=op://personal/acme/shykes-dev-env call deploy --token TOKEN --other-token op://shared/foo/some-other-place

[aluzzardi]

IMO, a plugin mechanism would be ideal but I don't know what kind of complexity that adds

My 2 cents, penciled in opinion:

1. A plugin mechanism is hard to design and, even more, maintain without breaking

• It would be our first foray into "meaningful interface" rather than mapping 1:1 what's in the module
• Using containers for secret plugins is not ideal because sandboxing prevents trust
  • Secret providers would themselves need secrets to work (e.g. vault needs a vault token that's on the host)
  • More advanced use cases just don't work (e.g. in this POC I'm spawning the op CLI for 1Password which communicates over a proprietary API with the local 1Password agent)

2. There are many secret providers, but it's a finite number

• I think with 5-10 in-tree providers we could handle 80-90% of use cases
• Having the env and cmd provider as a escape hatch would cover the remaining 10-20% of use cases

Based on the 2 assumptions above, IMO we should:

• Start with in-tree providers to get proper secret support out the door ASAP
• Eventually, take our time to provide an extensible design

[aluzzardi]

In follow-ups, it would be cool if mapSecret could additionally point to a Function that gets called on demand to retrieve the secret plaintext.

If we have that, I think we could+should deprecate SetSecret. If all secrets were obtained on demand rather than being mutable things stored in memory so so so much would be simplified (we could probably just flip on function call caching by default, for example)

Totally. Not sure how exactly, but I hope we can get rid of SetSecret in this PR -- the only remaining use case is a module returning a brand new secret. The "point to a Function" is interesting, no idea how that would work though?

[aluzzardi]

I believe this is a known pattern, although perhaps not ubiquitous. I think, if it passes the gut check of @kpenfound, we should consider "blessing" that pattern as the correct one. It would make the problem of mapping go away completely, by moving it entirely in the password manager. In that way it's much simpler: Dagger doesn't need to implement any custom configuration for mapping: you simply use the tool you already have, to organize your environment as you see fit. Then you just pass that item to dagger. For example:

$ dagger --env=op://personal/acme/shykes-dev-env call deploy --token TOKEN

I think this is too constraining as it requires re-tooling the secrets around dagger. Users would lose many of the benefits provided by their secret store (individual secret permissions, ownership, auditing, expiry, rotation, ...). Providers offer more than simple k/v storage (SSH key generation, Cloud tokens based on IAM, etc).

Doing this would be the equivalent of saying we don't support external secrets, instead dagger has its own secret mechanism. Similar to SOPS & co where the secrets are handled off platform, and the secret store is only there to store the key encrypting key.

Which is a valid model, but it's not a 1:1 alternative: either we support fetching secrets from providers, or we have our own secret mechanism and rely on an external vault to store the sensitive bits (whether it's a key encrypting key or an envrc-like file)

[shykes]

Dagger doesn't need to implement any custom configuration for mapping: you simply use the tool you already have, to organize your environment as you see fit. Then you just pass that item to dagger. For example:

$ dagger --env=op://personal/acme/shykes-dev-env call deploy --token TOKEN

I think this is too constraining as it requires re-tooling the secrets around dagger. Users would lose many of the benefits provided by their secret store (individual secret permissions, ownership, auditing, expiry, rotation, ...). Providers offer more than simple k/v storage (SSH key generation, Cloud tokens based on IAM, etc).

Doing this would be the equivalent of saying we don't support external secrets, instead dagger has its own secret mechanism. Similar to SOPS & co where the secrets are handled off platform, and the secret store is only there to store the key encrypting key.

Which is a valid model, but it's not a 1:1 alternative: either we support fetching secrets from providers, or we have our own secret mechanism and rely on an external vault to store the sensitive bits (whether it's a key encrypting key or an envrc-like file)

What I was suggesting was to have both as layered features:

1. Explicit per-argument mapping as you proposed
2. Virtual environments for convenience, in exchange for less fine-grained control

My issue is, if we only support the first layer (explicit per-argument mapping), how do we resolve the convenience problem? One of the most requested features is "contextual auth". How would you suggest we solve that problem, without having eg. op://shykes/super-specific/not-portable/path hardcoded in module source?

[aluzzardi]

My issue is, if we only support the first layer (explicit per-argument mapping), how do we resolve the convenience problem? One of the most requested features is "contextual auth". How would you suggest we solve that problem, without having eg. op://shykes/super-specific/not-portable/path hardcoded in module source?

Replied in an earlier comment #8730 (comment)

IMHO it's not tied to secrets specifically. Using the previous example, you'd probably have to do something like:

$ dagger --env=op://personal/acme/shykes-dev-env call deploy \
  --token TOKEN --bucket s3://shykes-own-non-production-bucket/assets

Assuming bucket defaults to s3://dagger-inc-production-bucket and op://personal/acme/shykes-dev-env does NOT have the credentials to interact with that (nor you wanted to interact with that bucket from your laptop, even if you had permissions).

You could stash the bucket in the secrets as well, but then it becomes this shadow system to map arbitrary k/v to environments, and everything becomes a secret. For instance, production bool would become a production *dagger.Secret which you're responsible to parse. Not to mention the loss in visibility from traces/TUI and the potential caching downsides.

In summary:

• I agree with the lack of convenience
• I think however it should be addressed in a secret-agnostic way, e.g. mapping arbitrary function inputs whether they're secrets or not
• The mapping doesn't need to be stored in a secret provider, provided the mapping does not contain sensitive data (e.g. map to a secret URI rather than the plaintext value)
• Environment mapping is orthogonal to secret providers. Without giving it too much thought, as an example design, we could have a dagger "envrc"-like that can map any argument to any value
  • Secrets are mapped to URIs (this spec). Therefore envrc files don't need to be secret (e.g. they can be committed in plaintext)
  • You could potentially have a collection of per-environment "envrc" files (e.g. .daggerc, .daggerrc_dev, ...)

[kpenfound]

My thinking is that to use this "virtual env" feature in your vault or password manager, you would have to adapt the layout of your vault. Specifically, you would have to create a single item containing all the key-values for your env, using custom fields.

I believe this is a known pattern, although perhaps not ubiquitous. I think, if it passes the gut check of @kpenfound, we should consider "blessing" that pattern as the correct one

This is where we start getting into the differences of "password managers" and "secret managers" IMO. In this way, they probably have different usage patterns with a tool like Dagger and we probably want both to feel natural. What I mean is, I could see a call with a "password manager" pointing to each individual value and looking something like:

dagger call my-pipeline --user op://foo/bar/user --password op://foo/bar/password --aws-token op://bar/baz/token

Those paths will point to specific secret values and the caller better have access to that secret.

On the other hand, with a "secret manager" like Vault or Infisical for example, you may have all of the credentials you need in a single map. And depending on who's asking for it, you might get handed different sets of credentials from the server. In that case, you might want a call to pull a whole map and look more like:

dagger call my-pipeline --secrets vault://ci/my-pipeline

What I was suggesting was to have both as layered features

That seems fine? I feel like adoption maturity will drive people to structure everything in a "secret manager" fashion. The part where it gets really sticky is when you're mixing password and secret managers in different environments. Like if you're using 1password locally and vault in Jenkins. In that situation I could see people using 1password entries more like secret maps to emulate what's happening in vault, etc.

[shykes]

IMHO it's not tied to secrets specifically. Using the previous example, you'd probably have to do something like:

$ dagger --env=op://personal/acme/shykes-dev-env call deploy \
  --token TOKEN --bucket s3://shykes-own-non-production-bucket/assets

Assuming bucket defaults to s3://dagger-inc-production-bucket and op://personal/acme/shykes-dev-env does NOT have the credentials to interact with that (nor you wanted to interact with that bucket from your laptop, even if you had permissions).

You could stash the bucket in the secrets as well, but then it becomes this shadow system to map arbitrary k/v to environments, and everything becomes a secret. For instance, production bool would become a production *dagger.Secret which you're responsible to parse. Not to mention the loss in visibility from traces/TUI and the potential caching downsides.

In summary:

• I agree with the lack of convenience
• I think however it should be addressed in a secret-agnostic way, e.g. mapping arbitrary function inputs whether they're secrets or not
• The mapping doesn't need to be stored in a secret provider, provided the mapping does not contain sensitive data (e.g. map to a secret URI rather than the plaintext value)

Yeah I agree it applies to other environment-specific configurations, which goes beyond secrets. I feel like "virtual environments" could be extended beyond secrets, to work with scalar configurations also. By the way that is how Github Actions "environment" feature works as well: they have secrets and "regular" variables. And of course, the original Unix environment also is used in that way.

So the revised example might be:

$ dagger --env=op://personal/acme/shykes-dev-env call deploy \
   --token TOKEN --bucket env:BUCKET

It works better in the shell:

$ dagger shell --env=op://personal/acme/shykes-dev-env
> deploy $TOKEN $BUCKET

In summary, is it fair to say the following @aluzzardi ?

1. you're not against virtual environments per se, but for them to work, they would have to:
2. Not be secrets-specific
3. Be flexible enough to be usable in real-world scenarios (eg. sometimes you can't reorganize all your secrets etc)

[github-actions]

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[jedevc]

Just to keep this moving - I think it feels like we want to support both patterns?

1. The pattern in this PR, where you explicitly map each secret
2. The virtual environments pattern, where you map one item structured for dagger, and you can lookup each TOKEN in there.

I think just doing item 1 for now feels like a good start, and definitely better than the current status quo, as long as we make sure to follow-up with item 2 (I don't feel like we need to hit both in the same PR - if we can, wonderful).

Totally. Not sure how exactly, but I hope we can get rid of SetSecret in this PR -- the only remaining use case is a module returning a brand new secret. The "point to a Function" is interesting, no idea how that would work though?

Could we maybe do that change separately? I think there's some tricky use cases like I mentioned at the end of #8730 (comment) that need some special consideration, and feel kind of separate to the design of secret providers long-term.

We need better SetSecret handling generally - maybe that's removal, or maybe we just need some tidier caching mechanics - but I think with this PR (and any follow-ups), we move the use cases where you even want to use SetSecret at all to a minimum, and we could carry on the discussion of what to do with it separately.

[shykes]

I took the liberty of rebasing

[shykes]

Here's a standalone script to try out this feature :)

#!/usr/bin/env dagger shell -q -m github.com/aluzzardi/dagger@secret-providers-poc

# op is only available as amd64 package on alpine
platform=linux/amd64

.core container --platform=$platform |
from alpine |
with-exec sh,-c,"echo https://downloads.1password.com/linux/alpinelinux/stable/ >> /etc/apk/repositories" |
with-file \
    /etc/apk/keys/support@1password.com-61ddfc31.rsa.pub \
    $(.http https://downloads.1password.com/linux/keys/alpinelinux/support@1password.com-61ddfc31.rsa.pub) |
with-exec apk,update |
with-exec apk,add,1password-cli |
with-file /bin/dagger $(cli | binary --platform=$platform) |
with-service-binding dagger-engine $(engine | service dev) |
with-env-variable _EXPERIMENTAL_DAGGER_RUNNER_HOST $(engine | service dev | endpoint --scheme=tcp) |
terminal --cmd=sh,-c,'op account add; eval $(op signin); dagger shell'

Instructions:

1. Execute the script
2. You will be prompted for 1password auth information.
3. You will land in an interactive dagger shell, with everything setup. Try .container | from alpine | with-secret-variable FOO $(.map-secret foo op://YOURVAULT/YOURSECRET) | terminal`

[vito]

Quick note for a tricky case we might need to handle (had to deal with it in Concourse):

Vault supports generating multiple fields when you request a credential, so you can have ephemeral/rapidly-rotating credentials:

$ vault read aws/creds/my-role
Key                Value
---                -----
lease_id           aws/creds/my-role/f3e92392-7d9c-09c8-c921-575d62fe80d8
lease_duration     768h
lease_renewable    true
access_key         AKIAIOWQXTLW36DV7IEA
secret_key         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
session_token     <nil>

If I were to try to pass the access key and secret key as separate flags, I might try something like this:

dagger call deploy --access-key vault://aws/creds/my-role.access_key --secret-key vault://aws/creds/my-role.secret_key

For this to work we would need to be careful not to fetch the same credential twice - if we do, I'll end up with a access key and secret key that don't work together.

Is that handled already?

[github-actions]

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[aluzzardi]

dagger call deploy --access-key vault://aws/creds/my-role.access_key --secret-key vault://aws/creds/my-role.secret_key

For this to work we would need to be careful not to fetch the same credential twice - if we do, I'll end up with a access key and secret key that don't work together.

How can the double fetch be avoided? Does that imply that we actually know the secret is vault://aws/creds/my-role and not vault://aws/creds/my-role.access_key, fetch more than the user requested (the entire thing), and keep it warm in case it's requested later on?

Or you're saying that for a given secret (vault://aws/creds/my-role.access_key), we should only fetch it once throughout the session?

edit

For the former, I think the only way would be by doing that in the provider directly: e.g. the vault provider would parse vault://aws/creds/my-role.access_key, fetch vault://aws/creds/my-role and keep it in-memory cached.

When vault://aws/creds/my-role.secret_key comes in, it would extract it from the cached vault://aws/creds/my-role secret. I don't see a generic way to do outside of the provider itself

edit edit

Is the .<field> syntax something you came up with? Looking at the docs, it seems like there's no "official" field syntax (CLI is through -field flag, GitHub Action is <path>SPACE<field>, GitLab is using vault: production/db/password # Translates to secret: kv-v2/data/production/db, field: password)

1Password is different in that the field is the last part of the path.

Looks like we'll need to handle that one at the provider level.

@kpenfound thoughts?

[aluzzardi]

I'm getting this error from all SDK tests (missing buildkit session id) ... any pointers @sipsma @helderco ?

3034: ! input: container.from.withMountedCache.withWorkdir.withDirectory.withServiceBinding.withEnvVariable.withMountedFile.withEnvVariable.withExec.withMountedSecret.withExec.sync InvalidArgument: rpc error: code = InvalidArgument desc = missing buildkit session id

[sipsma]

@aluzzardi Just looking at the code here, my best guess is that the error is coming from this line, and that field might be missing because it was newly added but isn't getting set in the AddSecret method here.

That's probably relevant since AddSecret is called when you grant another client access to it (here). So just need to make sure that field gets transferred over when the secret is added to another store.

[aluzzardi]

@sipsma 🤦 Sorry I thought there was some dark magic involved for getting the session ID automatically, this PR is quite old and missed the fact I forgot to plumb that argument through.

Surprisingly enough -- when I opened this PR, it did work (see screenshot of the original comment). Not sure why? I thought the CI errors were related to some SDK magic.

[aluzzardi]

@sipsma 🤦 Sorry I thought there was some dark magic involved for getting the session ID automatically, this PR is quite old and missed the fact I forgot to plumb that argument through.

Surprisingly enough -- when I opened this PR, it did work (see screenshot of the original comment). Not sure why? I thought the CI errors were related to some SDK magic.

Oh yeah -- I'm plumbing the session ID when calling MapSecret but not AddSecret. Design is still in progress (e.g. do we add "ephemeral secrets" through callbacks and get rid of AddSecret altogether?).

Not sure why the SDK tests are triggering the session ID code path since it's only called when Plaintext is nil (which only happens for MapSecret, which the SDK tests shouldn't be doing?)

edit: Right, that's what you mentioned

That's probably relevant since AddSecret is called when you grant another client access to it. So just need to make sure that field gets transferred over when the secret is added to another store.