Skip to content

Commit

Permalink
[ci_runner] Redact bazel sub command (#6661)
Browse files Browse the repository at this point in the history
  • Loading branch information
maggie-lou committed May 30, 2024
1 parent 29c1ebf commit 10a4740
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,9 @@ func (e *EventChannel) processSingleEvent(event *inpb.InvocationEvent, iid strin
if err := e.redactor.RedactAPIKey(e.ctx, event.BuildEvent); err != nil {
return err
}
e.redactor.RedactMetadata(event.BuildEvent)
if err := e.redactor.RedactMetadata(event.BuildEvent); err != nil {
return err
}
// Accumulate a subset of invocation fields in memory.
if err := e.beValues.AddEvent(event.BuildEvent); err != nil {
return err
Expand Down Expand Up @@ -1436,7 +1438,9 @@ func LookupInvocationWithCallback(ctx context.Context, env environment.Env, iid
if err := redactor.RedactAPIKeysWithSlowRegexp(ctx, event.BuildEvent); err != nil {
return err
}
redactor.RedactMetadata(event.BuildEvent)
if err := redactor.RedactMetadata(event.BuildEvent); err != nil {
return err
}
if err := beValues.AddEvent(event.BuildEvent); err != nil {
return err
}
Expand Down
2 changes: 2 additions & 0 deletions server/util/redact/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ go_library(
"//server/environment",
"//server/util/flag",
"//server/util/git",
"//server/util/status",
"@com_github_google_shlex//:shlex",
"@org_golang_google_protobuf//encoding/prototext",
],
)
Expand Down
22 changes: 19 additions & 3 deletions server/util/redact/redact.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (

"github.com/buildbuddy-io/buildbuddy/server/environment"
"github.com/buildbuddy-io/buildbuddy/server/util/flag"
"github.com/buildbuddy-io/buildbuddy/server/util/status"
"github.com/google/shlex"
"google.golang.org/protobuf/encoding/prototext"

bespb "github.com/buildbuddy-io/buildbuddy/proto/build_event_stream"
Expand Down Expand Up @@ -300,7 +302,7 @@ func splitCombinedForm(cf string) (string, string) {
return cf[:i], cf[i+1:]
}

func redactStructuredCommandLine(commandLine *clpb.CommandLine, allowedEnvVars []string) {
func redactStructuredCommandLine(commandLine *clpb.CommandLine, allowedEnvVars []string) error {
command := ""
residualChunks := []*clpb.ChunkList{}

Expand Down Expand Up @@ -348,6 +350,16 @@ func redactStructuredCommandLine(commandLine *clpb.CommandLine, allowedEnvVars [
}
}
}

// Redact bazel sub command (for remote runners)
if option.OptionName == "bazel_sub_command" {
cmdLineTokens, err := shlex.Split(option.OptionValue)
if err != nil {
return status.WrapError(err, "split bazel_sub_command")
}
redactCmdLine(cmdLineTokens)
option.OptionValue = strings.Join(cmdLineTokens, " ")
}
}
continue
}
Expand All @@ -370,6 +382,7 @@ func redactStructuredCommandLine(commandLine *clpb.CommandLine, allowedEnvVars [
redactResidualChunkList(cl)
}
}
return nil
}

func redactResidualChunkList(chunkList *clpb.ChunkList) {
Expand Down Expand Up @@ -454,7 +467,7 @@ func NewStreamingRedactor(env environment.Env) *StreamingRedactor {
}
}

func (r *StreamingRedactor) RedactMetadata(event *bespb.BuildEvent) {
func (r *StreamingRedactor) RedactMetadata(event *bespb.BuildEvent) error {
switch p := event.Payload.(type) {
case *bespb.BuildEvent_Progress:
{
Expand All @@ -477,7 +490,9 @@ func (r *StreamingRedactor) RedactMetadata(event *bespb.BuildEvent) {
}
case *bespb.BuildEvent_StructuredCommandLine:
{
redactStructuredCommandLine(p.StructuredCommandLine, r.allowedEnvVars)
if err := redactStructuredCommandLine(p.StructuredCommandLine, r.allowedEnvVars); err != nil {
return err
}
}
case *bespb.BuildEvent_OptionsParsed:
{
Expand Down Expand Up @@ -548,6 +563,7 @@ func (r *StreamingRedactor) RedactMetadata(event *bespb.BuildEvent) {
{
}
}
return nil
}

func (r *StreamingRedactor) RedactAPIKey(ctx context.Context, event *bespb.BuildEvent) error {
Expand Down
46 changes: 30 additions & 16 deletions server/util/redact/redact_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ func TestRedactMetadata_BuildStarted_RedactsOptionsDescription(t *testing.T) {
OptionsDescription: `--some_flag --another_flag`,
}

redactor.RedactMetadata(&bespb.BuildEvent{
err := redactor.RedactMetadata(&bespb.BuildEvent{
Payload: &bespb.BuildEvent_Started{Started: buildStarted},
})
require.NoError(t, err)

assert.Equal(t, "<REDACTED>", buildStarted.OptionsDescription)
}
Expand All @@ -77,11 +78,12 @@ func TestRedactMetadata_UnstructuredCommandLine_RemovesArgs(t *testing.T) {
Args: []string{"foo"},
}

redactor.RedactMetadata(&bespb.BuildEvent{
err := redactor.RedactMetadata(&bespb.BuildEvent{
Payload: &bespb.BuildEvent_UnstructuredCommandLine{
UnstructuredCommandLine: unstructuredCommandLine,
},
})
require.NoError(t, err)

assert.Equal(t, []string{}, unstructuredCommandLine.Args)
}
Expand All @@ -92,9 +94,10 @@ func TestRedactMetadata_StructuredCommandLine(t *testing.T) {
buildStarted := &bespb.BuildStarted{
OptionsDescription: "--build_metadata='ALLOW_ENV=FOO_ALLOWED,BAR_ALLOWED_PATTERN_*'",
}
redactor.RedactMetadata(&bespb.BuildEvent{
err := redactor.RedactMetadata(&bespb.BuildEvent{
Payload: &bespb.BuildEvent_Started{Started: buildStarted},
})
require.NoError(t, err)

for _, testCase := range []struct {
optionName string
Expand All @@ -121,7 +124,8 @@ func TestRedactMetadata_StructuredCommandLine(t *testing.T) {
CombinedForm: fmt.Sprintf("--%s=%s", testCase.optionName, testCase.inputValue),
}

redactor.RedactMetadata(structuredCommandLineEvent(option))
err := redactor.RedactMetadata(structuredCommandLineEvent(option))
require.NoError(t, err)

expectedCombinedForm := fmt.Sprintf("--%s=%s", testCase.optionName, testCase.expectedValue)
assert.Equal(t, expectedCombinedForm, option.CombinedForm)
Expand All @@ -138,7 +142,8 @@ func TestRedactMetadata_StructuredCommandLine(t *testing.T) {
event := structuredCommandLineEvent(option)
assert.NotEmpty(t, getCommandLineOptions(event), "sanity check: --default_override should be an option before redacting")

redactor.RedactMetadata(event)
err = redactor.RedactMetadata(event)
require.NoError(t, err)

assert.Empty(t, getCommandLineOptions(event), "--default_override options should be removed")

Expand All @@ -152,7 +157,8 @@ func TestRedactMetadata_StructuredCommandLine(t *testing.T) {
event = structuredCommandLineEvent(option)
assert.NotEmpty(t, getCommandLineOptions(event), "sanity check: EXPLICIT_COMMAND_LINE should be an option before redacting")

redactor.RedactMetadata(event)
err = redactor.RedactMetadata(event)
require.NoError(t, err)

assert.Empty(t, getCommandLineOptions(event), "EXPLICIT_COMMAND_LINE should be removed")
}
Expand Down Expand Up @@ -182,9 +188,10 @@ func TestRedactMetadata_OptionsParsed_StripsURLSecretsAndRemoteHeaders(t *testin
},
}

redactor.RedactMetadata(&bespb.BuildEvent{
err := redactor.RedactMetadata(&bespb.BuildEvent{
Payload: &bespb.BuildEvent_OptionsParsed{OptionsParsed: optionsParsed},
})
require.NoError(t, err)

assert.Equal(
t,
Expand Down Expand Up @@ -223,9 +230,10 @@ func TestRedactMetadata_ActionExecuted_StripsURLSecrets(t *testing.T) {
ActionMetadataLogs: []*bespb.File{fileWithURI("213wZJyTUyhXkj381312@uri")},
}

redactor.RedactMetadata(&bespb.BuildEvent{
err := redactor.RedactMetadata(&bespb.BuildEvent{
Payload: &bespb.BuildEvent_Action{Action: actionExecuted},
})
require.NoError(t, err)

assert.Equal(t, "uri", actionExecuted.Stdout.GetUri())
assert.Equal(t, "uri", actionExecuted.Stderr.GetUri())
Expand All @@ -239,9 +247,10 @@ func TestRedactMetadata_NamedSetOfFiles_StripsURLSecrets(t *testing.T) {
Files: []*bespb.File{fileWithURI("213wZJyTUyhXkj381312@uri")},
}

redactor.RedactMetadata(&bespb.BuildEvent{
err := redactor.RedactMetadata(&bespb.BuildEvent{
Payload: &bespb.BuildEvent_NamedSetOfFiles{NamedSetOfFiles: namedSetOfFiles},
})
require.NoError(t, err)

assert.Equal(t, "uri", namedSetOfFiles.Files[0].GetUri())
}
Expand All @@ -253,9 +262,10 @@ func TestRedactMetadata_TargetComplete_StripsURLSecrets(t *testing.T) {
DirectoryOutput: []*bespb.File{fileWithURI("213wZJyTUyhXkj381312@uri")},
}

redactor.RedactMetadata(&bespb.BuildEvent{
err := redactor.RedactMetadata(&bespb.BuildEvent{
Payload: &bespb.BuildEvent_Completed{Completed: targetComplete},
})
require.NoError(t, err)

assert.Equal(t, "uri", targetComplete.DirectoryOutput[0].GetUri())
}
Expand All @@ -267,9 +277,10 @@ func TestRedactMetadata_TestResult_StripsURLSecrets(t *testing.T) {
TestActionOutput: []*bespb.File{fileWithURI("213wZJyTUyhXkj381312@uri")},
}

redactor.RedactMetadata(&bespb.BuildEvent{
err := redactor.RedactMetadata(&bespb.BuildEvent{
Payload: &bespb.BuildEvent_TestResult{TestResult: testResult},
})
require.NoError(t, err)

assert.Equal(t, "uri", testResult.TestActionOutput[0].GetUri())
}
Expand All @@ -281,9 +292,10 @@ func TestRedactMetadata_TestSummary_StripsURLSecrets(t *testing.T) {
Failed: []*bespb.File{fileWithURI("213wZJyTUyhXkj381312@uri")},
}

redactor.RedactMetadata(&bespb.BuildEvent{
err := redactor.RedactMetadata(&bespb.BuildEvent{
Payload: &bespb.BuildEvent_TestSummary{TestSummary: testSummary},
})
require.NoError(t, err)

assert.Equal(t, "uri", testSummary.Passed[0].GetUri())
assert.Equal(t, "uri", testSummary.Failed[0].GetUri())
Expand All @@ -300,9 +312,10 @@ func TestRedactMetadata_BuildMetadata_StripsURLSecrets(t *testing.T) {
},
}

redactor.RedactMetadata(&bespb.BuildEvent{
err := redactor.RedactMetadata(&bespb.BuildEvent{
Payload: &bespb.BuildEvent_BuildMetadata{BuildMetadata: buildMetadata},
})
require.NoError(t, err)

assert.Equal(t, "https://github.com/buildbuddy-io/metadata_repo_url", buildMetadata.Metadata["REPO_URL"])
assert.Equal(t, `["--remote_header=\u003cREDACTED\u003e","--foo=SAFE"]`, buildMetadata.Metadata["EXPLICIT_COMMAND_LINE"])
Expand All @@ -316,9 +329,10 @@ func TestRedactMetadata_WorkspaceStatus_StripsRepoURLCredentials(t *testing.T) {
},
}

redactor.RedactMetadata(&bespb.BuildEvent{
err := redactor.RedactMetadata(&bespb.BuildEvent{
Payload: &bespb.BuildEvent_WorkspaceStatus{WorkspaceStatus: workspaceStatus},
})
require.NoError(t, err)

require.Equal(t, 1, len(workspaceStatus.Item))
assert.Equal(t, "https://github.com/buildbuddy-io/metadata_repo_url", workspaceStatus.Item[0].Value)
Expand Down Expand Up @@ -447,7 +461,7 @@ func TestRedactRunResidual(t *testing.T) {
chunkList := &clpb.ChunkList{
Chunk: tc.given,
}
redactor.RedactMetadata(&bespb.BuildEvent{
err := redactor.RedactMetadata(&bespb.BuildEvent{
Payload: &bespb.BuildEvent_StructuredCommandLine{
StructuredCommandLine: &clpb.CommandLine{
Sections: []*clpb.CommandLineSection{
Expand All @@ -469,7 +483,7 @@ func TestRedactRunResidual(t *testing.T) {
},
},
})

require.NoError(t, err)
require.Equal(t, tc.expected, chunkList.Chunk)
})
}
Expand Down

0 comments on commit 10a4740

Please sign in to comment.