Skip to content

feat: add Jenkins plugin with Kerberos/SAML SSO authentication#32

Open
rjeffman wants to merge 2 commits into
LeGambiArt:mainfrom
rjeffman:jenkins
Open

feat: add Jenkins plugin with Kerberos/SAML SSO authentication#32
rjeffman wants to merge 2 commits into
LeGambiArt:mainfrom
rjeffman:jenkins

Conversation

@rjeffman
Copy link
Copy Markdown
Collaborator

What's New

Jenkins Plugin

  • Browse jobs, folders, and views with health scores
  • Get build details, logs, and test results (JUnit)
  • Analyze pipeline stages with per-stage logs
  • Download and inspect artifacts
  • Smart caching (5min TTL, permanent for completed builds)
  • Automatic secret redaction (AWS keys, JWTs, passwords, connection strings)

Kerberos/SAML Authentication

  • Dynamic SPN derivation from request hostname
  • Automatic SAML POST binding detection and replay
  • Per-request domain allowlisting for SSO redirects
  • Enhanced config variable resolver: |hostname modifier, nested defaults

Tools: 15 tools total (4 primary, 11 deferred)

  • jenkins_list_jobs, jenkins_get_build, jenkins_get_build_log, jenkins_whoami
  • jenkins_get_pipeline_stages, jenkins_get_test_results, jenkins_list_artifacts, etc.

Changes

  • Add internal/proxy/saml.go - SAML SSO redirect handler
  • Add internal/auth/kerberos_provider.go - dynamic SPN support
  • Add plugins/jenkins/ - complete plugin with tests
  • Update golang.org/x/net to fix HTTP/2 vulnerability (GO-2026-4918)

@rjeffman rjeffman requested a review from sergio-correia May 13, 2026 21:23
@rjeffman rjeffman marked this pull request as ready for review May 14, 2026 02:30
Comment thread plugins/jenkins/plugin.yaml Outdated
Comment thread plugins/jenkins/plugin.yaml
Comment thread env.d/jenkins.env.example
@decko
Copy link
Copy Markdown
Collaborator

decko commented May 20, 2026

I think you need to rebase your work with current changes on wtmcp core @rjeffman. Let me know so we can help with the review of Jenkins plugin again

rjeffman added 2 commits May 20, 2026 11:47
Add read-only Jenkins plugin with pipeline results, build logs, and
test reports. Supports Kerberos/SPNEGO, API token, and bearer token
authentication with automatic secret scrubbing.

Core tools:
- jenkins_list_jobs: Browse jobs and folders with health scores
- jenkins_get_build: Detailed build info with artifacts and SCM changes
- jenkins_get_build_log: Console output with grep filter and secret redaction
- jenkins_whoami: Validate credentials and permissions

Extended tools:
- jenkins_get_pipeline_stages: Stage-level breakdown with durations
- jenkins_get_stage_log: Per-stage console output
- jenkins_get_test_results: JUnit XML parsing with stack traces
- jenkins_list_artifacts: List build artifacts
- jenkins_get_artifact: Download artifact files with secret scrubbing

Features:
- Smart caching with 5-minute TTL (permanent for completed builds)
- Automatic secret redaction (AWS keys, JWTs, passwords, connection strings)
- Build number aliases (lastBuild, lastFailedBuild, lastSuccessfulBuild)
- Folder navigation with slash-separated paths
- Log size limits (200 lines default, 5000 max) to prevent context overflow

Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Fix govulncheck failures in CI by upgrading from Go 1.25.9 to 1.25.10.

Vulnerabilities fixed:
- GO-2026-4982: Bypass of meta content URL escaping causes XSS
- GO-2026-4980: Escaper bypass leads to XSS in html/template

Both affect cmd/wtmcpctl/oauth.go template execution and are fixed
in Go 1.25.10.

Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Copy link
Copy Markdown
Contributor

@sergio-correia sergio-correia left a comment

Choose a reason for hiding this comment

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

Looks good overall. I pointed out a few issues that should be addressed.

if err:
return err

cache_key = "jenkins:whoami"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cache keys don't include the Jenkins URL, but the plugin accepts jenkins_url per-request. If someone queries two different Jenkins instances in the same session, the second call returns cached data from the first.

For example, jenkins:whoami is a static key shared across all instances. Same for jenkins:jobs:{folder_hash} when two servers have identically named folders (extremely common in enterprise).

Please add a URL-derived hash to every cache key. Something like:

url_hash = _cache_key_hash(handler._jenkins_url)
cache_key = f"jenkins:{url_hash}:whoami"

A _make_cache_key(*parts) helper that auto-includes the URL hash would prevent regressions across all 15 tools.

- JENKINS_USER
- JENKINS_TOKEN
- JENKINS_AUTH_TYPE
- JENKINS_URL
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The JENKINS_URL env var fallback in _setup_url() reads from handler.config.get("JENKINS_URL"), but there is no config: section in this file. The core only populates handler.config from manifest.Config plus internal fields (_session_dir, _output_dir).

Without this, every user who sets JENKINS_URL in env.d/jenkins.env and calls a tool without passing jenkins_url explicitly will get an invalid jenkins_url: '' error. The fallback advertised in the tool descriptions never works.

Please add after the env: block:

config:
  jenkins_url: "${JENKINS_URL}"

Then update tools_read.py:31 to handler.config.get("jenkins_url", "") (lowercase, matching the config key).

return auth_error

# Scrub secrets
scrubbed_log = helpers.scrub_log_content(body)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The wfapi/log endpoint returns JSON like {"text": "<log content>", "length": N, "hasMore": false}. The proxy parses this into a dict, so body here is a dict, not a string. scrub_log_content() returns "" for non-string input, so this will always produce an empty log against a real Jenkins instance.

The tests mask this because they mock body as a plain string.

Please extract the text field before scrubbing:

if isinstance(body, dict):
    log_text = body.get("text", "")
elif isinstance(body, str):
    log_text = body
else:
    log_text = ""
scrubbed_log = helpers.scrub_log_content(log_text)

}
_send(msg)
resp = _recv()
return resp.get("status") == "ok"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The core returns {"ok": true, "count": N} for cache_flush, not {"status": "ok"}. This means cache_flush() always returns False, so jenkins_flush_cache always reports failure even though the cache was actually flushed.

Also, the namespace parameter is ignored by the core (it always uses the plugin's own namespace). The jira plugin's simpler pattern is the right reference here:

def cache_flush():
    _send({"id": _gen_id("cache"), "type": "cache_flush"})
    resp = _recv()
    return resp.get("ok", False)

"size_bytes": len(body),
}

handler.cache_set(cache_key, result, ttl=0)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This caches permanently with no checks, but every other tool guards against two things before caching: (1) whether the build is still running, and (2) whether build_number is an alias like lastBuild.

Without the running-build check, partially written artifacts get permanently cached. Without the alias check, lastBuild artifacts get frozen to whatever build was current at first fetch.

Please add the same pattern used in jenkins_get_build and jenkins_get_build_log:

build_path = f"/{helpers.job_path(job_name)}/{build_number}/api/json"
build_status, build_body, _ = handler.http("GET", build_path, query={"tree": "building"})
building = build_body.get("building", False) if build_status in (200, 201) else False
if not building:
    try:
        int(build_number)
        handler.cache_set(cache_key, result, ttl=0)
    except (ValueError, TypeError):
        pass

}
)

passed = total - failed - skipped
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The JUnit XML spec defines errors alongside failures on <testsuite>. Errors represent unexpected exceptions (distinct from assertion failures). This line computes passed = total - failed - skipped without accounting for errors.

A testsuite with tests="10" failures="1" errors="3" skipped="0" would report 9 passed when only 6 actually passed. The <error> child elements on testcases are also not scanned, so those test cases silently disappear from the results.

Please add suite_errors = int(testsuite.get("errors", 0)) and include it: passed = total - failed - errors - skipped. Also scan for <error> elements alongside <failure>.

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.

3 participants