Skip to content

bug: revert removing github cred check #925

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.

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

Merged
merged 1 commit into from
Jan 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions pkg/engine/call.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ func (e *Engine) runCall(ctx Context, tool types.Tool, input string) (*Return, e
"TOOL_CALL_ARGS": strings.Join(toolNameArgs, " "),
"TOOL_CALL_BODY": body,
}))
if err != nil {
return nil, fmt.Errorf("failed to merge inputs for tool calls: %w", err)
}

newCtx := ctx
newCtx.Tool = ctx.Program.ToolSet[ref.ToolID]
Expand Down
43 changes: 31 additions & 12 deletions pkg/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"sort"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -791,9 +792,18 @@ func (r *Runner) handleCredentials(callCtx engine.Context, monitor Monitor, env
refresh bool
)

c, exists, err = r.credStore.Get(callCtx.Ctx, credName)
if err != nil {
return nil, fmt.Errorf("failed to get credentials for tool %s: %w", toolName, err)
// Only try to look up the cred if the tool is on GitHub or has an alias.
// If it is a GitHub tool and has an alias, the alias overrides the tool name, so we use it as the credential name.
if isGitHubTool(toolName) && credentialAlias == "" {
c, exists, err = r.credStore.Get(callCtx.Ctx, toolName)
if err != nil {
return nil, fmt.Errorf("failed to get credentials for tool %s: %w", toolName, err)
}
} else if credentialAlias != "" {
c, exists, err = r.credStore.Get(callCtx.Ctx, credentialAlias)
if err != nil {
return nil, fmt.Errorf("failed to get credential %s: %w", credentialAlias, err)
}
}

if c == nil {
Expand Down Expand Up @@ -859,17 +869,22 @@ func (r *Runner) handleCredentials(callCtx engine.Context, monitor Monitor, env
}

if !resultCredential.Ephemeral {
if isEmpty {
log.Warnf("Not saving empty credential for tool %s", toolName)
} else {
if refresh {
err = r.credStore.Refresh(callCtx.Ctx, resultCredential)
// Only store the credential if the tool is on GitHub or has an alias, and the credential is non-empty.
if (isGitHubTool(toolName) && callCtx.Program.ToolSet[ref.ToolID].Source.Repo != nil) || credentialAlias != "" {
if isEmpty {
log.Warnf("Not saving empty credential for tool %s", toolName)
} else {
err = r.credStore.Add(callCtx.Ctx, resultCredential)
}
if err != nil {
return nil, fmt.Errorf("failed to save credential for tool %s: %w", toolName, err)
if refresh {
err = r.credStore.Refresh(callCtx.Ctx, resultCredential)
} else {
err = r.credStore.Add(callCtx.Ctx, resultCredential)
}
if err != nil {
return nil, fmt.Errorf("failed to save credential for tool %s: %w", toolName, err)
}
}
} else {
log.Warnf("Not saving credential for tool %s - credentials will only be saved for tools from GitHub, or tools that use aliases.", toolName)
}
}
} else {
Expand All @@ -891,3 +906,7 @@ func (r *Runner) handleCredentials(callCtx engine.Context, monitor Monitor, env

return env, nil
}

func isGitHubTool(toolName string) bool {
return strings.HasPrefix(toolName, "github.com")
}
Loading