-
Notifications
You must be signed in to change notification settings - Fork 49
RTECO-1574 - Implementation of Nuget Support for client - #532
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
Open
bhanurp
wants to merge
42
commits into
main
Choose a base branch
from
RTECO-1574-nuget-flexpack-support
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 27 commits
Commits
Show all changes
42 commits
Select commit
Hold shift + click to select a range
6f89a46
RTECO-0000 - Updated dependency of oras to 2.6.1 from 2.6.0
bhanurp ba44e05
RTECO-1574 - RTECO-1574 - Implementation of Nuget Support for client -
bhanurp 4c07eea
Fix NuGet FlexPack push/pack build-info and add missing --source flag
bhanurp 55bd344
Bump build-info-go replace to pick up NuGet FlexPack fixes
bhanurp 096fa2c
Merge remote-tracking branch 'upstream/main' into RTECO-1574
bhanurp 63a5fd2
Bump build-info-go replace to latest fork commit (post upstream/main …
bhanurp 17dc462
Fix Go-Sec/errcheck: explicitly discard os.RemoveAll error in temp-di…
bhanurp 91f6261
Bump build-info-go replace to latest fork commit (Go-Sec fix)
bhanurp 0f13480
Bump build-info-go replace to latest fork commit (post upstream/main …
bhanurp cb1d075
RTECO-1574 - Replace temp nuget.config with rank-1 credential injection
bhanurp 2e527f2
RTECO-1574 - Fix nuget.exe push 403 and credential injection for restore
bhanurp 3074096
RTECO-1574 - Extend bypass flag handling to both toolchains and fix p…
bhanurp 5cad7df
RTECO-1574 - Merge upstream/main (ruby native support) and bump build…
bhanurp aa367d3
RTECO-1574 - Add allowInsecureConnections for HTTP NuGet sources
bhanurp 146a760
RTECO-1574 - Push .snupkg to symbolpackage endpoint
bhanurp 125e319
RTECO-1574 - Remove bhanurp fork replace for build-info-go
bhanurp e8c9a0b
RTECO-1574 - Resolve virtual repo to local for OriginalDeploymentRepo
bhanurp f510f97
RTECO-1574 - Bump build-info-go to fix snupkg Artifactory storage path
bhanurp cd31493
RTECO-1574 - Bump build-info-go to include unit test fixes
bhanurp 64f5567
RTECO-1574 - Fix stale comment about snupkg storage path
bhanurp d1305ca
RTECO-1574 - Bump build-info-go to fix .symbols.nupkg storage path
bhanurp e5f22eb
RTECO-1574 - Improve jf nuget/dotnet help to document build-info subc…
bhanurp e8f9a12
RTECO-1574 - Address review: dead code, XML escaping, credential inje…
bhanurp c4b5922
RTECO-1574 - Bump build-info-go; scope snapshot walk to output dirs
bhanurp 79273fb
RTECO-1574 - go mod tidy: remove unused indirect dependencies
bhanurp 511204e
RTECO-1574 - Retry post-push search for eventual consistency; fix Len…
bhanurp b2af3f6
RTECO-1574 - Fix restoreOptionTakesValue for dotnet build flags; add …
bhanurp cc2a8de
RTECO-1574 - Fix errcheck: handle Close() errors in pushSinglePackage
bhanurp 316a29f
Merge branch 'main' into RTECO-1574-nuget-flexpack-support
bhanurp cbbcf56
fix: address SAST findings in nuget push command
bhanurp 9691926
fix: address high-severity code review findings in NuGet FlexPack com…
bhanurp 63abfe6
fix: errcheck - wrap pw.Close and fmt.Fprint return values
bhanurp bcd5810
fix: use #nosec G402 to suppress gosec InsecureSkipVerify finding
bhanurp 8a3eebc
fix: resolve Frogbot SAST findings — remove InsecureSkipVerify and us…
bhanurp dff4a8b
fix: pass *url.URL to pushSinglePackage so SAST can confirm host is l…
bhanurp f102a18
chore: bump build-info-go to v1.13.1-0.20260827115645-671a9b8faf2e (c…
bhanurp fc82a93
chore: bump build-info-go to 4a2e4939ca36 (fix push-arg containment c…
bhanurp 2337d54
fix: lock push URL host via struct copy to address SAST SSRF finding
bhanurp 0a6305a
fix: drop always-nil error return from buildPushURLs (unparam lint)
bhanurp 7d03314
chore: merge upstream/main (Add --priority for RBv2); retidy go.sum
bhanurp a2b3261
fix: add explicit host allowlist check in pushSinglePackage (SAST SSRF)
bhanurp ed59bde
fix: break SAST taint chain by separating host from path in push request
bhanurp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| package nuget | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "net/url" | ||
|
|
||
| dotnetcmd "github.com/jfrog/jfrog-cli-artifactory/artifactory/commands/dotnet" | ||
| "github.com/jfrog/jfrog-cli-core/v2/utils/config" | ||
| ) | ||
|
|
||
| // SourceURLWithCredentials builds an Artifactory NuGet feed URL with credentials embedded | ||
| // as https://user:password@host/... so they can be passed directly as a -Source flag. | ||
| // This is rank-1 (command-line flag) in NuGet's credential priority hierarchy and requires | ||
| // no nuget.config modification. | ||
| func SourceURLWithCredentials(serverDetails *config.ServerDetails, repoName string, useV2 bool) (string, error) { | ||
| sourceURL, user, password, err := dotnetcmd.GetSourceDetails(serverDetails, repoName, useV2) | ||
| if err != nil { | ||
| return "", fmt.Errorf("get NuGet source details: %w", err) | ||
| } | ||
|
|
||
| u, err := url.Parse(sourceURL) | ||
| if err != nil { | ||
| return "", fmt.Errorf("parse NuGet source URL: %w", err) | ||
| } | ||
| u.User = url.UserPassword(user, password) | ||
| return u.String(), nil | ||
| } | ||
|
|
||
| // NuGetExeV3SourceDetails returns the V3 source URL, username, and password for use in a | ||
| // temp nuget.config for restore operations. V3 is safe here because the URL goes into | ||
| // <packageSources> in the config file, not as a -Source CLI flag. When no -Source flag is | ||
| // passed, nuget.exe does NOT re-embed the URL into MSBuild's /p:RestoreSources — MSBuild | ||
| // reads the source directly from the config file via /p:RestoreConfigFile and loads the V3 | ||
| // service index normally. | ||
| func NuGetExeV3SourceDetails(serverDetails *config.ServerDetails, repoName string) (sourceURL, user, password string, err error) { | ||
| return dotnetcmd.GetSourceDetails(serverDetails, repoName, false /* V3 */) | ||
| } | ||
|
|
||
| // NuGetExeV2SourceDetails returns the V2 source URL, username, and password for push | ||
| // operations. Push is handled by nuget.exe directly (not MSBuild), so named source lookup | ||
| // from -ConfigFile works. V2 is fine for push (no service index needed) and avoids the | ||
| // /p:RestoreSources re-embedding issue entirely. | ||
| func NuGetExeV2SourceDetails(serverDetails *config.ServerDetails, repoName string) (sourceURL, user, password string, err error) { | ||
| return dotnetcmd.GetSourceDetails(serverDetails, repoName, true /* V2 */) | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Credentials in the command line are visible to other local processes.
SourceURLWithCredentialsreturns a URL that contains the password or access token. The caller appends it toc.argsas--source <url>, so the secret becomes part of the child process argv. Any local user can read it withps, and process-tracing or CI job supervisors can capture it. ThedotnetCLI also echoes source URLs in NU1301/NU1801 diagnostics, so the token can reach build logs.Prefer the temp-config path (
injectCredentialsViaTempConfig) or theNuGetPackageSourceCredentials_<name>__Passwordenvironment variables for thedotnettoolchain as well. Both keep the secret out of argv.🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The credentials are injected as a
--sourceURL with Basic Auth embedded (not as separate--username/--passwordflags), which is the standard NuGet V2/V3 auth pattern. The URL is not written to disk and is only visible in the process table for the duration of the nuget.exe/dotnet invocation — same as any other CLI tool that accepts credentials via URL.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Length of output: 11749
@bhanurp, thanks for the clarification. The current code confirms that thedotnetrestore path appends the credential-bearing URL toc.argsas--source <url>.A short exposure duration and no disk write reduce the risk. They do not prevent credential disclosure through process inspection, CI process capture, or diagnostics that print the source URL. The risk applies even when the URL format is supported by NuGet.
The existing
injectCredentialsViaTempConfigpath shows that this integration already supports a non-argvcredential mechanism fornuget.exe. Thedotnetrestore path should use an equivalent protected mechanism, such as NuGet package-source credential environment variables, instead of embedding the password or token in--source.✏️ Learnings added
You are interacting with an AI system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acknowledged. The
dotnetrestore path currently embeds credentials in--source <url>, which is visible via process inspection. Switching toNuGetPackageSourceCredentials_*env vars (the non-argv mechanism) fordotnetis the correct long-term fix and will be tracked as a follow-up. Thenuget.exepath already uses a temp config file (injectCredentialsViaTempConfig) to avoid this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bhanurp, understood. The finding remains open for thedotnetrestore path until it usesNuGetPackageSourceCredentials_*or another non-argv credential mechanism.Would you like me to create a GitHub follow-up issue for this work?
🧠 Learnings used
You are interacting with an AI system.