Fix/amp 31205/pidc security test report fix

[brianbrix]

No description provided.

[Copilot]

🟡 Changes recommended

The HttpOnly language cookie breaks external-app language persistence via document.cookie.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Hardens cookie and Tomcat security settings while adding deployment runtime validation.

Changes:

• Secures language cookies and enforces SameSite behavior.
• Updates Tomcat and proxy/server configuration.
• Validates runtime files during builds and deployments.

File summaries

File	Summary
amp/src/main/java/org/digijava/kernel/util/DgUtil.java	Adds cookie security flags. Moderate issue (3 votes): HttpOnly prevents external apps from persisting the language preference.
amp/Dockerfile	Updates the Tomcat base image and validates runtime files.
amp/docker/server.xml	Configures forwarded ports and server identity.
amp/context.xml	Enables HttpOnly and strict SameSite cookies.
.github/workflows/deploy.yml	Adds runtime validation for reused and newly built images.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate security and compatibility issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

amp/Dockerfile:236

• This replaces the patch-pinned tomcat:8.5.79-jdk8 runtime with a floating 8.5 tag. Rebuilding the same commit can silently change Tomcat, the JDK, and the base OS, making deployments non-reproducible and potentially reintroducing the security findings this change is intended to address. Pin a tested patch tag or image digest.

FROM tomcat:8.5-jdk8-temurin

amp/Dockerfile:236

• This still selects the Tomcat 8.5 line, which is end-of-life, so replacing 8.5.79 with the floating 8.5 tag does not provide an ongoing security-supported runtime. Please move to a supported Tomcat line (Tomcat 9 is Java 8-compatible) and pin the approved patch version or digest rather than relying on a mutable tag.

FROM tomcat:8.5-jdk8-temurin

amp/context.xml:9

• Changing this to strict breaks the existing cross-site login handoff. The target site sets digi_session_id before redirecting to the configured login site, then the login site redirects back to /um/newSession; NewSession requires that cookie and returns noSessionCookie.jsp when it is absent. A cross-site top-level return navigation does not send a SameSite=Strict cookie, so users authenticating through a separate login site can no longer log in. Keep lax for this flow or redesign the handoff so the return does not depend on a Strict cookie.

    <CookieProcessor sameSiteCookies="strict" />

amp/src/main/java/org/digijava/kernel/util/DgUtil.java:268

• digi_language is also written from client-side code in TEMPLATE/ampTemplate/dashboard/external.js:40 and TEMPLATE/ampTemplate/gisModule/external.js:40. Once the server has set this cookie with HttpOnly, the browser will reject those document.cookie assignments, so subsequent ?language=... links into either SPA can leave the server/session using the old language. Keep this preference cookie script-writable, or replace the client-side update with a server-side language-setting endpoint before enabling HttpOnly.

        cookie.setHttpOnly(true);

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The Strict SameSite policy can break SSO, while authentication cookies and the Tomcat version still require security fixes.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

amp/Dockerfile:236

• This upgrades only to Tomcat 8.5.94, but the Tomcat 8.5 line reached end of life in 2024 and no longer receives security fixes. Since this PR is intended to address a security report, the image should move to a currently supported Tomcat line compatible with Java 8 (for example Tomcat 9) and validate the application migration.

FROM tomcat:8.5.94-jdk8-temurin-focal

amp/context.xml:3

• This Context setting hardens the container-managed session cookie, but it does not add HttpOnly or Secure to the application's manually-created authentication cookies. HttpLoginManager.saveSessionIdToCookie emits the digi_session_id used by autoLogin, and saveLoginCookie emits the remember-me credential cookie, without either flag; the security report can therefore still expose authentication material to JavaScript and HTTP. Set the attributes when those cookies are created (using the trusted HTTPS state for Secure).

<Context path="/" debug="0" reloadable="false" unloadDelay="4000" swallowOutput="true" useHttpOnly="true"

amp/src/main/java/org/digijava/kernel/util/DgUtil.java:268

• digi_language is also explicitly written by browser code in TEMPLATE/ampTemplate/dashboard/external.js and TEMPLATE/ampTemplate/gisModule/external.js via document.cookie. Once this cookie is marked HttpOnly, those existing ?language= entry-page writes can no longer modify the cookie, so the client-side language hand-off is broken or can leave conflicting cookies. Keep this non-sensitive preference cookie script-writable, or migrate those integrations to the server endpoint before enabling HttpOnly.

        cookie.setHttpOnly(true);

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Authentication cookie protections remain incomplete, and the Tomcat runtime is still end-of-life.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

amp/Dockerfile:236

• This updates the image but remains on the Tomcat 8.5 line (8.5.94), which is end-of-life and no longer receives security fixes. A security-report remediation based on this tag will drift back into known vulnerabilities; move to a supported Tomcat line compatible with the application's Java level, or document why the EOL runtime is unavoidable.

FROM tomcat:8.5.94-jdk8-temurin-focal

amp/context.xml:3

• useHttpOnly here only affects Tomcat-managed session cookies; it does not flag the custom authentication cookies emitted by HttpLoginManager.saveSessionIdToCookie and saveLoginCookie. The former is the bearer session token and the latter contains base64-encoded credentials, yet both are still emitted without HttpOnly/Secure, so this remediation leaves authentication cookies script-readable and usable over HTTP. Apply the flags at those cookie emission sites as well.

<Context path="/" debug="0" reloadable="false" unloadDelay="4000" swallowOutput="true" useHttpOnly="true"

amp/context.xml:9

• Applying SameSite=Strict at the Context level also affects the authentication cookies, including JSESSIONID and the custom digi_session_id. Browsers omit these cookies on cross-site top-level navigations, so an authenticated user following a link from another site can arrive without their session and be assigned a new one; the previous Lax setting intentionally preserved those GET navigations. Keep Lax for the auth cookies or verify this regression is acceptable and scope Strict more narrowly.

    <CookieProcessor sameSiteCookies="strict" />

amp/src/main/java/org/digijava/kernel/util/DgUtil.java:269

• This adds HttpOnly only to the server-generated digi_language cookie, but both existing external entry points call /rest/translations/languages/{lang} and then execute document.cookie = "digi_language=...;path=/". That client-side assignment can create or replace a same-name cookie without HttpOnly/Secure, so these flows still leave an unprotected cookie and undermine the guarantee added here. Move the cookie update into the server response or remove the client-side assignment and use the flagged response cookie.

        cookie.setHttpOnly(true);
        cookie.setSecure(request.isSecure());

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite