Skip to content

Commit a39f549

Browse files
committed
fix: use os.Getenv for CI_JOB_TOKEN to avoid persisting it in attestations
CI_JOB_TOKEN is sensitive and should not be included in the envVars map that gets persisted in attestations. Read it directly via os.Getenv instead, while keeping the other non-sensitive reviewer env vars (CI_PROJECT_PATH, CI_MERGE_REQUEST_PROJECT_PATH) in ListEnvVars. Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
1 parent b41a71e commit a39f549

6 files changed

Lines changed: 5 additions & 9 deletions

File tree

pkg/attestation/crafter/prmetadata.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,8 @@ func extractGitLabMRMetadata(ctx context.Context, envVars map[string]string) (bo
169169
if projectPath == "" {
170170
projectPath = envVars["CI_PROJECT_PATH"]
171171
}
172-
reviewers := fetchGitLabReviewers(ctx, envVars["CI_SERVER_URL"], projectPath, mrIID, envVars["CI_JOB_TOKEN"])
172+
// CI_JOB_TOKEN is read via os.Getenv to avoid persisting it in the attestation envVars map.
173+
reviewers := fetchGitLabReviewers(ctx, envVars["CI_SERVER_URL"], projectPath, mrIID, os.Getenv("CI_JOB_TOKEN"))
173174

174175
metadata := &PRMetadata{
175176
Platform: "gitlab",

pkg/attestation/crafter/prmetadata_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,9 +323,11 @@ func TestExtractGitLabMRMetadataWithReviewers(t *testing.T) {
323323
"CI_MERGE_REQUEST_TARGET_BRANCH_NAME": "main",
324324
"CI_SERVER_URL": server.URL,
325325
"CI_MERGE_REQUEST_PROJECT_PATH": "group/project",
326-
"CI_JOB_TOKEN": "test-token",
327326
}
328327

328+
// CI_JOB_TOKEN is read via os.Getenv (not from envVars) to avoid persisting it in attestations.
329+
t.Setenv("CI_JOB_TOKEN", "test-token")
330+
329331
isMR, metadata, err := extractGitLabMRMetadata(context.Background(), envVars)
330332
require.NoError(t, err)
331333
require.True(t, isMR)

pkg/attestation/crafter/runners/daggerpipeline.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@ func (r *DaggerPipeline) ListEnvVars() []*EnvVarDefinition {
7171
{"CI_MERGE_REQUEST_PROJECT_PATH", true},
7272
{"CI_SERVER_URL", true},
7373
{"CI_PROJECT_PATH", true},
74-
{"CI_JOB_TOKEN", true},
7574
{"GITLAB_USER_LOGIN", true},
7675
}
7776
}

pkg/attestation/crafter/runners/daggerpipeline_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ func (s *daggerPipelineSuite) TestListEnvVars() {
8181
{"CI_MERGE_REQUEST_PROJECT_PATH", true},
8282
{"CI_SERVER_URL", true},
8383
{"CI_PROJECT_PATH", true},
84-
{"CI_JOB_TOKEN", true},
8584
{"GITLAB_USER_LOGIN", true},
8685
}
8786
assert.Equal(s.T(), expected, s.runner.ListEnvVars())

pkg/attestation/crafter/runners/gitlabpipeline.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ func (r *GitlabPipeline) ListEnvVars() []*EnvVarDefinition {
7979
{"CI_RUNNER_DESCRIPTION", true},
8080
{"CI_COMMIT_REF_NAME", false},
8181
{"CI_PROJECT_PATH", false},
82-
{"CI_JOB_TOKEN", true},
8382
// MR-specific variables (optional - only present in MR contexts)
8483
{"CI_PIPELINE_SOURCE", true},
8584
{"CI_MERGE_REQUEST_IID", true},

pkg/attestation/crafter/runners/gitlabpipeline_test.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ func (s *gitlabPipelineSuite) TestListEnvVars() {
9292
{"CI_RUNNER_DESCRIPTION", true},
9393
{"CI_COMMIT_REF_NAME", false},
9494
{"CI_PROJECT_PATH", false},
95-
{"CI_JOB_TOKEN", true},
9695
{"CI_PIPELINE_SOURCE", true},
9796
{"CI_MERGE_REQUEST_IID", true},
9897
{"CI_MERGE_REQUEST_TITLE", true},
@@ -119,7 +118,6 @@ func (s *gitlabPipelineSuite) TestResolveEnvVars() {
119118
"CI_COMMIT_REF_NAME": "main",
120119
"CI_SERVER_URL": "https://gitlab.com",
121120
"CI_PROJECT_PATH": "chainloop/chainloop",
122-
"CI_JOB_TOKEN": "gitlab-job-token",
123121
"CI_PIPELINE_SOURCE": "merge_request_event",
124122
"CI_MERGE_REQUEST_IID": "42",
125123
"CI_MERGE_REQUEST_TITLE": "Add new feature",
@@ -142,7 +140,6 @@ func (s *gitlabPipelineSuite) TestResolveEnvVarsWithoutRunnerDescription() {
142140
s.T().Setenv("CI_MERGE_REQUEST_TARGET_BRANCH_NAME", "")
143141
s.T().Setenv("CI_MERGE_REQUEST_PROJECT_URL", "")
144142
s.T().Setenv("CI_MERGE_REQUEST_PROJECT_PATH", "")
145-
s.T().Setenv("CI_JOB_TOKEN", "")
146143

147144
resolvedEnvVars, errors := s.runner.ResolveEnvVars()
148145
s.Empty(errors, "Should not error when optional variables are missing")
@@ -194,7 +191,6 @@ func (s *gitlabPipelineSuite) SetupTest() {
194191
t.Setenv("CI_MERGE_REQUEST_TARGET_BRANCH_NAME", "main")
195192
t.Setenv("CI_MERGE_REQUEST_PROJECT_URL", "https://gitlab.com/chainloop/chainloop/-/merge_requests/42")
196193
t.Setenv("CI_PROJECT_PATH", "chainloop/chainloop")
197-
t.Setenv("CI_JOB_TOKEN", "gitlab-job-token")
198194
t.Setenv("CI_MERGE_REQUEST_PROJECT_PATH", "chainloop/chainloop")
199195
}
200196

0 commit comments

Comments
 (0)