executors: Secret values

[eseliger]

Description

At some point we need to allow a way for users to inject secrets into their SSBC executions without adding them to the batch spec inputs directly.

I think we could do something like ${{ secret.XX }} in templating and support setting those from the UI in a secrets menu.

My current plan is to

• add a table storing executor secrets pretty generally (scope, user, org, key, value, enc_key)
• add a batch changes UI in admin, org settings and user settings each to configure those, showing when it’s overwriting a global default (batch changes UI and not generalized, because batch changes ownership of a job works differently than code intel, but we can clone the admin page version of it for code intel easily, I still want to use a generic backend)
• say the transformer methods would need to figure out what secrets are required (is domain logic, batch changes would require parsing the spec file, code intel might look for defaults or similar, ..) and send them however the jobs need them, and make sure they are added to redacted values
• add 2 special docker secrets can be used to configure the docker login/pass (this would be v0.1, it won’t support multiple credentials but I think it applies broadly enough for BC and CI for now)

For the batches transformer, this would mean parsing the batch spec to find ${{ secrets. }} values and looking for docker secrets to pass along as well.

Additional plans:

• Add audit logs when secrets are being loaded
• UI should allow to both update the value and delete it, updating the updated_at timestamp

TBD: Should we allow to specify one-off secret values? This would make it very easy to use a secret in a batch change, but also make it impossible to ever run it again (think recurring batch changes, ..)

Impacted customers

• Product gap
• https://github.com/sourcegraph/accounts/issues/7491

Our use case is to update the version of a certain npm package in about 50 repositories in a single batch change. To do that, we need to be able to install private npm packages after cloning a repository (during the batch change execution).To be able to install these private npm packages, we need to add a way to authenticate against the npm registry.
Currently in our CI environments we create an .npmrc file (that is used for authenticating against npm) and inject the access token into it from an environment variable
These other CI environments, we are allowed to pass arbitrary environment variables after setting them in the web UI, that’s what we would like to have in batch changes 🙂
What would be even better is the ability to hide these environment variables, so after they are set they cannot be read from the Web UI anymore, only by the batch change executor(s).

• https://github.com/sourcegraph/accounts/issues/580

• https://github.com/sourcegraph/accounts/issues/287

The customer would like to use server side Batch Changes. However, they must use secrets to open and assign JIRA tickets to repository owners. The customer would like to store secrets to use as environment variables in batch changes. Including the credentials of the batch change executor.

[github-actions]

Heads up @macraig - the "team/code-intelligence" label was applied to this issue.

[malomarrec]

About https://github.com/sourcegraph/accounts/issues/580.
In this customer's workflow, each changeset is created alongside a Jira ticket. To do so, their first batch change run.step is a Jira ticket creation step. To pass credentials, my understanding is that they built a thin wrapper around src-cli which allows fetching credentials (eg. from Vault) and declaring them as environment variables locally.
This workflow breaks in SSBC.

How can we enable customers to:

• pass long-lived shared credentials to the executor / container? This sounds like the topic of this ticket
• pass short-lived credentials or user-specific credentials? This sounds like a more complex problem. The customer mentioned two ideas they had: SSBC can prompt for user input at each run, or it could pull from Vault.
  What do you think @eseliger?

[eseliger]

This should also handle docker pull secrets, ideally.

[malomarrec]

This is a fast follower for after 4.0 when we have tons of customers on cloud with executors than we can design this with.

[malomarrec]

Adding to Erik's comment: related: there should be a way for users to configure which docke registry to pull from, and to specify secrets for it. This is adjacent to this feature.

[malomarrec]

Added feedback from multiple customers on use cases in there that will help inform the design. It sounds like one of these "others already solved this well, let's copy their design" type feature. A subcase that is specific to use is having an easy to understand flow for setting docker secrets that are implicitly used by the executor when pulling containers.

[malomarrec]

Here are a few considerations regarding security / access control. ACLs are a dependency of having a broader RBAC design for Sourcegraph. Besides, here are things to consider building

• You can only set secrets from the UI, not get
• Some secrets are only gettable by the executor, not the code that it runs (I can think of only one example for now: Docker registry creds)
• Audit logs: each GET of a secret generates an autit log

And then of course, ACLs ...

[malomarrec]

@macraig @efritz do you have requirements for secrets in executors? What are the use cases for code intelligence? Can you share a user story of needing to add a secret?

[macraig]

@malomarrec https://github.com/sourcegraph/accounts/issues/8285 surfaced a use case described in sourcegraph/lsif-go#255. They want to index a repo and its dependencies; one of them is a private repo that needs authentication (currently it just fails to index that private dependency).

[malomarrec]

• I wonder if Pull containers from private registry #40479 really depends on this. So wondering if there's a way to make it so registry secrets are available to the executor, but inside the container: echo ${{secrets.container}} fails / doesn't expose the secret. The reason for doing this is all users need to use the secret, but very few users have good reasons to exfiltrate it.
• The scope for this ticket sounds quite big as is (in particular, I think audit logs are necessary if there are secrets shared across the instance). Wondering if one way to make it minimal is to start with a smaller scope and only allow user-scoped secrets.

[BolajiOlajide]

@eseliger Depending on how things go, I'll love to be a part of this and work with you to get this out if possible.

[eseliger]

Learnings:

• The log redaction also interferes with the diffs produced, as they are part of the logs (would potentially be fixed by https://github.com/sourcegraph/sourcegraph/issues/42476). We could technically try to "undo" the redaction, but that would cause another potential leak of the secret value.
• We cannot determine which secrets are actually being used, because https://github.com/sourcegraph/sourcegraph/issues/41831. That means it is effectively very hard to do cache invalidation today
• We have to transfer ALL potential secrets to the executor, regardless of if they will be used. This is not the worst, but also not truly great
• The most reasonable path forward would be to not make secrets generic for executors in the most generic sense. We do have log redaction as a native feature, but passing secrets down could mean we do something separate. For codeintel that could mean they use env vars, and combine that with log redaction, for batch changes that would likely be part of the input.json file, with log redaction set up. The UI/store for those secrets would then be separate.