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.

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

fix(misconf): disable git terminal prompt on tf module load #8026

Merged
merged 1 commit into from
Jan 3, 2025
Merged
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
10 changes: 7 additions & 3 deletions pkg/iac/scanners/terraform/parser/resolvers/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ func (r *remoteResolver) download(ctx context.Context, opt Options, dst string)
return err
}

var opts []getter.ClientOption

// Overwrite the file getter so that a file will be copied
getter.Getters["file"] = &getter.FileGetter{Copy: true}

Expand All @@ -87,7 +85,13 @@ func (r *remoteResolver) download(ctx context.Context, opt Options, dst string)
Pwd: opt.WorkingDir,
Getters: getter.Getters,
Mode: getter.ClientModeAny,
Options: opts,
}

terminalPrompt := os.Getenv("GIT_TERMINAL_PROMPT")
if err := os.Setenv("GIT_TERMINAL_PROMPT", "0"); err != nil {
Copy link
Member

@simar7 simar7 Dec 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it fallback to HTTP if this environment variable is set? Just wondering what happens with the current input.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable disables prompt when cloning a repository: https://git-scm.com/docs/git#Documentation/git.txt-codeGITTERMINALPROMPTcode

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was asking about the behavior that takes place when this is set.

2024-12-05T17:34:29-07:00       DEBUG   [module resolver] Caching module        key="cb2ee13f53c538b535a73ca65544a8a2"
2024-12-05T17:34:29-07:00       DEBUG   [module resolver] Downloading module    source="github.com/xxxxx/xxxxx?ref=v1"
2024-12-05T17:34:30-07:00       ERROR   [terraform evaluator] Failed to load module. Maybe try 'terraform init'?        err="failed to download: error downloading 'https://github.com/xxxxx/xxxxx.git?ref=v1': /usr/bin/git exited with 128: Cloning into '/var/folders/x7/p59nrsgs671_264kc0pv7k1m0000gn/T/.aqua/cache/cb2ee13f53c538b535a73ca65544a8a2'...\nfatal: could not read Username for 'https://github.com': terminal prompts disabled\n"
2024-12-05T17:34:30-07:00       DEBUG   [terraform evaluator] Starting post-submodules evaluation...

The error message is not entirely correct as in this case the prompt is disabled because of this changes introduced in the PR but we also didn't try downloading either to realize that it isn't a valid repo. Should we do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trivy successfully clones a repository if it is public. Otherwise, if the user has not provided credentials, Trivy logs a message that contains an error: could not read Username for “https://github.com”: terminal prompts disabled. I didn't quite understand about the fallback to HTTP, since it's the source that points to the git repository.

Copy link
Member

@simar7 simar7 Jan 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant that technically by disabling prompts, we don't differentiate between a source that doesn't exist vs a source that's private. I guess the outcome in both cases will be the same (Trivy cannot obtain it) but the error message is misleading IMO. Have I got that right?

opt.Logger.Error("Failed to set env", log.String("name", "GIT_TERMINAL_PROMPT"), log.Err(err))
} else {
defer os.Setenv("GIT_TERMINAL_PROMPT", terminalPrompt)
}

if err := client.Get(); err != nil {
Expand Down