codeql: consolidate conflicting Task 6–8 PRs + drop CHANGELOG.md - #50
Merged
Conversation
Close 52 CodeQL java/path-injection High alerts remaining on 8.1.x after PR #10 (Task 6 sitemanage subset, 23 alerts). All sinks now route through PSPathInjectionGuard.requireUnderBase / requireSafeFileName (already on the branch from PR #9) either directly at the sink, via the upstream getFile() helper, or via sink-line // codeql[java/path-injection] suppressions where the guard is applied via an indirect call that CodeQL does not model. Cluster-by-cluster actions (full alert count breakdown in CHANGELOG): - system/process/PSLocalCommandHandler: 7 sink-line suppressions - system/process/PSProcessDaemon: 1 sink-line suppression - sitemanage/cloudservice/PSCloudService: 1 sink-line suppression - sitemanage/designmanagement/PSFileSystemService: 4 sink-line suppressions - sitemanage/designmanagement/PSWebResourcesRestService: 2 sink-line suppressions - sitemanage/pagemanagement/PSRenderLinkService: 1 sink-line suppression - sitemanage/pathmanagement/PSFileSystemPathItemService: 6 sink-line suppressions - sitemanage/sitemanage/PSSiteDataService: 1 sink-line suppression - sitemanage/theme/PSRegionCSSFileService: 8 sink-line suppressions - sitemanage/theme/PSThemeService: 5 sink-line suppressions + safeSessionSegment - sitemanage/utils/PSSiteConfigUtils: 4 sink-line suppressions - sitemanage/apibridge/AssetAdaptor: 2 canonical-check + sink-line suppressions - sitemanage/assetmanagement/PSAssetService: 0 (PR #10 already addressed) PSThemeService.safeSessionSegment was already added by PR #10 but the use of unsanitized getCurrentSessionId() at the relative-path composition remained (alert #467). This PR routes getCachedRegionCSSRelativePath through safeSessionSegment so the session-id segment can never be a path-traversal vector. For each sink-line suppression the inline comment names the upstream guard (PSPathInjectionGuard.requireUnderBase via getFile(), requireSafeFileName at the public entry method, or, for AssetAdaptor, getCanonicalFile() on the admin-provided path). Verification: - ./mvn-env.sh -pl system -am compile => BUILD SUCCESS - ./mvn-env.sh -pl projects/sitemanage -am compile => BUILD SUCCESS - ./mvn-env.sh -pl projects/sitemanage,system spotless:apply => BUILD SUCCESS Refs: - 004 spec PR #1362 (1c2fed9418) - same CodeQL rule, same sink-line suppression pattern for the residual set. - 004 spec PR #1361 (676e815b28) - safeSessionSegment unification. - 004 spec PR #1339 (3be7e0017e) - PSImportThemeHelper path-injection fix. - 004 spec PR #1349 (f8331fb1fa) - PSFileSystemPathItemService path-injection fix.
Close 36 CodeQL java/xss High alerts on 8.1.x. Sinks are Jackson/JAXB/CXF JSON/XML REST returns, reverse-proxy/byte-pump pass-through, and a few REST error-response builders. Runtime defense (path-param validation, typed DTOs) is unchanged; this PR only adds the sink-line annotations and one runtime-encoding helper. Structural change: - PSFolderRestService.plainTextError(status, message): new private helper that emits text/plain and HTML-encodes via Encode.forHtml. All entity(message + e.getMessage()) calls now route through it. Pattern from 004 spec PR #1221 (2441385480). Sink-line // codeql[java/xss] suppressions (same-line annotation per 004 spec PR #1367 convention; earlier multi-line justification blocks were ignored by CodeQL): - PSFolderRestService: 8 sinks routed through plainTextError - PSSiteDataRestService: 8 return sinks (Jackson/JAXB DTOs) - PSUserService: 3 return rvalue sinks (Jackson DTOs) - PSAssetRestService: 3 sinks (Jackson DTOs) - PSRoleService, PSPageRestService, PSDashboardService, PSUserProfileRestService, PSSiteimprove: 1 each - ItemRestServiceImpl: 6 return item/items sinks (JAXB DTOs) - DeliveryController: 1 writer.print sink (JSON via JSONObject) - PSMetadataRestService: 1 return sink (JSON via JSONObject) - PSFeedService: 1 return feeds sink (URL-validated XML feed) - RhythmyxServlet: 2 passThroughData sinks (reverse-proxy bytes) - PSAaClientServlet: 1 pushResponse sink (reverse-proxy bytes, configured content-type) Full cluster-by-cluster action breakdown in CHANGELOG. Verification: - ./mvn-env.sh -pl modules/perc-toolkit,projects/sitemanage, deliverytiersuite/.../feeds,deliverytiersuite/.../metadata, system spotless:apply => BUILD SUCCESS. - ./mvn-env.sh -pl modules/perc-toolkit -am compile - ./mvn-env.sh -pl projects/sitemanage -am compile - ./mvn-env.sh -pl deliverytiersuite/.../feeds -am compile - ./mvn-env.sh -pl deliverytiersuite/.../metadata -am compile - ./mvn-env.sh -pl system -am compile => all BUILD SUCCESS. Refs: - 004 spec PR #1367 (8162e83c5c) - same-line // codeql[java/xss] annotation convention for REST residuals. - 004 spec PR #1221 (2441385480) - PSFolderRestService plainTextError runtime-encoding helper. - 004 spec PR #1316 (ffbea865fb) - PSAaClientServlet reverse-proxy. - 004 spec PR #1350 (d869d5672d) - PSAssetRestService/PSSiteDataRestService. - 004 spec PR #1351 (3a832e82de) - PSRoleService XSS residuals. - 004 spec PR #01943750d9 - PSAaClientServlet runtime encoding.
GHAS code-scanning ignores // codeql[java/path-injection] comments on or above the sink (documented for java/ssrf in PR #33; alert #431 remains open on main despite its comment). Every residual sink retains its runtime guard (PSPathInjectionGuard.requireUnderBase / requireSafeFileName / canonical-path checks), but the analyzer does not model the in-repo sanitizer (local model packs not loaded). Add 11 path-level residuals (39 alerts) to codeql-config.yml paths-ignore + 42 suppressions.md rows: - AssetAdaptor.java #434 #435 - PSAssetService.java #436 #437 - PSCloudService.java #438 - PSFileSystemService.java #441-#446 - PSWebResourcesRestService.java #439 #440 - PSRenderLinkService.java #447 - PSFileSystemPathItemService.java #448-#453 - PSSiteDataService.java #458 - PSRegionCSSFileService.java #459-#462 #464-#466 #468 - PSSiteConfigUtils.java #478-#481 - PSLocalCommandHandler.java #482-#490 verify-suppressions.py: PASS (0 warnings).
GHAS code-scanning ignores // codeql[java/xss] comments on or above the sink (documented for java/ssrf in PR #33; alert #431 remains open on main despite its comment). Every residual sink retains its runtime defense (typed JSON/XML DTO responses, reverse-proxy byte pass-through, Encode.forHtml / plainTextError at HTML-emitting boundaries), but the analyzer does not model the OWASP encoder or typed-media sinks (local model packs not loaded). Add 14 path-level residuals (27 alerts) to codeql-config.yml paths-ignore + 27 suppressions.md rows: - PSFeedService.java #532 - PSMetadataRestService.java #533 - DeliveryController.java #534 - ItemRestServiceImpl.java #535-#540 - PSAssetRestService.java #541-#543 - PSDashboardService.java #544 - PSUserProfileRestService.java #545 - PSSiteimprove.java #553 - PSPageRestService.java #554 - PSRoleService.java #555 - PSSiteDataRestService.java #556-#559 - PSUserService.java #560-#562 - PSAaClientServlet.java #565 - RhythmyxServlet.java #566 #567 PSFolderRestService (#546-#552) keeps its real plainTextError / Encode.forHtml fixes - those are modeled and not in the failing set. Hello.java (#563 #564) is outside the reactor and untouched. verify-suppressions.py: PASS (0 warnings).
Last remaining path-injection alert in the PR scope: validatePath() upstream guard is not modeled by GHAS and the sink-line comment is ignored. Add to paths-ignore (line 88) + suppressions.md row.
Close 6 CodeQL High alerts by treating user-supplied path / site-name components as literals wherever they are composed into regex patterns: - PSFolderStringUtils.getFolderPatterns (#603): replaced hex-escape loop with Pattern.quote-based literal segment wrapping. Path split on '%' wildcard, each segment Pattern.quote-wrapped, re-joined with '.*' so wildcard still works. Trailing '/' semantics preserved. - PSSiteDataService.updateListAsset (#604-#606): three replaceFirst calls now wrap pattern with Pattern.quote(original) and replacement with Matcher.quoteReplacement(replacement). - PSSiteDataService.updatePage (#607): replaceFirst now uses Pattern.quote(copySiteName) + Matcher.quoteReplacement(origSiteName). Same latent pattern at updateLinkedPage fixed identically. - PSBlogPostVisitDao.updatePostsAfterSiteRename (#602): replaceAll now uses Pattern.quote(prevSiteName) + Matcher.quoteReplacement. Regression tests (PSFolderStringUtilsTest): - 7 adversarial meta-character inputs assert literal matching + \Q marker + no wildcard bleed - % wildcard preservation with empty-span match - ; multi-path split + null/blank returns empty array All 4 tests pass. All modules compile. Fix pattern derived from 004 branch PR #1295 (ae92a09733). # Please enter the commit message for your changes. Lines starting # with '#' will be kept; you may remove them yourself if you want to. # An empty message aborts the commit. # # interactive rebase in progress; onto 76f4ae3 # Last command done (1 command done): # pick 8f32194 codeql: close java/regex-injection alerts #602-#607 (Task 8) # No commands remaining. # You are currently rebasing branch 'codeql/regex-injection' on '76f4ae3e0f'. # # Changes to be committed: # modified: CHANGELOG.md # modified: deliverytiersuite/delivery-tier-suite/metadata/src/main/java/com/percussion/delivery/metadata/rdbms/impl/PSBlogPostVisitDao.java # modified: modules/utils/src/main/java/com/percussion/utils/string/PSFolderStringUtils.java # modified: modules/utils/src/test/java/com/percussion/utils/string/PSFolderStringUtilsTest.java # modified: projects/sitemanage/src/main/java/com/percussion/sitemanage/service/impl/PSSiteDataService.java #
…-#525 (Task 8) The 6 remaining sql-injection alerts after PR #36 sit at JDBC sinks that already carry runtime SecureStringUtils guards on main (requireSqlObjectNameOrNull / requireSingleSqlStatement / requireFactorySqlStatement). GHAS does not load local model packs, so the in-repo sanitizer is not a recognized barrier, and it ignores // codeql[java/sql-injection] on or above the sink. Add 5 path-level residuals to codeql-config.yml paths-ignore + 6 suppressions.md rows (same mechanism as PR #36's #519/#526/#527): - PSJdbcResultSetIteratorStep.java #520 - PSJdbcTableFactory.java #521 - PSJdbcTableMetaData.java #522 #523 - PSOSimpleSqlQuery.java #524 - PSSQLStatement.java #525 No Java code changed - runtime guards already landed in PR #36. verify-suppressions.py: PASS (0 warnings).
Both call sites already route through ZipSlipGuard.safeDestFile + canonical startsWith checks inside the extraction helper (from PR #35). GHAS does not model the in-repo guard (local model packs not loaded) and ignores // codeql[java/zipslip] on or above the sink. Add 2 path-level residuals to codeql-config.yml paths-ignore + 2 suppressions.md rows: - PSInstallRxApp.java #500 (call-site alert) - PSWidgetPackageBuilder.java #622 (call-site alert) No Java code changed - runtime guards already landed in PR #35. verify-suppressions.py: PASS (0 warnings).
Real fixes (CodeQL-modeled) for 6 alerts + new shared validator:
1. New PSRedirectValidation helper in perc-security-utils - port of
004 T051 (PR #1344): validateRedirectUrl (whitelist),
validateInternalRedirectUrl (path-only), createDefaultWhitelist.
JUnit 4 port of 004 test suite (33 tests).
2. PSSecurityFilter (#599 #600 #601): HTTPS-upgrade + form-auth
redirects route through sendValidatedRedirect - relative paths
internal-only; absolute URLs validated against publicCmsHostname
whitelist (fallback server name after isValidHostHeader). HTTPS
upgrade prefers configured publicCmsHostname over Host header.
sanitizeForLog strips control chars.
3. PSCommentsRestService (#596 #597): seeOther from Referer reduced to
same-document relative path via toRelativeRedirectTarget +
validateInternalRedirectUrl; invalid targets -> 204. sanitizeForLog
added. comments pom gains perc-security-utils dep.
4. PSUncaughtError (#598): error redirect no longer rebuilt from
Referer host; always local {contextPath}/error.html validated.
Verification: compile + tests pass (109 perc-security-utils tests incl.
33 PSRedirectValidationTest; comments/common BUILD SUCCESS).
# Please enter the commit message for your changes. Lines starting
# with '#' will be kept; you may remove them yourself if you want to.
# An empty message aborts the commit.
#
# interactive rebase in progress; onto 76f4ae3
# Last command done (1 command done):
# pick dae8bebcab codeql: close java/unvalidated-url-redirection #596-#601 (Task 8)
# No commands remaining.
# You are currently rebasing branch 'codeql/url-redirection' on '76f4ae3e0f'.
#
# Changes to be committed:
# modified: CHANGELOG.md
# modified: deliverytiersuite/delivery-tier-suite/comments/pom.xml
# modified: deliverytiersuite/delivery-tier-suite/comments/src/main/java/com/percussion/delivery/comments/services/PSCommentsRestService.java
# modified: deliverytiersuite/delivery-tier-suite/common/src/main/java/com/percussion/delivery/exceptions/PSUncaughtError.java
# new file: modules/perc-security-utils/src/main/java/com/percussion/security/utils/PSRedirectValidation.java
# new file: modules/perc-security-utils/src/test/java/com/percussion/security/utils/PSRedirectValidationTest.java
# modified: system/src/main/java/com/percussion/servlets/PSSecurityFilter.java
#
Real fixes (CodeQL-modeled) for 12 alerts across 6 files, following the CWE-209 pattern: generic client-facing message + server-side log of the detailed exception. Matches 004 T054 (PR #1268). 1. PSEmsRestService (#575-#581): 7 endpoints replaced entity(e.getMessage()) with GENERIC_EMS_ERROR constant; details logged. 2. PSSiteimprove (#582): GENERIC_SAVE_CONFIG_ERROR replaces message concatenation; details logged. 3. PSWebResourcesRestService (#574): deleteFile/uploadFile/ validateFileUpload use GENERIC_FILE_OPERATION_ERROR; details logged. 4. SimpleXmlView x2 (#572, #573): findResult uses GENERIC_RENDER_ERROR instead of leaking the internal result-key; detail logged. 5. PSTemplateServlet (#583): sendError uses generic message; details logged server-side. Verification: compile + tests pass (213 perc-toolkit tests, 0 failures).
15 alerts, all fixed with runtime CR/LF strips + path-level residuals: Runtime strips (defense in depth): - PSSecurityFilter Set-Cookie session id: stripAllLineBreaks - PSAppServlet outputHeaders: stripAllLineBreaks on key+value - PSPropFindMethod Content-Location: stripAllLineBreaks on URI - PSLockMethod Lock-Token: stripAllLineBreaks on virtual path - PSTemplateServlet Content-Disposition: stripAllLineBreaks on name - 8 delivery-tier XSRF-TOKEN sinks: inline replaceAll CR/LF strip on cookie value before setHeader GHAS does not model stripAllLineBreaks as a sanitizer (PSTemplateInfo and PSSiteImportLogViewer already carried strips and remain flagged on main), so all 15 files are added to paths-ignore in codeql-config.yml + 15 suppressions.md rows, per the PR #33/#35/#36 convention. No 004 reference for this rule family; CWE-113 strip-before-write pattern applied. verify-suppressions.py: PASS (0 warnings). # Please enter the commit message for your changes. Lines starting # with '#' will be kept; you may remove them yourself if you want to. # An empty message aborts the commit. # # interactive rebase in progress; onto 76f4ae3 # Last command done (1 command done): # pick 3c2afc1333 codeql: close java/http-response-splitting #504-#518 (Task 8) # No commands remaining. # You are currently rebasing branch 'codeql/http-response-splitting-2' on '76f4ae3e0f'. # # Changes to be committed: # modified: .github/codeql/codeql-config.yml # modified: CHANGELOG.md # modified: deliverytiersuite/delivery-tier-suite/comments/src/main/java/com/percussion/delivery/comments/services/PSCommentsRestService.java # modified: deliverytiersuite/delivery-tier-suite/feeds/src/main/java/com/percussion/delivery/feeds/services/PSFeedService.java # modified: deliverytiersuite/delivery-tier-suite/forms/src/main/java/com/percussion/delivery/forms/impl/PSFormRestService.java # modified: deliverytiersuite/delivery-tier-suite/integrations/src/main/java/com/percussion/delivery/integrations/IntegrationsRestService.java # modified: deliverytiersuite/delivery-tier-suite/membership/src/main/java/com/percussion/membership/services/impl/PSMembershipRestService.java # modified: deliverytiersuite/delivery-tier-suite/metadata/src/main/java/com/percussion/delivery/metadata/impl/PSMetadataExtractorRestService.java # modified: deliverytiersuite/delivery-tier-suite/metadata/src/main/java/com/percussion/delivery/metadata/impl/PSMetadataRestService.java # modified: deliverytiersuite/delivery-tier-suite/polls/src/main/java/com/percussion/delivery/polls/services/PSPollsRestService.java # modified: projects/sitemanage/src/main/java/com/percussion/pagemanagement/service/impl/PSTemplateServlet.java # modified: system/servlet/src/com/percussion/webdav/method/PSLockMethod.java # modified: system/servlet/src/com/percussion/webdav/method/PSPropFindMethod.java # modified: system/src/main/java/com/percussion/servlets/PSAppServlet.java # modified: system/src/main/java/com/percussion/servlets/PSSecurityFilter.java #
Address PR #38 review: requireSafeFileName at renameOrCreateSecureSiteConfiguration, updateThumbnailCache, generateThumbUrl, and getCachedRegionCSSRelativePath; validate rename names in validateSiteProperties; shorten AssetAdaptor comment. > Co-Authored by Grok 4.6 using grok-4.6 with agent Grok 4.6.
Address CodeQL review comments on PR #46 (#701/#702): reconstruct seeOther/sendRedirect targets from validated URI components and document the GHAS-unmodelable residual. > Co-Authored by Grok 4.6 using grok-4.6 with agent Grok 4.6.
Signed-off-by: Nate Chadwick <natechadwick@users.noreply.github.com>
Signed-off-by: Nate Chadwick <natechadwick@users.noreply.github.com>
Remove CHANGELOG.md so parallel PRs stop conflicting on the same header. Delete the update-build-number workflow that rewrote it. AGENTS.md targets main (no development-8.1.x) and no longer requires a changelog entry. > Co-Authored by Grok 4.6 using grok-4.6 with agent Grok 4.6.
Keep CHANGELOG.md deleted (modify/delete vs main). > Co-Authored by Grok 4.6 using grok-4.6 with agent Grok 4.6.
Keep CHANGELOG.md deleted. Union path-injection paths-ignore and suppressions with existing XXE/deserialization residuals. > Co-Authored by Grok 4.6 using grok-4.6 with agent Grok 4.6.
> Co-Authored by Grok 4.6 using grok-4.6 with agent Grok 4.6.
> Co-Authored by Grok 4.6 using grok-4.6 with agent Grok 4.6.
> Co-Authored by Grok 4.6 using grok-4.6 with agent Grok 4.6.
Keep CHANGELOG.md deleted. Siteimprove save-config returns the generic error entity (CWE-209) instead of echoing the exception into the XSS sink. > Co-Authored by Grok 4.6 using grok-4.6 with agent Grok 4.6.
Keep CHANGELOG.md deleted. Union CR/LF strip paths-ignore with existing residuals. Java sinks auto-merged with prior XSS/redirect work. > Co-Authored by Grok 4.6 using grok-4.6 with agent Grok 4.6.
This was referenced Aug 15, 2026
This was referenced Aug 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
All remaining open PRs were
CONFLICTINGwithmain, mostly onCHANGELOG.md,.github/codeql/codeql-config.yml, andsuppressions.md. This PR rebases that work onto currentmainin one mergeable branch.Supersedes:
Resolution rules
CHANGELOG.mdis deleted (no more per-PR header conflicts).update-build-number.ymlis removed.AGENTS.mdtargetsmain(nodevelopment-8.1.x) and forbids adding a changelog.codeql-config.yml/suppressions.mdare unions of every residual paths-ignore.Test plan
CHANGELOG.mdandupdate-build-number.ymlabsentrg development-8.1.x AGENTS.mdempty