Skip to content

Conversation

dwnusbaum
Copy link
Member

After jenkinsci/github-branch-source-plugin#822, credential lookups for GitHubAppCredentials configured to use an inference-based repository access strategy fail when using GitSCMFileSystem. Normally GitHubSCMFileSystem is used instead of GitSCMFileSystem, and that works fine, but GitSCMFileSystem gets used in cases where you have to configure a GitSCM directly, since there is no GitHub-specific SCM implementation.

For example, you can run into this problem if you use "Pipeline script from SCM" to configure a Pipeline, enable lightweight checkout, and use GitHubAppCredentials for the GitSCM credentials.

For more context, credential lookups for GitHubAppCredentials were expected to fall into one of two cases:

  1. Credentials used by Jenkins or a plugin to contact GitHub in a way that the secrets will not be exposed directly to Jenkins users are expected to use Connector.lookupScanCredentials. This ensures proper owner inference and bypasses repository inference because the credential usage context is trusted. Plugins that need to do this kind of lookup may require changes like Use Connector.lookupScanCredentials to contextualize GitHubAppCredentials github-checks-plugin#398 (but others were already using Connector.lookupScanCredentials).
  2. Credentials whose secrets may be exposed directly to Jenkins users via things like the withCredentials step. Plugins that need to do this kind of lookup need to use CredentialsProvider.findCredentialById and pass an appropriate Run context for proper owner and repository inference, like is already done in this plugin in GitSCM.lookupScanCredentials.

GitSCMFileSystem is a bit of an awkward spot. Conceptually, it falls under case 1 and should use Connector.lookupScanCredentials to contextualize GitHubAppCredentials for a trusted context, but we can't add a github-branch-source dependency here or else we'll have circular dependencies. This leaves us with two options:

  • Some kind of API change upstream:
    • Perhaps a method in in credentials that is comparable to Credentials.forRun that allows contextualization of generic credential lookups, and avoids the need to use Connector.lookupScanCredentials directly when working with GitHubAppCredentials
    • Some way to (optionally?) use GitHubSCMFileSystem with GitSCM, or a GitHub-specific SCM implementation or similar that bypasses GitSCMFileSystem in this case
  • Something like the change in this PR, where run-based contextualization is used to allow some owner inference strategies to work with GitSCMFileSystem, although repository inference strategies would still not be supported

CC @jeromepochat

Testing done

See new automated test.

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests that demonstrate the feature works or the issue is fixed

@github-actions github-actions bot added the tests Automated test addition or improvement label Sep 2, 2025
Comment on lines +384 to +386
if (_build != null && credential != null && credential.forRun(_build) instanceof StandardCredentials standardCredential) {
credential = standardCredential;
}

Choose a reason for hiding this comment

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

question: No sure if GitSCMTelescope should contextualize the credentials similarly?

Copy link
Member Author

@dwnusbaum dwnusbaum Sep 3, 2025

Choose a reason for hiding this comment

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

Yeah I looked into that, but IDK. There is also this credentials lookup in AbstractGitSCMSource which won't work, and I don't think it can be made to work:

return CredentialsMatchers
.firstOrNull(
CredentialsProvider.lookupCredentialsInItem(StandardUsernameCredentials.class, context,
ACL.SYSTEM2, URIRequirementBuilder.fromUri(getRemote()).build()),
CredentialsMatchers.allOf(CredentialsMatchers.withId(credentialsId),
GitClient.CREDENTIALS_MATCHER));

Going by https://github.com/search?type=code&q=+owner%3Ajenkinsci+gitscmtelescope, I think the answer here would just be that GitSCMSource doesn't support inference-based options for GitHubAppCredentials, you must use GitHubSCMSource. IDK if there would ever be a reason that you would have to use GitSCMSource over GitHubSCMSource, but I don't think so.

For GitSCM and its use of GitSCMFileSystem, the situation is different, since there is no GitHubSCM.

}

@Test
public void filesystem_supports_credential_contextualization() throws Exception {

Choose a reason for hiding this comment

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

note: I confirm that the test fails without the credentials contextualization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Automated test addition or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants