Skip to content

Commit

Permalink
Prevent unneeded secrets injection in task env
Browse files Browse the repository at this point in the history
  • Loading branch information
pditommaso committed Jan 27, 2022
1 parent dfa8679 commit 26b4be9
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ class BashWrapperBuilder {

protected String getSecretsEnv() {
return SecretsLoader.isEnabled()
? SecretsLoader.instance.load() .getSecretsEnv()
? SecretsLoader.instance.load() .getSecretsEnv(secretNames)
: null
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,19 @@ class LocalSecretsProvider implements SecretsProvider, Closeable {
}

@Override
String getSecretsEnv() {
String getSecretsEnv(List<String> secretNames) {
if( !secretNames )
return null
final filter = secretNames.collect(it -> "-e '$it=.*'").join(' ')
final tmp = makeTempSecretsFile()
return tmp ? "source $tmp" : null
// mac does not allow source an anonymous pipe
// https://stackoverflow.com/a/32596626/395921
return tmp ? "source /dev/stdin <<<\"\$(cat <(grep -w $filter $tmp))\"" : null
}

@Deprecated
String getSecretsEnv() {
return getSecretsEnv(null)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,9 @@ interface SecretsProvider extends ExtensionPoint, Closeable {
/**
* @return A shell snippet defining the secrets as environment variables
*/
String getSecretsEnv(List<String> secretNames)

@Deprecated
String getSecretsEnv()

}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,12 @@ class DummySecretsProvider implements SecretsProvider {
@Override
void close() throws IOException { }

@Deprecated
String getSecretsEnv() {
return null
}

String getSecretsEnv(List<String> names) {
String result = ''
target.each { k,v -> result += "export $k=$v\n" }
return result
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,27 +196,40 @@ class LocalSecretsProviderTest extends Specification {
def folder = Files.createTempDirectory('test')
def secretFile = folder.resolve('secrets.json');
and:
def FOO = new SecretImpl('foo','x')
def BAR = new SecretImpl('bar', 'y')
def ALPHA = new SecretImpl('alpha','a')
def AALPHA = new SecretImpl('aalpha', 'b')
def DELTA = new SecretImpl('delta', 'd')
def OMEGA = new SecretImpl('omega', 'o')

def provider = new LocalSecretsProvider(storeFile: secretFile)
and:
provider.putSecret(FOO)
provider.putSecret(BAR)
provider.putSecret(ALPHA)
provider.putSecret(AALPHA)
provider.putSecret(DELTA)
provider.putSecret(OMEGA)

when:
def file = provider.makeTempSecretsFile()
then:
file.permissions == 'rw-------'
and:
file.text == '''\
export bar="y"
export foo="x"
export aalpha="b"
export alpha="a"
export delta="d"
export omega="o"
'''.stripIndent()

when:
def env = provider.getSecretsEnv()
def env = provider.getSecretsEnv(['alpha','omega'])
then:
env == "source /dev/stdin <<<\"\$(cat <(grep -w -e 'alpha=.*' -e 'omega=.*' $file))\""

when:
def result = ['env', '-i', 'bash', '-c', "$env; env|sort"].execute().text
then:
env == "source $file"
result.count('alpha=a')==1
result.count('omega=o')==1

cleanup:
folder?.deleteDir()
Expand Down

0 comments on commit 26b4be9

Please sign in to comment.