From b25a337b0de33ae4eb59e7bbd6aaccc1897e3e23 Mon Sep 17 00:00:00 2001 From: Jason Heithoff Date: Mon, 25 Aug 2025 16:04:56 -0400 Subject: [PATCH 1/6] feat(JENKINS-73851): Add SHA-256 HMAC webhook signature validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add SignatureAlgorithm enum with SHA-256 as default and SHA-1 for legacy support - Extend GHWebhookSignature class to support SHA-256 HMAC computation - Update HookSecretConfig to include configurable signature algorithm - Modify RequirePostWithGHHookPayload.Processor to use configured algorithm - Add comprehensive unit tests for SHA-256 functionality - Maintain backwards compatibility with existing SHA-1 configurations - Log deprecation warnings when SHA-1 is used This implements GitHub's recommended SHA-256 HMAC signature validation while maintaining backwards compatibility through configuration. SHA-256 becomes the default for enhanced security. Resolves: JENKINS-73851 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- .vscode/settings.json | 3 + .../github/config/HookSecretConfig.java | 35 +++++++ .../github/webhook/GHWebhookSignature.java | 65 ++++++++++++- .../webhook/RequirePostWithGHHookPayload.java | 85 +++++++++++++---- .../github/webhook/SignatureAlgorithm.java | 66 +++++++++++++ .../config/HookSecretConfigSHA256Test.java | 47 ++++++++++ .../webhook/GHWebhookSignatureSHA256Test.java | 92 +++++++++++++++++++ .../webhook/SignatureAlgorithmTest.java | 40 ++++++++ 8 files changed, 411 insertions(+), 22 deletions(-) create mode 100644 .vscode/settings.json create mode 100644 src/main/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithm.java create mode 100644 src/test/java/org/jenkinsci/plugins/github/config/HookSecretConfigSHA256Test.java create mode 100644 src/test/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignatureSHA256Test.java create mode 100644 src/test/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithmTest.java diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 000000000..7b016a89f --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "java.compile.nullAnalysis.mode": "automatic" +} \ No newline at end of file diff --git a/src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java b/src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java index 248348907..b67b039a2 100644 --- a/src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java +++ b/src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java @@ -12,6 +12,7 @@ import hudson.util.ListBoxModel; import hudson.util.Secret; import jenkins.model.Jenkins; +import org.jenkinsci.plugins.github.webhook.SignatureAlgorithm; import org.jenkinsci.plugins.plaincredentials.StringCredentials; import org.kohsuke.stapler.DataBoundConstructor; @@ -25,10 +26,23 @@ public class HookSecretConfig extends AbstractDescribableImpl { private String credentialsId; + private SignatureAlgorithm signatureAlgorithm; @DataBoundConstructor public HookSecretConfig(String credentialsId) { + this(credentialsId, null); + } + + /** + * Constructor with signature algorithm specification. + * + * @param credentialsId the credentials ID for the webhook secret + * @param signatureAlgorithm the signature algorithm to use (defaults to SHA-256) + * @since 1.45.0 + */ + public HookSecretConfig(String credentialsId, SignatureAlgorithm signatureAlgorithm) { this.credentialsId = credentialsId; + this.signatureAlgorithm = signatureAlgorithm != null ? signatureAlgorithm : SignatureAlgorithm.DEFAULT; } /** @@ -44,6 +58,16 @@ public Secret getHookSecret() { public String getCredentialsId() { return credentialsId; } + + /** + * Gets the signature algorithm to use for webhook validation. + * + * @return the configured signature algorithm, defaults to SHA-256 + * @since 1.45.0 + */ + public SignatureAlgorithm getSignatureAlgorithm() { + return signatureAlgorithm != null ? signatureAlgorithm : SignatureAlgorithm.DEFAULT; + } /** * @param credentialsId a new ID @@ -53,6 +77,17 @@ public String getCredentialsId() { public void setCredentialsId(String credentialsId) { this.credentialsId = credentialsId; } + + /** + * Ensures backwards compatibility during deserialization. + * Sets default algorithm to SHA-256 for existing configurations. + */ + private Object readResolve() { + if (signatureAlgorithm == null) { + signatureAlgorithm = SignatureAlgorithm.DEFAULT; + } + return this; + } @Extension public static class DescriptorImpl extends Descriptor { diff --git a/src/main/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignature.java b/src/main/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignature.java index 4ded97d8e..345b546ea 100644 --- a/src/main/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignature.java +++ b/src/main/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignature.java @@ -23,6 +23,7 @@ public class GHWebhookSignature { private static final Logger LOGGER = LoggerFactory.getLogger(GHWebhookSignature.class); private static final String HMAC_SHA1_ALGORITHM = "HmacSHA1"; + private static final String HMAC_SHA256_ALGORITHM = "HmacSHA256"; public static final String INVALID_SIGNATURE = "COMPUTED_INVALID_SIGNATURE"; private final String payload; @@ -47,19 +48,43 @@ public static GHWebhookSignature webhookSignature(String payload, Secret secret) /** * Computes a RFC 2104-compliant HMAC digest using SHA1 of a payloadFrom with a given key (secret). * + * @deprecated Use {@link #sha256()} for enhanced security * @return HMAC digest of payloadFrom using secret as key. Will return COMPUTED_INVALID_SIGNATURE * on any exception during computation. */ + @Deprecated public String sha1() { + return computeSignature(HMAC_SHA1_ALGORITHM); + } + + /** + * Computes a RFC 2104-compliant HMAC digest using SHA256 of a payload with a given key (secret). + * This is the recommended method for webhook signature validation. + * + * @return HMAC digest of payload using secret as key. Will return COMPUTED_INVALID_SIGNATURE + * on any exception during computation. + * @since 1.45.0 + */ + public String sha256() { + return computeSignature(HMAC_SHA256_ALGORITHM); + } + + /** + * Computes HMAC signature using the specified algorithm. + * + * @param algorithm The HMAC algorithm to use (e.g., "HmacSHA1", "HmacSHA256") + * @return HMAC digest as hex string, or INVALID_SIGNATURE on error + */ + private String computeSignature(String algorithm) { try { - final SecretKeySpec keySpec = new SecretKeySpec(secret.getPlainText().getBytes(UTF_8), HMAC_SHA1_ALGORITHM); - final Mac mac = Mac.getInstance(HMAC_SHA1_ALGORITHM); + final SecretKeySpec keySpec = new SecretKeySpec(secret.getPlainText().getBytes(UTF_8), algorithm); + final Mac mac = Mac.getInstance(algorithm); mac.init(keySpec); final byte[] rawHMACBytes = mac.doFinal(payload.getBytes(UTF_8)); return Hex.encodeHexString(rawHMACBytes); } catch (Exception e) { - LOGGER.error("", e); + LOGGER.error("Error computing {} signature", algorithm, e); return INVALID_SIGNATURE; } } @@ -68,15 +93,45 @@ public String sha1() { * @param digest computed signature from external place (GitHub) * * @return true if computed and provided signatures identical + * @deprecated Use {@link #matches(String, SignatureAlgorithm)} for explicit algorithm selection */ + @Deprecated public boolean matches(String digest) { - String computed = sha1(); - LOGGER.trace("Signature: calculated={} provided={}", computed, digest); + return matches(digest, SignatureAlgorithm.SHA1); + } + + /** + * Validates a signature using the specified algorithm. + * Uses constant-time comparison to prevent timing attacks. + * + * @param digest the signature to validate (without algorithm prefix) + * @param algorithm the signature algorithm to use + * @return true if computed and provided signatures match + * @since 1.45.0 + */ + public boolean matches(String digest, SignatureAlgorithm algorithm) { + String computed; + switch (algorithm) { + case SHA256: + computed = sha256(); + break; + case SHA1: + computed = sha1(); + break; + default: + LOGGER.warn("Unsupported signature algorithm: {}", algorithm); + return false; + } + + LOGGER.trace("Signature validation: algorithm={} calculated={} provided={}", + algorithm, computed, digest); + if (digest == null && computed == null) { return true; } else if (digest == null || computed == null) { return false; } else { + // Use constant-time comparison to prevent timing attacks return MessageDigest.isEqual(computed.getBytes(UTF_8), digest.getBytes(UTF_8)); } } diff --git a/src/main/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayload.java b/src/main/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayload.java index e6944d4ea..36c095104 100644 --- a/src/main/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayload.java +++ b/src/main/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayload.java @@ -27,8 +27,6 @@ import java.nio.charset.StandardCharsets; import java.security.interfaces.RSAPublicKey; import java.util.List; -import java.util.Objects; -import java.util.stream.Collectors; import static com.cloudbees.jenkins.GitHubWebHook.X_INSTANCE_IDENTITY; import static com.google.common.base.Charsets.UTF_8; @@ -61,12 +59,24 @@ class Processor extends Interceptor { private static final Logger LOGGER = getLogger(Processor.class); /** - * Header key being used for the payload signatures. + * Header key being used for the legacy SHA-1 payload signatures. * * @see Developer manual + * @deprecated Use SHA-256 signatures with X-Hub-Signature-256 header */ + @Deprecated public static final String SIGNATURE_HEADER = "X-Hub-Signature"; + + /** + * Header key being used for the SHA-256 payload signatures (recommended). + * + * @see GitHub Documentation + * @since 1.45.0 + */ + public static final String SIGNATURE_HEADER_SHA256 = "X-Hub-Signature-256"; + private static final String SHA1_PREFIX = "sha1="; + private static final String SHA256_PREFIX = "sha256="; @Override public Object invoke(StaplerRequest2 req, StaplerResponse2 rsp, Object instance, Object[] arguments) @@ -138,26 +148,67 @@ protected void shouldContainParseablePayload(Object[] arguments) throws Invocati * Checks that an incoming request has a valid signature, * if a hook secret is specified in the GitHub plugin config. * If no hook secret is configured, then the signature is ignored. + * + * Uses the configured signature algorithm (SHA-256 by default, SHA-1 for legacy support). * * @param req Incoming request. * @throws InvocationTargetException if any of preconditions is not satisfied */ protected void shouldProvideValidSignature(StaplerRequest2 req, Object[] args) throws InvocationTargetException { - List secrets = GitHubPlugin.configuration().getHookSecretConfigs().stream(). - map(HookSecretConfig::getHookSecret).filter(Objects::nonNull).collect(Collectors.toList()); - - if (!secrets.isEmpty()) { - Optional signHeader = Optional.fromNullable(req.getHeader(SIGNATURE_HEADER)); - isTrue(signHeader.isPresent(), "Signature was expected, but not provided"); - - String digest = substringAfter(signHeader.get(), SHA1_PREFIX); - LOGGER.trace("Trying to verify sign from header {}", signHeader.get()); - isTrue( - secrets.stream().anyMatch(secret -> - GHWebhookSignature.webhookSignature(payloadFrom(req, args), secret).matches(digest)), - String.format("Provided signature [%s] did not match to calculated", digest) - ); + List secretConfigs = GitHubPlugin.configuration().getHookSecretConfigs(); + + if (!secretConfigs.isEmpty()) { + boolean validSignatureFound = false; + + for (HookSecretConfig config : secretConfigs) { + Secret secret = config.getHookSecret(); + if (secret == null) { + continue; + } + + SignatureAlgorithm algorithm = config.getSignatureAlgorithm(); + String headerName = algorithm.getHeaderName(); + String expectedPrefix = algorithm.getSignaturePrefix(); + + Optional signHeader = Optional.fromNullable(req.getHeader(headerName)); + if (!signHeader.isPresent()) { + LOGGER.debug("No signature header {} found for algorithm {}", headerName, algorithm); + continue; + } + + String fullSignature = signHeader.get(); + if (!fullSignature.startsWith(expectedPrefix)) { + LOGGER.debug("Signature header {} does not start with expected prefix {}", + fullSignature, expectedPrefix); + continue; + } + + String digest = substringAfter(fullSignature, expectedPrefix); + LOGGER.trace("Verifying {} signature from header {}", algorithm, fullSignature); + + boolean isValid = GHWebhookSignature.webhookSignature(payloadFrom(req, args), secret) + .matches(digest, algorithm); + + if (isValid) { + validSignatureFound = true; + + // Log deprecation warning for SHA-1 usage + if (algorithm == SignatureAlgorithm.SHA1) { + LOGGER.warn("Using deprecated SHA-1 signature validation. " + + "Consider upgrading webhook configuration to use SHA-256 for enhanced security."); + } else { + LOGGER.debug("Successfully validated {} signature", algorithm); + } + break; + } else { + LOGGER.debug("Signature validation failed for algorithm {}", algorithm); + } + } + + isTrue(validSignatureFound, + "No valid signature found. Ensure webhook is configured with a supported signature algorithm " + + "(SHA-256 recommended, SHA-1 for legacy compatibility)."); } } diff --git a/src/main/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithm.java b/src/main/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithm.java new file mode 100644 index 000000000..4eecdba03 --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithm.java @@ -0,0 +1,66 @@ +package org.jenkinsci.plugins.github.webhook; + +/** + * Enumeration of supported webhook signature algorithms. + * + * @since 1.45.0 + */ +public enum SignatureAlgorithm { + /** + * SHA-256 HMAC signature validation (recommended). + * Uses X-Hub-Signature-256 header with sha256= prefix. + */ + SHA256("sha256", "X-Hub-Signature-256", "HmacSHA256"), + + /** + * SHA-1 HMAC signature validation (legacy). + * Uses X-Hub-Signature header with sha1= prefix. + * + * @deprecated Use SHA256 for enhanced security + */ + @Deprecated + SHA1("sha1", "X-Hub-Signature", "HmacSHA1"); + + private final String prefix; + private final String headerName; + private final String javaAlgorithm; + + /** + * Default algorithm for new configurations - SHA-256 for security. + */ + public static final SignatureAlgorithm DEFAULT = SHA256; + + SignatureAlgorithm(String prefix, String headerName, String javaAlgorithm) { + this.prefix = prefix; + this.headerName = headerName; + this.javaAlgorithm = javaAlgorithm; + } + + /** + * @return the prefix used in signature strings (e.g. "sha256", "sha1") + */ + public String getPrefix() { + return prefix; + } + + /** + * @return the HTTP header name for this algorithm + */ + public String getHeaderName() { + return headerName; + } + + /** + * @return the Java algorithm name for HMAC computation + */ + public String getJavaAlgorithm() { + return javaAlgorithm; + } + + /** + * @return the expected signature prefix including equals sign (e.g. "sha256=", "sha1=") + */ + public String getSignaturePrefix() { + return prefix + "="; + } +} \ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/github/config/HookSecretConfigSHA256Test.java b/src/test/java/org/jenkinsci/plugins/github/config/HookSecretConfigSHA256Test.java new file mode 100644 index 000000000..21de0bbb1 --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/github/config/HookSecretConfigSHA256Test.java @@ -0,0 +1,47 @@ +package org.jenkinsci.plugins.github.config; + +import org.jenkinsci.plugins.github.webhook.SignatureAlgorithm; +import org.junit.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +/** + * Tests for SHA-256 configuration in {@link HookSecretConfig}. + * + * @since 1.45.0 + */ +public class HookSecretConfigSHA256Test { + + @Test + public void shouldDefaultToSHA256Algorithm() { + HookSecretConfig config = new HookSecretConfig("test-credentials"); + + assertThat("Should default to SHA-256 algorithm", + config.getSignatureAlgorithm(), equalTo(SignatureAlgorithm.SHA256)); + } + + @Test + public void shouldAcceptExplicitSHA256Algorithm() { + HookSecretConfig config = new HookSecretConfig("test-credentials", SignatureAlgorithm.SHA256); + + assertThat("Should use explicitly set SHA-256 algorithm", + config.getSignatureAlgorithm(), equalTo(SignatureAlgorithm.SHA256)); + } + + @Test + public void shouldAcceptSHA1Algorithm() { + HookSecretConfig config = new HookSecretConfig("test-credentials", SignatureAlgorithm.SHA1); + + assertThat("Should use explicitly set SHA-1 algorithm", + config.getSignatureAlgorithm(), equalTo(SignatureAlgorithm.SHA1)); + } + + @Test + public void shouldDefaultToSHA256WhenNullAlgorithmProvided() { + HookSecretConfig config = new HookSecretConfig("test-credentials", null); + + assertThat("Should default to SHA-256 when null algorithm provided", + config.getSignatureAlgorithm(), equalTo(SignatureAlgorithm.SHA256)); + } +} \ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignatureSHA256Test.java b/src/test/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignatureSHA256Test.java new file mode 100644 index 000000000..df2280160 --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignatureSHA256Test.java @@ -0,0 +1,92 @@ +package org.jenkinsci.plugins.github.webhook; + +import hudson.util.Secret; +import org.junit.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +/** + * Tests for SHA-256 functionality in {@link GHWebhookSignature}. + * + * @since 1.45.0 + */ +public class GHWebhookSignatureSHA256Test { + + private static final String SECRET_CONTENT = "It's a Secret to Everybody"; + private static final String PAYLOAD = "Hello, World!"; + // Expected SHA-256 signature based on GitHub's documentation + private static final String EXPECTED_SHA256_DIGEST = "757107ea0eb2509fc211221cce984b8a37570b6d7586c22c46f4379c8b043e17"; + + @Test + public void shouldComputeCorrectSHA256Signature() { + Secret secret = Secret.fromString(SECRET_CONTENT); + GHWebhookSignature signature = GHWebhookSignature.webhookSignature(PAYLOAD, secret); + + String computed = signature.sha256(); + + assertThat("SHA-256 signature should match expected value", + computed, equalTo(EXPECTED_SHA256_DIGEST)); + } + + @Test + public void shouldValidateSHA256SignatureCorrectly() { + Secret secret = Secret.fromString(SECRET_CONTENT); + GHWebhookSignature signature = GHWebhookSignature.webhookSignature(PAYLOAD, secret); + + boolean isValid = signature.matches(EXPECTED_SHA256_DIGEST, SignatureAlgorithm.SHA256); + + assertThat("Valid SHA-256 signature should be accepted", isValid, equalTo(true)); + } + + @Test + public void shouldRejectInvalidSHA256Signature() { + Secret secret = Secret.fromString(SECRET_CONTENT); + GHWebhookSignature signature = GHWebhookSignature.webhookSignature(PAYLOAD, secret); + + String invalidDigest = "invalid_signature_digest"; + boolean isValid = signature.matches(invalidDigest, SignatureAlgorithm.SHA256); + + assertThat("Invalid SHA-256 signature should be rejected", isValid, equalTo(false)); + } + + @Test + public void shouldRejectSHA1SignatureWhenExpectingSHA256() { + String secretContent = "test-secret"; + Secret secret = Secret.fromString(secretContent); + GHWebhookSignature signature = GHWebhookSignature.webhookSignature(PAYLOAD, secret); + + // Get SHA-1 digest but try to validate as SHA-256 + String sha1Digest = signature.sha1(); + boolean isValid = signature.matches(sha1Digest, SignatureAlgorithm.SHA256); + + assertThat("SHA-1 signature should be rejected when expecting SHA-256", + isValid, equalTo(false)); + } + + @Test + public void shouldHandleDifferentPayloads() { + Secret secret = Secret.fromString(SECRET_CONTENT); + String payload1 = "payload1"; + String payload2 = "payload2"; + + GHWebhookSignature signature1 = GHWebhookSignature.webhookSignature(payload1, secret); + GHWebhookSignature signature2 = GHWebhookSignature.webhookSignature(payload2, secret); + + String digest1 = signature1.sha256(); + String digest2 = signature2.sha256(); + + assertThat("Different payloads should produce different signatures", + digest1.equals(digest2), equalTo(false)); + + // Each signature should validate its own payload + assertThat("Signature 1 should validate payload 1", + signature1.matches(digest1, SignatureAlgorithm.SHA256), equalTo(true)); + assertThat("Signature 2 should validate payload 2", + signature2.matches(digest2, SignatureAlgorithm.SHA256), equalTo(true)); + + // Cross-validation should fail + assertThat("Signature 1 should not validate payload 2's digest", + signature1.matches(digest2, SignatureAlgorithm.SHA256), equalTo(false)); + } +} \ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithmTest.java b/src/test/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithmTest.java new file mode 100644 index 000000000..722b1c1fa --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithmTest.java @@ -0,0 +1,40 @@ +package org.jenkinsci.plugins.github.webhook; + +import org.junit.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +/** + * Tests for {@link SignatureAlgorithm}. + * + * @since 1.45.0 + */ +public class SignatureAlgorithmTest { + + @Test + public void shouldHaveCorrectSHA256Properties() { + SignatureAlgorithm algorithm = SignatureAlgorithm.SHA256; + + assertThat("SHA-256 prefix", algorithm.getPrefix(), equalTo("sha256")); + assertThat("SHA-256 header", algorithm.getHeaderName(), equalTo("X-Hub-Signature-256")); + assertThat("SHA-256 Java algorithm", algorithm.getJavaAlgorithm(), equalTo("HmacSHA256")); + assertThat("SHA-256 signature prefix", algorithm.getSignaturePrefix(), equalTo("sha256=")); + } + + @Test + public void shouldHaveCorrectSHA1Properties() { + SignatureAlgorithm algorithm = SignatureAlgorithm.SHA1; + + assertThat("SHA-1 prefix", algorithm.getPrefix(), equalTo("sha1")); + assertThat("SHA-1 header", algorithm.getHeaderName(), equalTo("X-Hub-Signature")); + assertThat("SHA-1 Java algorithm", algorithm.getJavaAlgorithm(), equalTo("HmacSHA1")); + assertThat("SHA-1 signature prefix", algorithm.getSignaturePrefix(), equalTo("sha1=")); + } + + @Test + public void shouldDefaultToSHA256() { + assertThat("Default algorithm should be SHA-256", + SignatureAlgorithm.DEFAULT, equalTo(SignatureAlgorithm.SHA256)); + } +} \ No newline at end of file From 9a53e1067c5f5bb183a6c206f11f9f7e51bea1b8 Mon Sep 17 00:00:00 2001 From: Jason Heithoff Date: Mon, 25 Aug 2025 18:35:01 -0400 Subject: [PATCH 2/6] feat(JENKINS-73851): Add UI for signature algorithm selection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add doFillSignatureAlgorithmItems() method to provide dropdown options - Create signature algorithm selection dropdown in Jenkins configuration UI - Add help documentation for signature algorithm selection - Update HookSecretConfig constructor to parse algorithm from UI string input - Add parseSignatureAlgorithm() method with case-insensitive parsing - Update tests to work with new string-based constructor - Add comprehensive test cases for algorithm parsing edge cases Users can now choose between SHA-256 (Recommended) and SHA-1 (Legacy) signature algorithms through the Jenkins UI in the GitHub plugin configuration section. The dropdown properly displays both options with SHA-256 set as default for enhanced security, while SHA-1 remains available for legacy compatibility. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- .../github/config/HookSecretConfig.java | 53 +++++++++++++++---- .../github/webhook/SignatureAlgorithm.java | 2 +- .../config/HookSecretConfig/config.groovy | 4 ++ .../help-signatureAlgorithm.html | 11 ++++ .../config/HookSecretConfigSHA256Test.java | 23 +++++++- 5 files changed, 80 insertions(+), 13 deletions(-) create mode 100644 src/main/resources/org/jenkinsci/plugins/github/config/HookSecretConfig/help-signatureAlgorithm.html diff --git a/src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java b/src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java index b67b039a2..c4c144a18 100644 --- a/src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java +++ b/src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java @@ -29,21 +29,18 @@ public class HookSecretConfig extends AbstractDescribableImpl private SignatureAlgorithm signatureAlgorithm; @DataBoundConstructor - public HookSecretConfig(String credentialsId) { - this(credentialsId, null); + public HookSecretConfig(String credentialsId, String signatureAlgorithm) { + this.credentialsId = credentialsId; + this.signatureAlgorithm = parseSignatureAlgorithm(signatureAlgorithm); } /** - * Constructor with signature algorithm specification. - * - * @param credentialsId the credentials ID for the webhook secret - * @param signatureAlgorithm the signature algorithm to use (defaults to SHA-256) - * @since 1.45.0 + * Legacy constructor for backwards compatibility. */ - public HookSecretConfig(String credentialsId, SignatureAlgorithm signatureAlgorithm) { - this.credentialsId = credentialsId; - this.signatureAlgorithm = signatureAlgorithm != null ? signatureAlgorithm : SignatureAlgorithm.DEFAULT; + public HookSecretConfig(String credentialsId) { + this(credentialsId, null); } + /** * Gets the currently used secret being used for payload verification. @@ -68,6 +65,16 @@ public String getCredentialsId() { public SignatureAlgorithm getSignatureAlgorithm() { return signatureAlgorithm != null ? signatureAlgorithm : SignatureAlgorithm.DEFAULT; } + + /** + * Gets the signature algorithm name for UI binding. + * + * @return the algorithm name as string (e.g., "SHA256", "SHA1") + * @since 1.45.0 + */ + public String getSignatureAlgorithmName() { + return getSignatureAlgorithm().name(); + } /** * @param credentialsId a new ID @@ -88,6 +95,22 @@ private Object readResolve() { } return this; } + + /** + * Parses signature algorithm from UI string input. + */ + private SignatureAlgorithm parseSignatureAlgorithm(String algorithmName) { + if (algorithmName == null || algorithmName.trim().isEmpty()) { + return SignatureAlgorithm.DEFAULT; + } + + try { + return SignatureAlgorithm.valueOf(algorithmName.trim().toUpperCase()); + } catch (IllegalArgumentException e) { + // Default to SHA-256 for invalid input + return SignatureAlgorithm.DEFAULT; + } + } @Extension public static class DescriptorImpl extends Descriptor { @@ -96,6 +119,16 @@ public static class DescriptorImpl extends Descriptor { public String getDisplayName() { return "Hook secret configuration"; } + + /** + * Provides dropdown items for signature algorithm selection. + */ + public ListBoxModel doFillSignatureAlgorithmItems() { + ListBoxModel items = new ListBoxModel(); + items.add("SHA-256 (Recommended)", "SHA256"); + items.add("SHA-1 (Legacy)", "SHA1"); + return items; + } @SuppressWarnings("unused") public ListBoxModel doFillCredentialsIdItems(@QueryParameter String credentialsId) { diff --git a/src/main/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithm.java b/src/main/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithm.java index 4eecdba03..073c80922 100644 --- a/src/main/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithm.java +++ b/src/main/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithm.java @@ -63,4 +63,4 @@ public String getJavaAlgorithm() { public String getSignaturePrefix() { return prefix + "="; } -} \ No newline at end of file +} diff --git a/src/main/resources/org/jenkinsci/plugins/github/config/HookSecretConfig/config.groovy b/src/main/resources/org/jenkinsci/plugins/github/config/HookSecretConfig/config.groovy index 85e11ffae..2e5cce9ff 100644 --- a/src/main/resources/org/jenkinsci/plugins/github/config/HookSecretConfig/config.groovy +++ b/src/main/resources/org/jenkinsci/plugins/github/config/HookSecretConfig/config.groovy @@ -6,3 +6,7 @@ def c = namespace(lib.CredentialsTagLib); f.entry(title: _("Shared secret"), field: "credentialsId", help: descriptor.getHelpFile('sharedSecret')) { c.select(context: app, includeUser: false, expressionAllowed: false) } + +f.entry(title: _("Signature algorithm"), field: "signatureAlgorithm") { + f.select() +} diff --git a/src/main/resources/org/jenkinsci/plugins/github/config/HookSecretConfig/help-signatureAlgorithm.html b/src/main/resources/org/jenkinsci/plugins/github/config/HookSecretConfig/help-signatureAlgorithm.html new file mode 100644 index 000000000..ffe31af39 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/github/config/HookSecretConfig/help-signatureAlgorithm.html @@ -0,0 +1,11 @@ +
+

Choose the signature algorithm for webhook validation:

+
    +
  • SHA-256 (Recommended): Modern, secure HMAC signature validation using the + X-Hub-Signature-256 header. This is GitHub's recommended approach for enhanced security.
    • +
  • SHA-1 (Legacy): Legacy HMAC signature validation using the + X-Hub-Signature header. Only use this for existing webhooks during migration period.
    • +
+

Note: When changing algorithms, ensure your GitHub webhook configuration uses the corresponding + signature header (X-Hub-Signature-256 for SHA-256 or X-Hub-Signature for SHA-1).

+
\ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/github/config/HookSecretConfigSHA256Test.java b/src/test/java/org/jenkinsci/plugins/github/config/HookSecretConfigSHA256Test.java index 21de0bbb1..2c15bfaea 100644 --- a/src/test/java/org/jenkinsci/plugins/github/config/HookSecretConfigSHA256Test.java +++ b/src/test/java/org/jenkinsci/plugins/github/config/HookSecretConfigSHA256Test.java @@ -23,7 +23,7 @@ public void shouldDefaultToSHA256Algorithm() { @Test public void shouldAcceptExplicitSHA256Algorithm() { - HookSecretConfig config = new HookSecretConfig("test-credentials", SignatureAlgorithm.SHA256); + HookSecretConfig config = new HookSecretConfig("test-credentials", "SHA256"); assertThat("Should use explicitly set SHA-256 algorithm", config.getSignatureAlgorithm(), equalTo(SignatureAlgorithm.SHA256)); @@ -31,7 +31,7 @@ public void shouldAcceptExplicitSHA256Algorithm() { @Test public void shouldAcceptSHA1Algorithm() { - HookSecretConfig config = new HookSecretConfig("test-credentials", SignatureAlgorithm.SHA1); + HookSecretConfig config = new HookSecretConfig("test-credentials", "SHA1"); assertThat("Should use explicitly set SHA-1 algorithm", config.getSignatureAlgorithm(), equalTo(SignatureAlgorithm.SHA1)); @@ -44,4 +44,23 @@ public void shouldDefaultToSHA256WhenNullAlgorithmProvided() { assertThat("Should default to SHA-256 when null algorithm provided", config.getSignatureAlgorithm(), equalTo(SignatureAlgorithm.SHA256)); } + + @Test + public void shouldDefaultToSHA256WhenInvalidAlgorithmProvided() { + HookSecretConfig config = new HookSecretConfig("test-credentials", "INVALID"); + + assertThat("Should default to SHA-256 when invalid algorithm provided", + config.getSignatureAlgorithm(), equalTo(SignatureAlgorithm.SHA256)); + } + + @Test + public void shouldBeCaseInsensitive() { + HookSecretConfig config1 = new HookSecretConfig("test-credentials", "sha256"); + HookSecretConfig config2 = new HookSecretConfig("test-credentials", "Sha1"); + + assertThat("Should handle lowercase SHA-256", + config1.getSignatureAlgorithm(), equalTo(SignatureAlgorithm.SHA256)); + assertThat("Should handle mixed case SHA-1", + config2.getSignatureAlgorithm(), equalTo(SignatureAlgorithm.SHA1)); + } } \ No newline at end of file From 49f96ca89203684bac9602b2a3881a7311013e2f Mon Sep 17 00:00:00 2001 From: Jason Heithoff Date: Mon, 25 Aug 2025 19:33:35 -0400 Subject: [PATCH 3/6] fix(JENKINS-73851): Fix checkstyle violations in SHA-256 implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove trailing whitespace from all modified files - Fix line length violations by properly wrapping long lines - Fix operator wrap issues by placing operators on new lines - Maintain consistent code formatting throughout 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- .../github/config/HookSecretConfig.java | 15 ++++--- .../github/webhook/GHWebhookSignature.java | 8 ++-- .../webhook/RequirePostWithGHHookPayload.java | 39 +++++++++---------- 3 files changed, 29 insertions(+), 33 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java b/src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java index c4c144a18..3f0e3585f 100644 --- a/src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java +++ b/src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java @@ -33,14 +33,13 @@ public HookSecretConfig(String credentialsId, String signatureAlgorithm) { this.credentialsId = credentialsId; this.signatureAlgorithm = parseSignatureAlgorithm(signatureAlgorithm); } - + /** * Legacy constructor for backwards compatibility. */ public HookSecretConfig(String credentialsId) { this(credentialsId, null); } - /** * Gets the currently used secret being used for payload verification. @@ -55,7 +54,7 @@ public Secret getHookSecret() { public String getCredentialsId() { return credentialsId; } - + /** * Gets the signature algorithm to use for webhook validation. * @@ -65,7 +64,7 @@ public String getCredentialsId() { public SignatureAlgorithm getSignatureAlgorithm() { return signatureAlgorithm != null ? signatureAlgorithm : SignatureAlgorithm.DEFAULT; } - + /** * Gets the signature algorithm name for UI binding. * @@ -84,7 +83,7 @@ public String getSignatureAlgorithmName() { public void setCredentialsId(String credentialsId) { this.credentialsId = credentialsId; } - + /** * Ensures backwards compatibility during deserialization. * Sets default algorithm to SHA-256 for existing configurations. @@ -95,7 +94,7 @@ private Object readResolve() { } return this; } - + /** * Parses signature algorithm from UI string input. */ @@ -103,7 +102,7 @@ private SignatureAlgorithm parseSignatureAlgorithm(String algorithmName) { if (algorithmName == null || algorithmName.trim().isEmpty()) { return SignatureAlgorithm.DEFAULT; } - + try { return SignatureAlgorithm.valueOf(algorithmName.trim().toUpperCase()); } catch (IllegalArgumentException e) { @@ -119,7 +118,7 @@ public static class DescriptorImpl extends Descriptor { public String getDisplayName() { return "Hook secret configuration"; } - + /** * Provides dropdown items for signature algorithm selection. */ diff --git a/src/main/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignature.java b/src/main/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignature.java index 345b546ea..491223c76 100644 --- a/src/main/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignature.java +++ b/src/main/java/org/jenkinsci/plugins/github/webhook/GHWebhookSignature.java @@ -68,7 +68,6 @@ public String sha1() { public String sha256() { return computeSignature(HMAC_SHA256_ALGORITHM); } - /** * Computes HMAC signature using the specified algorithm. * @@ -99,7 +98,7 @@ private String computeSignature(String algorithm) { public boolean matches(String digest) { return matches(digest, SignatureAlgorithm.SHA1); } - + /** * Validates a signature using the specified algorithm. * Uses constant-time comparison to prevent timing attacks. @@ -122,10 +121,9 @@ public boolean matches(String digest, SignatureAlgorithm algorithm) { LOGGER.warn("Unsupported signature algorithm: {}", algorithm); return false; } - - LOGGER.trace("Signature validation: algorithm={} calculated={} provided={}", + + LOGGER.trace("Signature validation: algorithm={} calculated={} provided={}", algorithm, computed, digest); - if (digest == null && computed == null) { return true; } else if (digest == null || computed == null) { diff --git a/src/main/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayload.java b/src/main/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayload.java index 36c095104..9edb26307 100644 --- a/src/main/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayload.java +++ b/src/main/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayload.java @@ -66,15 +66,14 @@ class Processor extends Interceptor { */ @Deprecated public static final String SIGNATURE_HEADER = "X-Hub-Signature"; - /** * Header key being used for the SHA-256 payload signatures (recommended). * - * @see GitHub Documentation + * @see + * GitHub Documentation * @since 1.45.0 */ public static final String SIGNATURE_HEADER_SHA256 = "X-Hub-Signature-256"; - private static final String SHA1_PREFIX = "sha1="; private static final String SHA256_PREFIX = "sha256="; @@ -148,7 +147,7 @@ protected void shouldContainParseablePayload(Object[] arguments) throws Invocati * Checks that an incoming request has a valid signature, * if a hook secret is specified in the GitHub plugin config. * If no hook secret is configured, then the signature is ignored. - * + * * Uses the configured signature algorithm (SHA-256 by default, SHA-1 for legacy support). * * @param req Incoming request. @@ -157,46 +156,46 @@ protected void shouldContainParseablePayload(Object[] arguments) throws Invocati protected void shouldProvideValidSignature(StaplerRequest2 req, Object[] args) throws InvocationTargetException { List secretConfigs = GitHubPlugin.configuration().getHookSecretConfigs(); - + if (!secretConfigs.isEmpty()) { boolean validSignatureFound = false; - + for (HookSecretConfig config : secretConfigs) { Secret secret = config.getHookSecret(); if (secret == null) { continue; } - + SignatureAlgorithm algorithm = config.getSignatureAlgorithm(); String headerName = algorithm.getHeaderName(); String expectedPrefix = algorithm.getSignaturePrefix(); - + Optional signHeader = Optional.fromNullable(req.getHeader(headerName)); if (!signHeader.isPresent()) { LOGGER.debug("No signature header {} found for algorithm {}", headerName, algorithm); continue; } - + String fullSignature = signHeader.get(); if (!fullSignature.startsWith(expectedPrefix)) { - LOGGER.debug("Signature header {} does not start with expected prefix {}", + LOGGER.debug("Signature header {} does not start with expected prefix {}", fullSignature, expectedPrefix); continue; } - + String digest = substringAfter(fullSignature, expectedPrefix); LOGGER.trace("Verifying {} signature from header {}", algorithm, fullSignature); - + boolean isValid = GHWebhookSignature.webhookSignature(payloadFrom(req, args), secret) .matches(digest, algorithm); - + if (isValid) { validSignatureFound = true; - // Log deprecation warning for SHA-1 usage if (algorithm == SignatureAlgorithm.SHA1) { - LOGGER.warn("Using deprecated SHA-1 signature validation. " + - "Consider upgrading webhook configuration to use SHA-256 for enhanced security."); + LOGGER.warn("Using deprecated SHA-1 signature validation. " + + "Consider upgrading webhook configuration to use SHA-256 " + + "for enhanced security."); } else { LOGGER.debug("Successfully validated {} signature", algorithm); } @@ -205,10 +204,10 @@ protected void shouldProvideValidSignature(StaplerRequest2 req, Object[] args) LOGGER.debug("Signature validation failed for algorithm {}", algorithm); } } - - isTrue(validSignatureFound, - "No valid signature found. Ensure webhook is configured with a supported signature algorithm " + - "(SHA-256 recommended, SHA-1 for legacy compatibility)."); + + isTrue(validSignatureFound, + "No valid signature found. Ensure webhook is configured with a supported signature algorithm " + + "(SHA-256 recommended, SHA-1 for legacy compatibility)."); } } From b30851851e9c1d5d0890ed892ab5a21af02b1358 Mon Sep 17 00:00:00 2001 From: Jason Heithoff Date: Mon, 25 Aug 2025 19:36:17 -0400 Subject: [PATCH 4/6] chore: Remove .vscode/settings.json and add to .gitignore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove IDE-specific configuration file from version control - Add .vscode/ directory to .gitignore to prevent future tracking - Keep IDE configurations local to individual developer environments 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- .gitignore | 1 + .vscode/settings.json | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) delete mode 100644 .vscode/settings.json diff --git a/.gitignore b/.gitignore index 99b3c61f0..41dfd3e40 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ target # autogenerated resources src/main/webapp/css/* +.vscode/ diff --git a/.vscode/settings.json b/.vscode/settings.json deleted file mode 100644 index 7b016a89f..000000000 --- a/.vscode/settings.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "java.compile.nullAnalysis.mode": "automatic" -} \ No newline at end of file From 6448d9e4856c5aaf24e7fba759b795c1994cd078 Mon Sep 17 00:00:00 2001 From: Jason Heithoff Date: Mon, 25 Aug 2025 20:35:54 -0400 Subject: [PATCH 5/6] feat(JENKINS-73851): Add system property override for default signature algorithm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add jenkins.github.webhook.signature.default system property - Allows overriding default from SHA-256 to SHA-1 for CI compatibility - Maintains SHA-256 as secure default when no property is set - Dynamic evaluation prevents static initialization issues - Added comprehensive test coverage and documentation Usage: - Default: SHA-256 (secure) - CI override: -Djenkins.github.webhook.signature.default=SHA1 - Invalid values fallback to SHA-256 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- .../github/config/HookSecretConfig.java | 8 ++--- .../github/webhook/SignatureAlgorithm.java | 36 +++++++++++++++++-- .../help-signatureAlgorithm.html | 2 ++ .../config/HookSecretConfigSHA256Test.java | 22 ++++++++++++ .../webhook/SignatureAlgorithmTest.java | 2 +- 5 files changed, 63 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java b/src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java index 3f0e3585f..9db733af7 100644 --- a/src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java +++ b/src/main/java/org/jenkinsci/plugins/github/config/HookSecretConfig.java @@ -62,7 +62,7 @@ public String getCredentialsId() { * @since 1.45.0 */ public SignatureAlgorithm getSignatureAlgorithm() { - return signatureAlgorithm != null ? signatureAlgorithm : SignatureAlgorithm.DEFAULT; + return signatureAlgorithm != null ? signatureAlgorithm : SignatureAlgorithm.getDefault(); } /** @@ -90,7 +90,7 @@ public void setCredentialsId(String credentialsId) { */ private Object readResolve() { if (signatureAlgorithm == null) { - signatureAlgorithm = SignatureAlgorithm.DEFAULT; + signatureAlgorithm = SignatureAlgorithm.getDefault(); } return this; } @@ -100,14 +100,14 @@ private Object readResolve() { */ private SignatureAlgorithm parseSignatureAlgorithm(String algorithmName) { if (algorithmName == null || algorithmName.trim().isEmpty()) { - return SignatureAlgorithm.DEFAULT; + return SignatureAlgorithm.getDefault(); } try { return SignatureAlgorithm.valueOf(algorithmName.trim().toUpperCase()); } catch (IllegalArgumentException e) { // Default to SHA-256 for invalid input - return SignatureAlgorithm.DEFAULT; + return SignatureAlgorithm.getDefault(); } } diff --git a/src/main/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithm.java b/src/main/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithm.java index 073c80922..6668f6e81 100644 --- a/src/main/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithm.java +++ b/src/main/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithm.java @@ -26,9 +26,21 @@ public enum SignatureAlgorithm { private final String javaAlgorithm; /** - * Default algorithm for new configurations - SHA-256 for security. + * System property to override default signature algorithm. + * Set to "SHA1" to use legacy SHA-1 as default for backwards compatibility. */ - public static final SignatureAlgorithm DEFAULT = SHA256; + public static final String DEFAULT_ALGORITHM_PROPERTY = "jenkins.github.webhook.signature.default"; + + /** + * Gets the default algorithm for new configurations. + * Defaults to SHA-256 for security, but can be overridden via system property. + * This is evaluated dynamically to respect system property changes. + * + * @return the default algorithm based on current system property + */ + public static SignatureAlgorithm getDefault() { + return getDefaultAlgorithm(); + } SignatureAlgorithm(String prefix, String headerName, String javaAlgorithm) { this.prefix = prefix; @@ -63,4 +75,24 @@ public String getJavaAlgorithm() { public String getSignaturePrefix() { return prefix + "="; } + + /** + * Determines the default signature algorithm based on system property. + * Defaults to SHA-256 for security, but allows SHA-1 override for legacy environments. + * + * @return the default algorithm to use + */ + private static SignatureAlgorithm getDefaultAlgorithm() { + String property = System.getProperty(DEFAULT_ALGORITHM_PROPERTY); + if (property == null || property.trim().isEmpty()) { + // No property set, use secure SHA-256 default + return SHA256; + } + try { + return SignatureAlgorithm.valueOf(property.trim().toUpperCase()); + } catch (IllegalArgumentException e) { + // Invalid property value, default to secure SHA-256 + return SHA256; + } + } } diff --git a/src/main/resources/org/jenkinsci/plugins/github/config/HookSecretConfig/help-signatureAlgorithm.html b/src/main/resources/org/jenkinsci/plugins/github/config/HookSecretConfig/help-signatureAlgorithm.html index ffe31af39..5092fb6d9 100644 --- a/src/main/resources/org/jenkinsci/plugins/github/config/HookSecretConfig/help-signatureAlgorithm.html +++ b/src/main/resources/org/jenkinsci/plugins/github/config/HookSecretConfig/help-signatureAlgorithm.html @@ -8,4 +8,6 @@

Note: When changing algorithms, ensure your GitHub webhook configuration uses the corresponding signature header (X-Hub-Signature-256 for SHA-256 or X-Hub-Signature for SHA-1).

+

System Property Override: The default algorithm can be overridden using the system property + -Djenkins.github.webhook.signature.default=SHA1 for backwards compatibility with legacy CI environments.

\ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/github/config/HookSecretConfigSHA256Test.java b/src/test/java/org/jenkinsci/plugins/github/config/HookSecretConfigSHA256Test.java index 2c15bfaea..698b56911 100644 --- a/src/test/java/org/jenkinsci/plugins/github/config/HookSecretConfigSHA256Test.java +++ b/src/test/java/org/jenkinsci/plugins/github/config/HookSecretConfigSHA256Test.java @@ -63,4 +63,26 @@ public void shouldBeCaseInsensitive() { assertThat("Should handle mixed case SHA-1", config2.getSignatureAlgorithm(), equalTo(SignatureAlgorithm.SHA1)); } + + @Test + public void shouldRespectSystemPropertyOverride() { + // Save original property + String originalProperty = System.getProperty("jenkins.github.webhook.signature.default"); + + try { + // Test SHA1 override + System.setProperty("jenkins.github.webhook.signature.default", "SHA1"); + HookSecretConfig config = new HookSecretConfig("test-credentials"); + + assertThat("Should use SHA-1 when system property is set", + config.getSignatureAlgorithm(), equalTo(SignatureAlgorithm.SHA1)); + } finally { + // Restore original property + if (originalProperty != null) { + System.setProperty("jenkins.github.webhook.signature.default", originalProperty); + } else { + System.clearProperty("jenkins.github.webhook.signature.default"); + } + } + } } \ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithmTest.java b/src/test/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithmTest.java index 722b1c1fa..37b16eeeb 100644 --- a/src/test/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithmTest.java +++ b/src/test/java/org/jenkinsci/plugins/github/webhook/SignatureAlgorithmTest.java @@ -35,6 +35,6 @@ public void shouldHaveCorrectSHA1Properties() { @Test public void shouldDefaultToSHA256() { assertThat("Default algorithm should be SHA-256", - SignatureAlgorithm.DEFAULT, equalTo(SignatureAlgorithm.SHA256)); + SignatureAlgorithm.getDefault(), equalTo(SignatureAlgorithm.SHA256)); } } \ No newline at end of file From 1ecbb4d7e235bb1ce9940972227445c81c210fb2 Mon Sep 17 00:00:00 2001 From: Jason Heithoff Date: Tue, 26 Aug 2025 18:44:37 -0400 Subject: [PATCH 6/6] fix(JENKINS-73851): Fix failing tests --- .../plugins/github/webhook/RequirePostWithGHHookPayload.java | 4 ++-- .../java/com/cloudbees/jenkins/GitHubWebHookFullTest.java | 4 +++- .../github/webhook/RequirePostWithGHHookPayloadTest.java | 2 ++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayload.java b/src/main/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayload.java index 9edb26307..9a36c06f7 100644 --- a/src/main/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayload.java +++ b/src/main/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayload.java @@ -74,8 +74,8 @@ class Processor extends Interceptor { * @since 1.45.0 */ public static final String SIGNATURE_HEADER_SHA256 = "X-Hub-Signature-256"; - private static final String SHA1_PREFIX = "sha1="; - private static final String SHA256_PREFIX = "sha256="; + public static final String SHA1_PREFIX = "sha1="; + public static final String SHA256_PREFIX = "sha256="; @Override public Object invoke(StaplerRequest2 req, StaplerResponse2 rsp, Object instance, Object[] arguments) diff --git a/src/test/java/com/cloudbees/jenkins/GitHubWebHookFullTest.java b/src/test/java/com/cloudbees/jenkins/GitHubWebHookFullTest.java index 2c8383932..add363db8 100644 --- a/src/test/java/com/cloudbees/jenkins/GitHubWebHookFullTest.java +++ b/src/test/java/com/cloudbees/jenkins/GitHubWebHookFullTest.java @@ -32,7 +32,7 @@ import static org.hamcrest.Matchers.notNullValue; import static org.jenkinsci.plugins.github.test.HookSecretHelper.removeSecretIn; import static org.jenkinsci.plugins.github.test.HookSecretHelper.storeSecretIn; -import static org.jenkinsci.plugins.github.webhook.RequirePostWithGHHookPayload.Processor.SIGNATURE_HEADER; +import static org.jenkinsci.plugins.github.webhook.RequirePostWithGHHookPayload.Processor.*; /** * @author lanwen (Merkushev Kirill) @@ -92,6 +92,7 @@ public void shouldParseJsonWebHookFromGH() throws Exception { @Test public void shouldParseJsonWebHookFromGHWithSignHeader() throws Exception { String hash = "355e155fc3d10c4e5f2c6086a01281d2e947d932"; + String hash256 = "85e61999573c7023720a12375e1e55d18a0870e1ef880736f6ffc9273d0519e3"; String secret = "123"; storeSecretIn(config, secret); @@ -99,6 +100,7 @@ public void shouldParseJsonWebHookFromGHWithSignHeader() throws Exception { .header(eventHeader(GHEvent.PUSH)) .header(JSON_CONTENT_TYPE) .header(SIGNATURE_HEADER, format("sha1=%s", hash)) + .header(SIGNATURE_HEADER_SHA256, format("%s%s", SHA256_PREFIX, hash256)) .body(classpath(String.format("payloads/ping_hash_%s_secret_%s.json", hash, secret))) .log().all() .expect().log().all().statusCode(SC_OK).request().post(getPath()); diff --git a/src/test/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayloadTest.java b/src/test/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayloadTest.java index d1725fda6..878e9f1a6 100644 --- a/src/test/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayloadTest.java +++ b/src/test/java/org/jenkinsci/plugins/github/webhook/RequirePostWithGHHookPayloadTest.java @@ -134,8 +134,10 @@ public void shouldNotPassOnMalformedSignature() throws Exception { @Test public void shouldPassWithValidSignature() throws Exception { final String signature = "sha1=49d5f5cf800a81f257324912969a2d325d13d3fc"; + final String signature256 = "sha256=569beaec8ea1c9deccec283d0bb96aeec0a77310c70875343737ae72cffa7044"; when(req.getHeader(RequirePostWithGHHookPayload.Processor.SIGNATURE_HEADER)).thenReturn(signature); + when(req.getHeader(RequirePostWithGHHookPayload.Processor.SIGNATURE_HEADER_SHA256)).thenReturn(signature256); doReturn(PAYLOAD).when(processor).payloadFrom(req, null); processor.shouldProvideValidSignature(req, null);