feat: apply netrc/env credentials to Maven repository authentication - #2583
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/test/java/dev/jbang/dependencies/TestNetrcMavenAuth.java`:
- Around line 127-135: The `testGetCredentialsForHostReturnsCredentials` test is
flaky because `NetUtil.getCredentialsForHost(...)` will prefer `GITHUB_TOKEN`
for hosts recognized by `isAGithubHost(...)`, including `maven.pkg.github.com`.
Add the same environment guard used by the other tests (or switch the test to a
non-GitHub host) so the `.netrc` path is exercised consistently and the
`assertThat(creds[0]...)` / `assertThat(creds[1]...)` checks remain stable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b6df2b24-d97c-47c9-82ea-cfd7d11041dd
📒 Files selected for processing (4)
src/main/java/dev/jbang/dependencies/ArtifactResolver.javasrc/main/java/dev/jbang/util/NetUtil.javasrc/test/java/dev/jbang/dependencies/TestNetrcMavenAuth.javasrc/test/java/dev/jbang/util/TestNetrcAuth.java
|
@quintesse i think this works but good with extra set of eyes before release (would like it in as it makes netrc / gh-creds feature complete) |
daba49b to
bc2690c
Compare
|
It all looks fine. But can you give me an actual example of how to use the GH credentials that I can run? |
is the example in the docs sufficient or need something more? |
|
note, you probably don't even need the login key here as gh-auth is pure token/pwd based. |
Well, the thing is that example isn't immediately usable. I can't copy & paste it to see if it works. So I wondered if there was perhaps a more "public" example that anyone could use. |
|
Try jbang --java 21 --fresh --repos 'quarkus-github-action=https://maven.pkg.github.com/quarkusio/conversational-release-action/' --repos 'mavencentral' io.quarkus.bot:conversational-release-action:999-SNAPSHOT |
|
Doesn't seem to work? With the final cause: |
|
Did you create a .netrc / _netrc file in your home directory? What does --verbose tell you? |
|
The verbose tells me the last line in my previous message. And I don't know what to put in that netrc file, you didn't mention that when you said "Try ..." :-) |
|
machine maven.pkg.github.com |
|
Same result and I don't see anything special in the verbose output (same unauthorized exception) |
|
are you sure you are running with the build ? :) it should print info about looking for .netrc etc? |
5fda622 to
3743764
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends JBang’s authentication system so Maven repository resolution (Aether/MIMA) can reuse the same credential sources already used for HTTP downloads, including .netrc, env tokens, and optional delegation to external credential helpers.
Changes:
- Introduces shared host-based credential lookup via
NetUtil.getCredentialsForHost()and applies it to MavenRemoteRepositoryauthentication. - Adds
.netrcextensions (jbang-auth,jbang-auth-host,jbang-auth-scheme) and wiring for external helpers (git credential fill,gh auth token,glab config get token) plus a--no-netrcflag. - Expands tests (including WireMock integration tests) and updates documentation to describe the updated credential precedence and new helper options.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/dev/jbang/util/NetUtil.java | Adds shared credential lookup chain, helper delegation, and .netrc disable support; updates HTTP auth header selection. |
| src/main/java/dev/jbang/util/NetrcParser.java | Extends .netrc parsing for jbang-auth* keys and enforces safe POSIX permissions. |
| src/main/java/dev/jbang/util/Util.java | Extends command runner to support stdin/env and introduces a quiet/timeout variant for helper calls. |
| src/main/java/dev/jbang/dependencies/ArtifactResolver.java | Applies NetUtil.getCredentialsForHost() to Maven RemoteRepository auth baseline. |
| src/main/java/dev/jbang/cli/BaseCommand.java | Adds --no-netrc option and integrates it into CLI parsing. |
| src/test/java/dev/jbang/util/TestNetrcParser.java | Adds tests for unsafe permissions, default path, and new .netrc jbang-auth* keys. |
| src/test/java/dev/jbang/util/TestNetrcAuth.java | Adds tests for disabling netrc, bearer scheme, and updates env-skip condition. |
| src/test/java/dev/jbang/util/TestDescribeAuthMethod.java | Adds tests ensuring describeAuthMethod() reports git-credential selection correctly. |
| src/test/java/dev/jbang/dependencies/TestNetrcMavenAuth.java | New WireMock integration suite verifying Maven resolver sends expected Authorization headers with various credential sources. |
| docs/modules/ROOT/pages/dependencies.adoc | Documents that Maven auth can come from settings.xml, .netrc, env tokens, and helpers (via HTTP auth docs). |
| docs/modules/ROOT/pages/configuration.adoc | Updates HTTP/Maven auth precedence docs and adds detailed .netrc helper documentation + --no-netrc. |
| AGENTS.md | Captures the intended credential precedence and shared-lookup guidance for contributors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Returns the value of the JBang-specific {@code jbang-auth} key, or | ||
| * {@code null} if not present. The value is a comma-separated list of auth | ||
| * methods tried in order. Recognised methods: | ||
| * <ul> | ||
| * <li>{@code git-credential} — delegate to {@code git credential fill}</li> | ||
| * <li>{@code gh-auth} — delegate to {@code gh auth token}</li> | ||
| * <li>{@code glab-auth} — delegate to {@code glab config get token}</li> | ||
| * <li>{@code env.NAME} — read the {@code NAME} environment variable</li> | ||
| * <li>{@code jbang-auth-scheme} — HTTP auth scheme: basic or bearer</li> | ||
| * </ul> | ||
| */ |
| /** | ||
| * Returns the default netrc file path for the current platform. | ||
| * {@code ~/.netrc} on Unix/macOS, {@code ~/_netrc} on Windows. | ||
| * Returns the default netrc file path: {@code ~/.netrc}. | ||
| */ | ||
| public static Path getDefaultNetrcPath() { | ||
| Path home = Paths.get(System.getProperty("user.home")); | ||
| if (Util.isWindows()) { | ||
| return home.resolve("_netrc"); | ||
| } | ||
| return home.resolve(".netrc"); | ||
| return Paths.get(System.getProperty("user.home")).resolve(".netrc"); | ||
| } |
| org.junit.jupiter.api.Assumptions.assumeTrue( | ||
| System.getenv(NetUtil.JBANG_AUTH_BASIC_USERNAME) == null | ||
| || System.getenv(NetUtil.JBANG_AUTH_BASIC_PASSWORD) == null, | ||
| && System.getenv(NetUtil.JBANG_AUTH_BASIC_PASSWORD) == null, | ||
| "Skipping: JBANG_AUTH_BASIC_* env vars are set"); |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
src/main/java/dev/jbang/util/Util.java (1)
960-983: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnotate the nullable subprocess API.
The return value,
stdin, andenvexplicitly allownull; expose that contract with JSpecify annotations.As per coding guidelines, use
@jspecifynullability annotations where applicable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/dev/jbang/util/Util.java` around lines 960 - 983, Annotate the subprocess API in Util with JSpecify nullability annotations: mark the return value of runCommand overloads and the stdin and env parameters as nullable, including the private runCommand implementation where applicable. Preserve the existing non-null contract for command arguments and other parameters.Source: Coding guidelines
src/main/java/dev/jbang/util/NetrcParser.java (1)
51-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnotate the new nullable credential fields.
These values explicitly permit
null; annotate the fields, constructor parameters, and public return types with@Nullable.As per coding guidelines, use
@jspecifynullability annotations where applicable.Also applies to: 91-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/dev/jbang/util/NetrcParser.java` around lines 51 - 63, Annotate the nullable credential members in NetrcEntry with the project’s `@jspecify` `@Nullable` annotation, including the corresponding constructor parameters and public return types. Apply this consistently to jbangAuth, jbangAuthHost, jbangAuthScheme, and any other newly nullable credential fields covered by the same constructor/accessors.Source: Coding guidelines
src/main/java/dev/jbang/util/NetUtil.java (1)
803-860: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnotate the nullable credential result.
getCredentialsForHostpublicly documents a nullable return but does not express it through JSpecify annotations.As per coding guidelines, use
@jspecifynullability annotations where applicable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/dev/jbang/util/NetUtil.java` around lines 803 - 860, Annotate the nullable return of getCredentialsForHost with the project’s JSpecify nullable annotation, matching the existing import and annotation conventions. Keep the method’s documented two-element credential array and null fallback behavior unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/modules/ROOT/pages/configuration.adoc`:
- Around line 193-195: Update the NOTE in the configuration documentation to
list `jbang-auth-scheme` alongside `jbang-auth` and `jbang-auth-host`, covering
all three JBang-specific `.netrc` extensions.
In `@src/main/java/dev/jbang/util/NetrcParser.java`:
- Around line 79-90: Update the documentation for the jbang-auth key in the
method that lists recognized authentication methods by removing
jbang-auth-scheme from that list; keep the separate HTTP auth scheme
configuration undocumented here and leave the other entries unchanged.
In `@src/main/java/dev/jbang/util/NetUtil.java`:
- Around line 697-727: Separate exact and default .netrc lookups in the HTTP
authentication flow around getNetrc().getEntry(host), and apply the same change
to the Maven and description lookup paths around the referenced branches.
Preserve the exact-entry attempt, but fall back to the independently retrieved
default entry when credential extraction fails; add a regression unit test
covering a failing exact helper with valid default credentials, plus an
integration test if the existing test structure supports it.
- Around line 593-650: Update getGitCredentials, getGhAuthCredentials, and
getGlabAuthCredentials so failed or missing helper results are cached per host
instead of returning null from computeIfAbsent. Use an Optional or existing
failure sentinel in gitCredentialCache, then unwrap it at each method boundary
while preserving the current successful credential values and per-call username
handling.
In `@src/main/java/dev/jbang/util/Util.java`:
- Around line 960-975: Update the public runCommand(String stdin, Map<String,
String> env, String... cmd) overload and its runCommand(String... cmd) delegate
so they no longer pass an unbounded timeout of 0; use the established finite
default timeout or require callers to invoke the timeout-aware overload, while
preserving existing command execution behavior.
- Around line 997-1011: Update the timed execution branch in the
process-handling method around BufferedReader br and p.waitFor so stdout is
drained concurrently before waiting for process completion. Preserve the timeout
behavior, but ensure output consumption continues while the child runs,
preventing a full merged output pipe from causing a false timeout.
---
Nitpick comments:
In `@src/main/java/dev/jbang/util/NetrcParser.java`:
- Around line 51-63: Annotate the nullable credential members in NetrcEntry with
the project’s `@jspecify` `@Nullable` annotation, including the corresponding
constructor parameters and public return types. Apply this consistently to
jbangAuth, jbangAuthHost, jbangAuthScheme, and any other newly nullable
credential fields covered by the same constructor/accessors.
In `@src/main/java/dev/jbang/util/NetUtil.java`:
- Around line 803-860: Annotate the nullable return of getCredentialsForHost
with the project’s JSpecify nullable annotation, matching the existing import
and annotation conventions. Keep the method’s documented two-element credential
array and null fallback behavior unchanged.
In `@src/main/java/dev/jbang/util/Util.java`:
- Around line 960-983: Annotate the subprocess API in Util with JSpecify
nullability annotations: mark the return value of runCommand overloads and the
stdin and env parameters as nullable, including the private runCommand
implementation where applicable. Preserve the existing non-null contract for
command arguments and other parameters.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 25be52ba-61d3-4537-ad43-6f8dcebcba8d
📒 Files selected for processing (12)
AGENTS.mddocs/modules/ROOT/pages/configuration.adocdocs/modules/ROOT/pages/dependencies.adocsrc/main/java/dev/jbang/cli/BaseCommand.javasrc/main/java/dev/jbang/dependencies/ArtifactResolver.javasrc/main/java/dev/jbang/util/NetUtil.javasrc/main/java/dev/jbang/util/NetrcParser.javasrc/main/java/dev/jbang/util/Util.javasrc/test/java/dev/jbang/dependencies/TestNetrcMavenAuth.javasrc/test/java/dev/jbang/util/TestDescribeAuthMethod.javasrc/test/java/dev/jbang/util/TestNetrcAuth.javasrc/test/java/dev/jbang/util/TestNetrcParser.java
✅ Files skipped from review due to trivial changes (1)
- docs/modules/ROOT/pages/dependencies.adoc
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/dev/jbang/dependencies/ArtifactResolver.java
| Credential results are cached per-host for the lifetime of the JBang process, so the subprocess is only invoked once per host. | ||
|
|
||
| NOTE: `jbang-auth` and `jbang-auth-host` are JBang-specific extensions to the `.netrc` format. They are silently ignored by `curl`, `wget`, `git`, and other tools that read `.netrc`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Include jbang-auth-scheme in the extension note.
The documentation defines three JBang-specific keys—jbang-auth, jbang-auth-host, and jbang-auth-scheme—but the note lists only the first two. Update it to cover all three.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/modules/ROOT/pages/configuration.adoc` around lines 193 - 195, Update
the NOTE in the configuration documentation to list `jbang-auth-scheme`
alongside `jbang-auth` and `jbang-auth-host`, covering all three JBang-specific
`.netrc` extensions.
| /** | ||
| * Returns the value of the JBang-specific {@code jbang-auth} key, or | ||
| * {@code null} if not present. The value is a comma-separated list of auth | ||
| * methods tried in order. Recognised methods: | ||
| * <ul> | ||
| * <li>{@code git-credential} — delegate to {@code git credential fill}</li> | ||
| * <li>{@code gh-auth} — delegate to {@code gh auth token}</li> | ||
| * <li>{@code glab-auth} — delegate to {@code glab config get token}</li> | ||
| * <li>{@code env.NAME} — read the {@code NAME} environment variable</li> | ||
| * <li>{@code jbang-auth-scheme} — HTTP auth scheme: basic or bearer</li> | ||
| * </ul> | ||
| */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Do not list jbang-auth-scheme as an authentication method.
runAuthMethod treats this value as unknown because the scheme is configured through a separate netrc key.
Proposed documentation fix
- * <li>{`@code` jbang-auth-scheme} — HTTP auth scheme: basic or bearer</li>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Returns the value of the JBang-specific {@code jbang-auth} key, or | |
| * {@code null} if not present. The value is a comma-separated list of auth | |
| * methods tried in order. Recognised methods: | |
| * <ul> | |
| * <li>{@code git-credential} — delegate to {@code git credential fill}</li> | |
| * <li>{@code gh-auth} — delegate to {@code gh auth token}</li> | |
| * <li>{@code glab-auth} — delegate to {@code glab config get token}</li> | |
| * <li>{@code env.NAME} — read the {@code NAME} environment variable</li> | |
| * <li>{@code jbang-auth-scheme} — HTTP auth scheme: basic or bearer</li> | |
| * </ul> | |
| */ | |
| /** | |
| * Returns the value of the JBang-specific {`@code` jbang-auth} key, or | |
| * {`@code` null} if not present. The value is a comma-separated list of auth | |
| * methods tried in order. Recognised methods: | |
| * <ul> | |
| * <li>{`@code` git-credential} — delegate to {`@code` git credential fill}</li> | |
| * <li>{`@code` gh-auth} — delegate to {`@code` gh auth token}</li> | |
| * <li>{`@code` glab-auth} — delegate to {`@code` glab config get token}</li> | |
| * <li>{`@code` env.NAME} — read the {`@code` NAME} environment variable</li> | |
| * </ul> | |
| */ |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/dev/jbang/util/NetrcParser.java` around lines 79 - 90, Update
the documentation for the jbang-auth key in the method that lists recognized
authentication methods by removing jbang-auth-scheme from that list; keep the
separate HTTP auth scheme configuration undocumented here and leave the other
entries unchanged.
| /** Runs {@code git credential fill} for the given hostname. Cached per-host. */ | ||
| static String[] getGitCredentials(String host) { | ||
| return gitCredentialCache.computeIfAbsent(host, h -> { | ||
| String output = Util.runCommandQuietly( | ||
| "protocol=https\nhost=" + h + "\n\n", | ||
| Collections.singletonMap("GIT_TERMINAL_PROMPT", "0"), | ||
| AUTH_HELPER_TIMEOUT_SECONDS, | ||
| "git", "credential", "fill"); | ||
| if (output != null) { | ||
| String username = null, password = null; | ||
| for (String line : output.split("\n")) { | ||
| if (line.startsWith("username=")) | ||
| username = line.substring(9); | ||
| else if (line.startsWith("password=")) | ||
| password = line.substring(9); | ||
| } | ||
| if (!isNullOrBlankString(username) && !isNullOrBlankString(password)) { | ||
| verboseMsg("Using git credential for host: " + h); | ||
| return new String[] { username, password }; | ||
| } | ||
| } | ||
| return null; | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Runs {@code gh auth token --hostname <host>} via {@link Util#runCommand}. | ||
| * Token cached per-host; username applied per-call from the netrc entry's login | ||
| * field. | ||
| */ | ||
| static String[] getGhAuthCredentials(String host, String login) { | ||
| String[] cached = gitCredentialCache.computeIfAbsent("gh-auth:" + host, k -> { | ||
| String token = Util.runCommandQuietly(null, null, AUTH_HELPER_TIMEOUT_SECONDS, | ||
| "gh", "auth", "token", "--hostname", host); | ||
| if (!isNullOrBlankString(token)) { | ||
| verboseMsg("Using gh auth token for host: " + host); | ||
| return new String[] { "", token.trim() }; | ||
| } | ||
| return null; | ||
| }); | ||
| return withUsername(cached, login, ""); | ||
| } | ||
|
|
||
| /** | ||
| * Runs {@code glab config get token --host <host>} via {@link Util#runCommand}. | ||
| */ | ||
| static String[] getGlabAuthCredentials(String host, String login) { | ||
| String[] cached = gitCredentialCache.computeIfAbsent("glab-auth:" + host, k -> { | ||
| String token = Util.runCommandQuietly(null, null, AUTH_HELPER_TIMEOUT_SECONDS, | ||
| "glab", "config", "get", "token", "--host", host); | ||
| if (!isNullOrBlankString(token)) { | ||
| verboseMsg("Using glab auth token for host: " + host); | ||
| return new String[] { "", token.trim() }; | ||
| } | ||
| return null; | ||
| }); | ||
| return withUsername(cached, login, "__token__"); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Cache failed credential-helper lookups too.
computeIfAbsent does not store mappings when the callback returns null, so missing or failing helpers rerun on every lookup and can repeatedly incur the ten-second timeout. Cache an Optional or failure sentinel.
As per coding guidelines, external process results must be cached per host for the process lifetime.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/dev/jbang/util/NetUtil.java` around lines 593 - 650, Update
getGitCredentials, getGhAuthCredentials, and getGlabAuthCredentials so failed or
missing helper results are cached per host instead of returning null from
computeIfAbsent. Use an Optional or existing failure sentinel in
gitCredentialCache, then unwrap it at each method boundary while preserving the
current successful credential values and per-call username handling.
Source: Coding guidelines
| // 2. .netrc exact host match (more specific than env vars) | ||
| if (auth == null) { | ||
| URL url = urlConnection.getURL(); | ||
| if (url.getUserInfo() != null) { | ||
| String[] credentials = url.getUserInfo().split(":", 2); | ||
| String username = PropertiesValueResolver.replaceProperties(credentials[0]); | ||
| String password = credentials.length > 1 ? PropertiesValueResolver.replaceProperties(credentials[1]) | ||
| : ""; | ||
| String id = username + ":" + password; | ||
| String encodedId = Base64.getEncoder().encodeToString(id.getBytes(StandardCharsets.UTF_8)); | ||
| auth = "Basic " + encodedId; | ||
| verboseMsg("Using URL credentials for host: " + host); | ||
| entry = getNetrc().getEntry(host).orElse(null); | ||
| if (entry != null && !entry.isDefault()) { | ||
| String[] creds = extractCredsFromEntry(entry, host); | ||
| if (creds != null) { | ||
| auth = toHttpAuth(entry, creds); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // 3. Check .netrc / _netrc file | ||
| // 3. GitHub/GitLab env vars use Bearer auth for HTTP downloads | ||
| if (auth == null) { | ||
| NetrcParser.NetrcEntry entry = getNetrc().getEntry(host).orElse(null); | ||
| if (entry != null && !isNullOrBlankString(entry.getLogin()) && !isNullOrBlankString(entry.getPassword())) { | ||
| String login = entry.getLogin(); | ||
| String id = login + ":" + entry.getPassword(); | ||
| String encodedId = Base64.getEncoder().encodeToString(id.getBytes(StandardCharsets.UTF_8)); | ||
| auth = "Basic " + encodedId; | ||
| verboseMsg("Using .netrc credentials for host: " + host); | ||
| String githubToken = System.getenv("GITHUB_TOKEN"); | ||
| String gitlabToken = System.getenv("GITLAB_TOKEN"); | ||
| if (isAGithubUrl(urlConnection) && !isNullOrBlankString(githubToken)) { | ||
| auth = AuthHeader.authorization("Bearer " + githubToken); | ||
| verboseMsg("Using GITHUB_TOKEN environment variable for host: " + host); | ||
| } else if (isAGitlabUrl(urlConnection) && !isNullOrBlankString(gitlabToken)) { | ||
| auth = AuthHeader.authorization("Bearer " + gitlabToken); | ||
| verboseMsg("Using GITLAB_TOKEN environment variable for host: " + host); | ||
| } | ||
| } | ||
|
|
||
| // 4. Fall back to global basic auth env vars | ||
| // 4. .netrc default entry (less specific than env vars) | ||
| if (auth == null && entry != null && entry.isDefault()) { | ||
| String[] creds = extractCredsFromEntry(entry, host); | ||
| if (creds != null) { | ||
| auth = toHttpAuth(entry, creds); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the default netrc entry reachable after an exact entry fails.
getEntry(host) returns either the exact entry or the default. When an exact helper yields no credentials, entry remains exact, so the later default branch is always skipped. Retrieve exact and default entries separately across HTTP, Maven, and description lookup.
Add a regression test with a failing exact helper and a valid default entry. Based on learnings, always add unit tests, and integration tests when relevant, for bug fixes.
Also applies to: 821-848, 920-942
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/dev/jbang/util/NetUtil.java` around lines 697 - 727, Separate
exact and default .netrc lookups in the HTTP authentication flow around
getNetrc().getEntry(host), and apply the same change to the Maven and
description lookup paths around the referenced branches. Preserve the
exact-entry attempt, but fall back to the independently retrieved default entry
when credential extraction fails; add a regression unit test covering a failing
exact helper with valid default credentials, plus an integration test if the
existing test structure supports it.
Sources: Coding guidelines, Learnings
| public static String runCommand(String... cmd) { | ||
| return runCommand(null, null, cmd); | ||
| } | ||
|
|
||
| /** | ||
| * Runs a command and returns its stdout, or {@code null} on failure. | ||
| * | ||
| * @param stdin Content to write to the process's stdin, or {@code null} | ||
| * @param env Extra environment variables to add to the process, or | ||
| * {@code null} | ||
| * @param cmd The command and arguments to execute | ||
| * @return The stdout output of the command or {@code null} if the command | ||
| * failed or could not be run | ||
| */ | ||
| public static String runCommand(String stdin, Map<String, String> env, String... cmd) { | ||
| return runCommand(stdin, env, 0, true, cmd); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not retain an unbounded default timeout.
Line 975 passes 0, so either public overload can block forever when a subprocess hangs. Use a finite default or require callers to provide one.
As per coding guidelines, external process calls must set a timeout.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/dev/jbang/util/Util.java` around lines 960 - 975, Update the
public runCommand(String stdin, Map<String, String> env, String... cmd) overload
and its runCommand(String... cmd) delegate so they no longer pass an unbounded
timeout of 0; use the established finite default timeout or require callers to
invoke the timeout-aware overload, while preserving existing command execution
behavior.
Source: Coding guidelines
| BufferedReader br = new BufferedReader(new InputStreamReader(p.getInputStream())); | ||
| String cmdOutput = br.lines().collect(Collectors.joining("\n")); | ||
| int exitCode = p.waitFor(); | ||
| String cmdOutput; | ||
| int exitCode; | ||
| if (timeoutSeconds <= 0) { | ||
| cmdOutput = br.lines().collect(Collectors.joining("\n")); | ||
| exitCode = p.waitFor(); | ||
| } else { | ||
| boolean finished = p.waitFor(timeoutSeconds, TimeUnit.SECONDS); | ||
| if (!finished) { | ||
| p.destroyForcibly(); | ||
| verboseMsg("Command timed out: " + String.join(" ", cmd)); | ||
| return null; | ||
| } | ||
| cmdOutput = br.lines().collect(Collectors.joining("\n")); | ||
| exitCode = p.exitValue(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Drain process output while waiting for completion.
The timed branch calls waitFor before reading stdout. A child filling the merged output pipe will block and be incorrectly killed as timed out. Start output consumption concurrently before waiting.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/dev/jbang/util/Util.java` around lines 997 - 1011, Update the
timed execution branch in the process-handling method around BufferedReader br
and p.waitFor so stdout is drained concurrently before waiting for process
completion. Preserve the timeout behavior, but ensure output consumption
continues while the child runs, preventing a full merged output pipe from
causing a false timeout.
139ab7b to
68831c9
Compare
Maven's Aether resolver does not honor .netrc files, so repositories like GitHub Packages that require authentication failed to resolve. Adds NetUtil.getCredentialsForHost() — a shared credential lookup used by both HTTP downloads and Maven resolution: 1. .netrc exact host match (most specific) 2. GITHUB_TOKEN / GITLAB_TOKEN env vars 3. .netrc default entry 4. JBANG_AUTH_BASIC_* env vars For Maven repos, settings.xml <server> entries take precedence over all of the above when the server <id> matches the repository ID. Adds jbang-auth keys for .netrc credential helper delegation: machine maven.pkg.github.com login myuser jbang-auth gh-auth,git-credential jbang-auth-host github.com Supported helpers: - gh-auth: runs 'gh auth token --hostname <host>' - glab-auth: runs 'glab config get token --host <host>' - git-credential: runs 'git credential fill' with prompting disabled - env.PASSWORD / env.USERNAME-PASSWORD: reads credentials from env vars Adds jbang-auth-scheme for direct HTTP downloads, supporting basic (default) and bearer. Maven authentication remains username/password credentials as expected by the resolver. Also adds --no-netrc, uses ~/.netrc consistently, ignores unsafe POSIX .netrc permissions, caches successful helper results per host, and runs external helpers with a timeout without logging helper output on failure. Adds WireMock and unit coverage for credential precedence, Maven Authorization headers, helper fallbacks, auth schemes, and netrc parsing.
68831c9 to
22f1ec0
Compare
Problem
jbang --repos 'name=https://maven.pkg.github.com/...' artifact:id:versionfails to resolve because Maven's Aether resolver does not honor.netrcfiles. JBang already had netrc support for HTTP downloads, but the Maven artifact resolver createdRemoteRepositoryobjects without any authentication.Continuation of #2565.
Solution
Credential lookup chain (most-specific first)
The auth chain has been reordered so explicit per-host config wins over generic env vars:
https://user:pass@host/...).netrcexact host matchGITHUB_TOKEN/GITLAB_TOKENenv vars.netrcdefaultentryJBANG_AUTH_BASIC_*env varsFor Maven repositories,
~/.m2/settings.xml<server>entries take precedence over all of the above when the server<id>matches the repository ID.Shared credential lookup
Extracted
NetUtil.getCredentialsForHost(String host)— returnsString[] {username, password}or null. Both the HTTP download path and Maven resolver share this method, eliminating duplicated lookup logic.Maven repository auth
ArtifactResolver.toRemoteRepo()callsgetCredentialsForHost()for each repository URL's hostname and attaches AetherAuthenticationwhen credentials are found.Git credential helper support (closes #2570)
Adds a JBang-specific
jbangkey to.netrcentries:When
getCredentialsForHost()encountersjbang git-credential, it runsgit credential fillto obtain credentials from whatever backend the user has configured (macOS Keychain, Windows Credential Manager, credential-store, etc.).jbangkey trigger the subprocesslogin/passwordin the same entry if git credential failsTests
WireMock integration tests verify actual
Authorizationheaders on HTTP requests:.netrccredentials sent after 401 challenge-responsesettings.xml<server>credentials sent to matching reposettings.xmltakes precedence over.netrcfor same repo ID.netrcused as fallback whensettings.xmlserver ID doesn't matchjbang git-credentialdelegates togit credential filllogin/passwordSummary by CodeRabbit
New Features
.netrc, environment tokens, credential helpers, and Maven settings.--no-netrcoption to disable.netrclookup for individual runs..netrcfiles with unsafe permissions.Bug Fixes
Documentation
.netrcconfiguration guidance.