Allow URL-specific avatar image sources in CSP#26021
Allow URL-specific avatar image sources in CSP#26021somiljain2006 wants to merge 14 commits intojenkinsci:masterfrom
Conversation
MarkEWaite
left a comment
There was a problem hiding this comment.
Several review comments that need to be addressed. Once those are addressed, I'd like to ask GitHub Copilot to review the pull request as well.
There was a problem hiding this comment.
Pull request overview
This pull request introduces URL-level allowlisting for avatar images in Content Security Policy (CSP), enabling plugins to allowlist specific avatar image URLs rather than just entire domains. This provides more granular control over CSP img-src directives while maintaining backward compatibility with the existing domain-based approach.
Key changes:
- Added a new
allowUrl()API method for URL-specific CSP allowlisting - Implemented
normalizeUrl()to canonicalize URLs for consistent CSP entries - Added comprehensive test coverage for URL normalization edge cases
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| core/src/main/java/jenkins/security/csp/AvatarContributor.java | Adds allowUrl() API, normalizeUrl() URL canonicalization logic, and infrastructure to store/apply URL-specific CSP sources alongside existing domain-based sources |
| core/src/test/java/jenkins/security/csp/AvatarContributorTest.java | Adds comprehensive unit tests for URL normalization covering IPv6, IDN, port handling, fragment stripping, credential rejection, and deduplication |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@MarkEWaite, could you please review the updated code? I’ve addressed the feedback and pushed the latest changes. |
|
@strangelookingnerd, I have added the required test. Can you review it? |
test/src/test/java/jenkins/security/csp/AvatarContributorIntegrationTest.java
Outdated
Show resolved
Hide resolved
test/src/test/java/jenkins/security/csp/AvatarContributorIntegrationTest.java
Outdated
Show resolved
Hide resolved
test/src/test/java/jenkins/security/csp/AvatarContributorIntegrationTest.java
Outdated
Show resolved
Hide resolved
test/src/test/java/jenkins/security/csp/AvatarContributorIntegrationTest.java
Outdated
Show resolved
Hide resolved
|
@strangelookingnerd @daniel-beck Can you review the PR? I have applied the required fix. |
Fixes #23888
Introduce URL-level allowlisting for avatar images to enable more targeted img-src CSP directives. This preserves the existing domain-based behavior while providing an opt-in API for plugins that can determine exact avatar URLs.
Testing done
Ensured all tests still passed.
Proposed changelog entries
Allow plugins to allowlist exact avatar image URLs in Content Security Policy.
Proposed changelog category
/label rfe
Proposed upgrade guidelines
N/A
Desired reviewers
@jenkinsci/core-pr-reviewers, @daniel-beck, @MarkEWaite, @strangelookingnerd
Before the changes are marked as
ready-for-merge:Maintainer checklist
upgrade-guide-neededlabel is set, and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidateto be considered.