Skip to content

Conversation

@Mutix
Copy link

@Mutix Mutix commented Nov 7, 2025

Fixes: JENKINS-76158, JENKINS-75977

Problem Description

When a new branch or tag is created and pushed to a remote repository, then immediately used to trigger a downstream Jenkins build using the git-parameter-plugin, the build fails with:

ERROR: Invalid parameter value: (StringParameterValue)

This occurs when:

  • Job A creates and pushes a new branch or tag v1.2.3
  • Job A immediately triggers Job B (downstream build via build step, API call, or other trigger) with parameter GIT_TAG=v1.2.3
  • The validation in Job B fails because the cache doesn't contain the newly created tag

Root Cause Analysis

Background: SECURITY-3419

SECURITY-3419 added parameter validation to prevent arbitrary values from being injected. The validation uses a cached set of allowed values (allowedValues field) that is populated by fetching tags/branches from the remote git repository.

Stale cache problem

The original implementation only refreshed the cache when it was null, but not when a value was not found in an existing cache.

Cache clearing behavior observed:

  • CLI invocation: createValue(CLICommand) explicitly sets allowedValues = null to force cache refresh
  • Stapler requests (web UI, downstream builds, API): createValue(StaplerRequest) does not appear to clear the cache

Understanding the cache population for Stapler requests:
The original comment stated: "Cache does not need to be cleared when invoked through Stapler because generateParamList is called to fill the cache"

Based on my investigation, this appears to be misleading:

  • generateParamList() is called by generateContents() (which populates the cache)
  • generateContents() is called by doFillValueItems() - which appears to be the UI form rendering endpoint
  • For downstream builds and API calls, it seems no UI form is rendered, so doFillValueItems() may not be called
  • Therefore, createValue(StaplerRequest) does not appear to call generateParamList() to populate the cache
  • This suggests the original implementation lacked a mechanism to refresh allowed values for Stapler requests

Note

I may be misunderstanding the flow here, so feedback is welcome!

The problem scenario:

  1. Job B's cache is populated at some point with tags [v1.2.2]
  2. Job A creates and pushes a new tag v1.2.3
  3. Job A triggers Job B (downstream build) with parameter GIT_TAG=v1.2.3
  4. createValue(StaplerRequest) is called → cache appears not to be cleared
  5. isValid() is called → cache is NOT null, so it's not refreshed
  6. allowedValues.contains("v1.2.3") returns false → validation fails
  7. Build fails with "Invalid parameter value" error

The Fix

Modified isValid() to implement a two-tier validation approach that refreshes the cache when a value is not found.

Key improvements:

  1. Cache hit: If value is found in cache, return immediately (performance optimization)
  2. Cache miss: If value NOT found in cache (or cache is null), refresh from git and check again
  3. Handles stale cache: If cache exists but doesn't contain the value, we refresh it
  4. Handles new branches/tags: Newly created branches/tags will trigger a cache refresh and be validated correctly

Alternative considered:
Clearing the cache for Stapler requests (like CLI does with allowedValues = null) was considered, but the cache refresh on miss approach was chosen to avoid unnecessary git fetches on every build. This way, cache hits remain fast while cache misses automatically refresh.

Also updated the comment in createValue(CLICommand) to clarify that isValid() handles cache refresh when needed.

Compatibility

  • Backwards compatible: No API changes, no configuration changes, only internal validation logic
  • Jenkins core: No minimum version change required
  • Security: Maintains SECURITY-3419 protections
  • Performance: Fast path ensures cached values are still fast, slow path only triggers when needed

Escape Hatch

Users experiencing issues can disable validation using the system property:

-Dnet.uaznia.lukanus.hudson.plugins.gitparameter.GitParameterDefinition.allowAnyParameterValue=true

Testing done

Manual Testing with Local Jenkins Instance

Tested with mvn hpi:run using a realistic CI/CD scenario where one job creates a git tag and immediately triggers a downstream build.

Test Repository: https://github.com/Mutix/jenkins-git-param-testing

This repository contains:

  • README.md - Complete step-by-step testing guide
  • JobA-CreateTag.jenkinsfile - Pipeline that creates new tags and triggers JobB
  • JobB-UseTag.jenkinsfile - Pipeline with git parameter that validates tags
  • FAILING_SCENARIO_OLD_CODE.md - Documentation of the bug with console output
  • SUCCESS_SCENARIO_WITH_FIX.md - Documentation of the fix working with console output

Test Scenario (Reproduces the Bug):

  1. Run JobA (first time) - Creates tag and triggers JobB → ✅ Succeeds (no cache exists yet)
  2. Open JobB in Jenkins UI and view the git parameter dropdown → This populates the cache
  3. Run JobA again (second time) - Creates a NEW tag and triggers JobB
    • Tag is successfully created and pushed to GitHub
    • JobB is triggered with the new tag as parameter
    • This is where the bug occurs - cache exists but doesn't contain the new tag

Results:

  • Without fix: JobB fails with hudson.AbortException: Invalid parameter value: (StringParameterValue) GIT_TAG='test-tag-1762473037224' - cache is stale and doesn't contain the newly created tag
    • See FAILING_SCENARIO_OLD_CODE.md for complete console output
  • With fix: JobB succeeds - cache is refreshed when tag not found in existing cache, tag is validated successfully
    • See SUCCESS_SCENARIO_WITH_FIX.md for complete console output
  • Security maintained: Invalid/non-existent tags are still rejected
  • All existing tests pass: mvn clean test

To reproduce the test:

  1. Fork the test repository: https://github.com/Mutix/jenkins-git-param-testing
  2. Follow the instructions in README.md
  3. Compare the failing scenario (old code) vs success scenario (with fix)

Automated Testing

Existing tests continue to pass: mvn clean test

Note on adding new tests:
Attempted to add unit tests using reflection to simulate a stale cache scenario, but discovered that the test infrastructure triggers the CLI code path (createValue(CLICommand)), which already clears the cache before validation (line 183). The bug specifically affects the Stapler request flow (createValue(StaplerRequest2)) used by downstream builds and API triggers, which is more complex to test in isolation. Open to suggestions for testing the Stapler request flow if maintainers have preferred patterns for this.

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

Refresh cache from git when a parameter value is not found in the
existing cache, not just when cache is null. This fixes validation
failures when downstream builds use newly created branches/tags.

Maintains SECURITY-3419 protections and backwards compatibility.
@Mutix Mutix requested a review from a team as a code owner November 7, 2025 00:33
@Mutix
Copy link
Author

Mutix commented Nov 11, 2025

Hi @MarkEWaite, look forward to your feedback and progressing this through the workflow

@MarkEWaite
Copy link
Contributor

Hi @MarkEWaite, look forward to your feedback and progressing this through the workflow

Thanks for your submission @Mutix . I'm deeply involved in some other issues at the moment and probably won't be able to review and merge this for at least 2 weeks.

@Mutix
Copy link
Author

Mutix commented Nov 11, 2025

Thanks, appreciate it. Let me know if there's anything else I can do to help 👍

@MarkEWaite
Copy link
Contributor

Thanks, appreciate it. Let me know if there's anything else I can do to help 👍

I'm not a very active user of the plugin. I'd love to have you adopt the plugin if you are an active user of the plugin and are willing to adopt it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants