Fix wrong masking of one-char-secrets in Jenkins output

[gallardo]

The fix makes sure that the Base64 secret patterns are not empty.

See https://issues.jenkins.io/browse/JENKINS-72412

Testing done

The first of these commits include unit tests to demonstrate this issue.

### Submitter checklist
- [X] Make sure you are opening from a **topic/feature/bugfix branch** (right side) and not your main branch!
- [X] Ensure that the pull request title represents the desired changelog entry
- [X] Please describe what you did
- [X] Link to relevant issues in GitHub or Jira
- [ ] Link to relevant pull requests, esp. upstream and downstream changes
- [X] Ensure you have provided tests - that demonstrates feature works or fixes the issue

[jglick]

amending #269 FTR

[jglick]

Probably fine so far as it goes but I suspect what you really wanted was

diff --git src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java
index c4f45cf..f062de5 100644
--- src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java
+++ src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java
@@ -323,6 +323,11 @@ public class GitHubAppCredentials extends BaseStandardCredentials implements Sta
         return appID;
     }
 
+    @Override
+    public boolean isUsernameSecret() {
+        return false;
+    }
+
     @NonNull
     public synchronized GitHubAppCredentials withOwner(@NonNull String owner) {
         if (this.owner != null) {

Care to test & file ^^^?

[gallardo]

I suspect what you really wanted was

+    @Override
+    public boolean isUsernameSecret() {
+        return false;
+    }

I'm new to Jenkins code, but if I interpret this correctly, this is absolutely what we want: https://issues.jenkins.io/browse/JENKINS-66675

[gallardo]

Care to test & file ^^^?

@jglick Tested :heavy_check_mark:

Being you the author of the fix, it'd be more appropriate if you file the PR yourself. There is the already mentioned issue https://issues.jenkins.io/browse/JENKINS-66675 reporting on this. I have edited it to reflect that the component github-branch-source-plugin is also affected.

[schiasileon]

@jglick is there any update on this? this is messing up a few of our build logs

[jglick]

@schiasileon do you have some one-character passwords?

…is it x?

[schiasileon]

@schiasileon do you have some one-character passwords?

…is it x?

This mostly concerns us as we use a custom implementation that allows for multiline secrets from vault where we have some troubles with single character lines or empty lines being treated as separate secrets. Not a huge issue, but a fix would be nice. Also on some instances we have empty secrets due to our CasC setup that cause this issue

[jglick]

a custom implementation

Of what?

multiline secrets from vault where we have some troubles with single character lines or empty lines being treated as separate secrets

Secret masking does not support multiline secrets. You have to use a secret file type. Perhaps this plugin should be improved to at least print a warning if a secret includes a line terminator, since at least its raw form cannot possibly match. (An input with a newline could still be masked in its Base64 form, but you are still in danger of accidentally printing it without Base64 encoding.)

we have empty secrets due to our CasC setup

AFAICT empty “secrets” are already ignored https://github.com/jenkinsci/credentials-binding-plugin/blob/89bbe4e9df9fd171fc63018dcefceaad4648b7ba/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatterns.java#L60 but perhaps that only applies to the actual secret as opposed to the encoded form? I think the more general fix here is to just move the filter down a line (or duplicate it) since it is the encoded form, rather than the original “secret” per se, which cannot be empty in the final regexp. maskingOfOneCharSecretShouldNotMangleOutput should still cover the change.

[realies]

@jglick, all output inside withCredentials is masked. This is hugely inconvenient to debug. Could someone please take a look at this?