From 4e6a695aa2f00e360df0101bd215c385c74651af Mon Sep 17 00:00:00 2001 From: Grant Linville Date: Thu, 13 Jun 2024 15:41:41 -0400 Subject: [PATCH 1/4] fix: redact output of credential tools Signed-off-by: Grant Linville --- pkg/runner/monitor.go | 19 +++++++++++++++++++ pkg/runner/runner.go | 2 +- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/pkg/runner/monitor.go b/pkg/runner/monitor.go index 48e64b0b..526e9443 100644 --- a/pkg/runner/monitor.go +++ b/pkg/runner/monitor.go @@ -28,3 +28,22 @@ func (n noopMonitor) Stop(string, error) {} func (n noopMonitor) Pause() func() { return func() {} } + +type credWrapper struct { + m Monitor +} + +func (n credWrapper) Event(e Event) { + if e.Type == EventTypeCallFinish { + e.Content = "credential tool output redacted" + } + n.m.Event(e) +} + +func (n credWrapper) Stop(s string, err error) { + n.m.Stop(s, err) +} + +func (n credWrapper) Pause() func() { + return n.m.Pause() +} diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index f330f15f..d52236e2 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -858,7 +858,7 @@ func (r *Runner) handleCredentials(callCtx engine.Context, monitor Monitor, env return nil, fmt.Errorf("failed to create subcall context for tool %s: %w", credToolName, err) } - res, err := r.call(subCtx, monitor, env, input) + res, err := r.call(subCtx, credWrapper{monitor}, env, input) if err != nil { return nil, fmt.Errorf("failed to run credential tool %s: %w", credToolName, err) } From bf402f962e96631db38e8fd513852674d5ce25ef Mon Sep 17 00:00:00 2001 From: Grant Linville Date: Thu, 13 Jun 2024 22:47:43 -0400 Subject: [PATCH 2/4] change var name Signed-off-by: Grant Linville --- pkg/runner/monitor.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/runner/monitor.go b/pkg/runner/monitor.go index 526e9443..0efa79b5 100644 --- a/pkg/runner/monitor.go +++ b/pkg/runner/monitor.go @@ -33,17 +33,17 @@ type credWrapper struct { m Monitor } -func (n credWrapper) Event(e Event) { +func (c credWrapper) Event(e Event) { if e.Type == EventTypeCallFinish { e.Content = "credential tool output redacted" } - n.m.Event(e) + c.m.Event(e) } -func (n credWrapper) Stop(s string, err error) { - n.m.Stop(s, err) +func (c credWrapper) Stop(s string, err error) { + c.m.Stop(s, err) } -func (n credWrapper) Pause() func() { - return n.m.Pause() +func (c credWrapper) Pause() func() { + return c.m.Pause() } From 89343d2e05cfe17f1b5903db2dce2e48d7749cc4 Mon Sep 17 00:00:00 2001 From: Grant Linville Date: Fri, 14 Jun 2024 11:08:28 -0400 Subject: [PATCH 3/4] exclude output of cred tools in CallFinish events Signed-off-by: Grant Linville --- pkg/runner/monitor.go | 19 ------------------- pkg/runner/runner.go | 12 ++++++++++-- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/pkg/runner/monitor.go b/pkg/runner/monitor.go index 0efa79b5..48e64b0b 100644 --- a/pkg/runner/monitor.go +++ b/pkg/runner/monitor.go @@ -28,22 +28,3 @@ func (n noopMonitor) Stop(string, error) {} func (n noopMonitor) Pause() func() { return func() {} } - -type credWrapper struct { - m Monitor -} - -func (c credWrapper) Event(e Event) { - if e.Type == EventTypeCallFinish { - e.Content = "credential tool output redacted" - } - c.m.Event(e) -} - -func (c credWrapper) Stop(s string, err error) { - c.m.Stop(s, err) -} - -func (c credWrapper) Pause() func() { - return c.m.Pause() -} diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index d52236e2..7f04408b 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -542,7 +542,7 @@ func (r *Runner) resume(callCtx engine.Context, monitor Monitor, env []string, s Time: time.Now(), CallContext: callCtx.GetCallContext(), Type: EventTypeCallFinish, - Content: *state.Continuation.Result, + Content: getFinishEventContent(*state, callCtx), }) if callCtx.Tool.Chat { return &State{ @@ -786,6 +786,14 @@ func (r *Runner) subCalls(callCtx engine.Context, monitor Monitor, env []string, return state, callResults, nil } +func getFinishEventContent(state State, callCtx engine.Context) string { + if callCtx.ToolCategory == engine.CredentialToolCategory { + return "" + } + + return *state.Continuation.Result +} + func (r *Runner) handleCredentials(callCtx engine.Context, monitor Monitor, env []string) ([]string, error) { // Since credential tools (usually) prompt the user, we want to only run one at a time. r.credMutex.Lock() @@ -858,7 +866,7 @@ func (r *Runner) handleCredentials(callCtx engine.Context, monitor Monitor, env return nil, fmt.Errorf("failed to create subcall context for tool %s: %w", credToolName, err) } - res, err := r.call(subCtx, credWrapper{monitor}, env, input) + res, err := r.call(subCtx, monitor, env, input) if err != nil { return nil, fmt.Errorf("failed to run credential tool %s: %w", credToolName, err) } From f199c1a2b66736c8db12cbbf03492b9663d794ed Mon Sep 17 00:00:00 2001 From: Grant Linville Date: Fri, 14 Jun 2024 12:29:10 -0400 Subject: [PATCH 4/4] add comment Signed-off-by: Grant Linville --- pkg/runner/runner.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index 7f04408b..d3ef3f95 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -787,6 +787,7 @@ func (r *Runner) subCalls(callCtx engine.Context, monitor Monitor, env []string, } func getFinishEventContent(state State, callCtx engine.Context) string { + // If it is a credential tool, the finish event contains its output, which is sensitive, so we don't return it. if callCtx.ToolCategory == engine.CredentialToolCategory { return "" }