Skip to content
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

Jump to bottom

Remove PWD from provenance env #825

Merged
merged 4 commits into from
Sep 9, 2022
Merged

Remove PWD from provenance env #825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ea70100
Remove PWD from provenance env
Sep 8, 2022
4ecd21c
fix non-zero length allocation
Sep 8, 2022
faacccc
Fix pre-submits
Sep 8, 2022
2de8565
Merge branch 'main' into remove-pwd
Sep 9, 2022
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
2 changes: 1 addition & 1 deletion .github/workflows/scripts/pre-submit.e2e.go.default.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ e2e_verify_predicate_buildConfig_step_env "0" "$ATTESTATION" "[]"
e2e_verify_predicate_buildConfig_step_workingDir "0" "$ATTESTATION" "$PWD/internal/builders/go/e2e-presubmits"

# Second step is the actual compilation.
e2e_verify_predicate_buildConfig_step_env "1" "$ATTESTATION" "[\"GOOS=linux\",\"GOARCH=amd64\",\"GO111MODULE=on\",\"CGO_ENABLED=0\", \"PWD=$PWD/internal/builders/go/e2e-presubmits\"]"
e2e_verify_predicate_buildConfig_step_env "1" "$ATTESTATION" "[\"GOOS=linux\",\"GOARCH=amd64\",\"GO111MODULE=on\",\"CGO_ENABLED=0\"]"
e2e_verify_predicate_buildConfig_step_workingDir "1" "$ATTESTATION" "$PWD/internal/builders/go/e2e-presubmits"

if [[ -n "$LDFLAGS" ]]; then
Expand Down
15 changes: 0 additions & 15 deletions internal/builders/go/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@ func checkWorkingDir(t *testing.T, wd, expected string) {
func Test_runBuild(t *testing.T) {
t.Parallel()

pwd, err := os.Getwd()
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

tests := []struct {
subject string
name string
Expand Down Expand Up @@ -77,7 +72,6 @@ func Test_runBuild(t *testing.T) {
"GOARCH=amd64",
"GO111MODULE=on",
"CGO_ENABLED=0",
"PWD=" + pwd,
},
},
{
Expand All @@ -95,7 +89,6 @@ func Test_runBuild(t *testing.T) {
envs: []string{
"GOOS=linux",
"GOARCH=amd64",
"PWD=" + pwd,
},
},
{
Expand All @@ -113,7 +106,6 @@ func Test_runBuild(t *testing.T) {
envs: []string{
"GOOS=linux",
"GOARCH=amd64",
"PWD=" + pwd,
},
},
{
Expand All @@ -131,7 +123,6 @@ func Test_runBuild(t *testing.T) {
"GOARCH=amd64",
"GO111MODULE=on",
"CGO_ENABLED=0",
"PWD=" + pwd,
},
},
{
Expand All @@ -149,7 +140,6 @@ func Test_runBuild(t *testing.T) {
"GOARCH=amd64",
"GO111MODULE=on",
"CGO_ENABLED=0",
"PWD=" + pwd,
},
},
{
Expand All @@ -169,7 +159,6 @@ func Test_runBuild(t *testing.T) {
"GOARCH=amd64",
"GO111MODULE=on",
"CGO_ENABLED=0",
"PWD=" + pwd,
},
},
{
Expand All @@ -188,7 +177,6 @@ func Test_runBuild(t *testing.T) {
"GOARCH=amd64",
"GO111MODULE=on",
"CGO_ENABLED=0",
"PWD=" + pwd,
},
},
{
Expand All @@ -207,7 +195,6 @@ func Test_runBuild(t *testing.T) {
"GOARCH=amd64",
"GO111MODULE=on",
"CGO_ENABLED=0",
"PWD=" + pwd,
},
},
{
Expand All @@ -228,7 +215,6 @@ func Test_runBuild(t *testing.T) {
"GOARCH=amd64",
"GO111MODULE=on",
"CGO_ENABLED=0",
"PWD=" + pwd,
},
},
{
Expand All @@ -249,7 +235,6 @@ func Test_runBuild(t *testing.T) {
"GOARCH=amd64",
"GO111MODULE=on",
"CGO_ENABLED=0",
"PWD=" + filepath.Join(pwd, "./valid/path/"),
},
workingDir: "./valid/path/",
},
Expand Down
11 changes: 7 additions & 4 deletions internal/runner/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,20 +103,22 @@ func (r *CommandRunner) runStep(ctx context.Context, step *CommandStep, dry bool
args := step.Command[1:]

// Copy and merge the environment.
env := make([]string, len(r.Env), len(r.Env)+len(step.Env)+1)
env := make([]string, len(r.Env), len(r.Env)+len(step.Env))
copy(env, r.Env)
env = append(env, step.Env...)

// Set the POSIX PWD env var.
posixEnv := make([]string, len(env), len(env)+1)
copy(posixEnv, env)
pwd, err := filepath.Abs(step.WorkingDir)
if err != nil {
return nil, err
}
env = append(env, "PWD="+pwd)
posixEnv = append(posixEnv, "PWD="+pwd)

cmd := exec.CommandContext(ctx, name, args...)
cmd.Dir = pwd
cmd.Env = env
cmd.Env = posixEnv
cmd.Stdout = os.Stdout
if r.Stdout != nil {
cmd.Stdout = r.Stdout
Expand All @@ -138,7 +140,8 @@ func (r *CommandRunner) runStep(ctx context.Context, step *CommandStep, dry bool
}

return &CommandStep{
Command: append([]string{name}, args...),
Command: append([]string{name}, args...),
// NOTE: We don't actually include POSIX env vars as they are redundant.
Env: env,
WorkingDir: pwd,
}, nil
Expand Down
10 changes: 6 additions & 4 deletions internal/runner/runner_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ func TestCommandRunner_StepEnv(t *testing.T) {
Command: []string{"bash", "-c", "echo -n $TEST"},
// NOTE: this overrides other env var.
Env: []string{"TEST=fuga"},
// NOTE: WorkingDir default to CWD
},
},
Stdout: out,
Expand All @@ -51,7 +52,7 @@ func TestCommandRunner_StepEnv(t *testing.T) {
{
Command: []string{"bash", "-c", "echo -n $TEST"},
// TODO(https://github.com/slsa-framework/slsa-github-generator/issues/782): de-duplicate env.
Env: []string{"TEST=hoge", "TEST=fuga", "PWD=" + pwd},
Env: []string{"TEST=hoge", "TEST=fuga"},
WorkingDir: pwd,
},
})
Expand All @@ -73,6 +74,7 @@ func TestCommandRunner_RunnerEnv(t *testing.T) {
Command: []string{"bash", "-c", "echo -n $STEP"},
// NOTE: this overrides other env var.
Env: []string{"STEP=fuga"},
// NOTE: WorkingDir default to CWD
},
},
Stdout: out,
Expand All @@ -91,7 +93,7 @@ func TestCommandRunner_RunnerEnv(t *testing.T) {
diff := cmp.Diff(steps, []*CommandStep{
{
Command: []string{"bash", "-c", "echo -n $STEP"},
Env: []string{"RUNNER=hoge", "STEP=fuga", "PWD=" + pwd},
Env: []string{"RUNNER=hoge", "STEP=fuga"},
WorkingDir: pwd,
},
})
Expand Down Expand Up @@ -133,12 +135,12 @@ func TestCommandRunner_RunnerMulti(t *testing.T) {
diff := cmp.Diff(steps, []*CommandStep{
{
Command: []string{"bash", "-c", "echo $STEP1"},
Env: []string{"STEP1=hoge", "PWD=" + pwd},
Env: []string{"STEP1=hoge"},
WorkingDir: pwd,
},
{
Command: []string{"bash", "-c", "echo $STEP2"},
Env: []string{"STEP2=fuga", "PWD=" + pwd},
Env: []string{"STEP2=fuga"},
WorkingDir: pwd,
},
})
Expand Down