improve startup scripts and logs#1236
Conversation
Replace fixed /tmp paths with mktemp-allocated files and directories. The private.yml copy is created with mode 0600 and registered with trap ... EXIT for automatic cleanup. SAVEDIR is also allocated with mktemp -d instead of a fixed path.
Switch uaa.client.redirect_uri.matching_mode default from legacy to exact, aligning with RFC 6749 and current best practice for OAuth2 redirect URI validation. BREAKING CHANGE: clients relying on subdomain or path wildcard matching must register exact redirect URIs or explicitly set uaa.client.redirect_uri.matching_mode: legacy in their manifest before upgrading.
Change uaa.logging_level default from DEBUG to INFO. The DEBUG level produces verbose Spring Security and JDBC output that is not appropriate for production deployments. Extend the log4j2 redaction pattern to cover code=, access_token=, refresh_token=, and id_token= in addition to the existing password= and client_secret= patterns.
configure_tomcat transfers ownership of /var/vcap/data/uaa/ to vcap, which includes cert-cache and the Java truststore within it. Add resecure_cert_cache() to run after configure_tomcat and configure_spring_boot. It restores cert-cache to root:root with mode 0711 and all enclosed files to root:root 0644, so the vcap process retains read access to the truststore without write access.
There was a problem hiding this comment.
Pull request overview
This PR updates UAA release operational defaults and hardening around release tooling, logging redaction, and startup-time file permissions, and introduces a breaking default change for redirect URI matching behavior.
Changes:
- Harden
perform-release.shtemp handling forprivate.ymlby usingmktempand anEXITcleanup trap. - Extend Log4j2 message redaction to cover additional OAuth-related parameters/tokens.
- Re-secure
cert-cacheownership/permissions after Tomcat/Spring Boot setup, and change defaults for logging level and redirect URI matching mode.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| scripts/perform-release.sh | Switches to mktemp for temp paths and securely stages private.yml across branch switches. |
| jobs/uaa/templates/config/log4j2.properties.erb | Extends the Log4j2 %replace redaction regex to redact more credential/token fields. |
| jobs/uaa/templates/bin/pre-start.erb | Adds resecure_cert_cache() and invokes it after configure_tomcat/configure_spring_boot to restore secure ownership/modes. |
| jobs/uaa/spec | Updates defaults (uaa.logging_level to INFO, redirect matching default to exact) and documents the breaking redirect-matching change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| SAVEDIR=$(mktemp -d) | ||
| RELEASES=$SAVEDIR/releases | ||
| FINAL_BUILDS=$SAVEDIR/.final_builds |
| # we save private.yml to a secure temp file so it survives branch switches | ||
| # and is cleaned up automatically on exit. | ||
| PRIVATE_YML_COPY=$(mktemp) | ||
| chmod 0600 "${PRIVATE_YML_COPY}" | ||
| trap 'rm -f "${PRIVATE_YML_COPY}"' EXIT | ||
|
|
| %> | ||
| property.log_directory = /var/vcap/sys/log/uaa | ||
| property.log_pattern=[<%= timestamp_format %>] uaa%X{context} - %pid [%t] - [%X{traceId},%X{spanId}] .... %5p --- %c{1}: %replace{%m}{(?<=password=|client_secret=)([^&]*)}{<redacted>}%n | ||
| property.log_pattern=[<%= timestamp_format %>] uaa%X{context} - %pid [%t] - [%X{traceId},%X{spanId}] .... %5p --- %c{1}: %replace{%m}{(?<=password=|client_secret=|code=|access_token=|refresh_token=|id_token=)([^&\s]*)}{<redacted>}%n |
| NOTE: changing this from `legacy` to `exact` is a breaking change for clients that rely on | ||
| wildcard or subdomain redirect URI matching. Review all registered client redirect URIs before | ||
| enabling `exact` mode in existing deployments. | ||
| default: exact |
strehle
left a comment
There was a problem hiding this comment.
Two issues from the review addressed below (on the local branch pr/improve-startup-scripts-and-logs):
1. SAVEDIR not cleaned up in trap
The trap only removed PRIVATE_YML_COPY but not SAVEDIR (the mktemp -d temp directory). Fixed:
-trap 'rm -f "${PRIVATE_YML_COPY}"' EXIT
+trap 'rm -rf "${SAVEDIR}" "${PRIVATE_YML_COPY}"' EXIT2. config/private.yml restored without permissions
cp "${PRIVATE_YML_COPY}" config/ would silently create a world-readable config/private.yml if one didn't already exist, undoing the chmod 0600 on the temp copy. Fixed using install:
-cp "${PRIVATE_YML_COPY}" config/
+install -m 0600 "${PRIVATE_YML_COPY}" config/private.ymlNote: copy_master_release_metadata still does rm -rf $SAVEDIR; mkdir -p $SAVEDIR which would break the trap invariant, but it's currently commented out — worth fixing or removing if it's dead code.
| # and is cleaned up automatically on exit. | ||
| PRIVATE_YML_COPY=$(mktemp) | ||
| chmod 0600 "${PRIVATE_YML_COPY}" | ||
| trap 'rm -f "${PRIVATE_YML_COPY}"' EXIT |
| %> | ||
| property.log_directory = /var/vcap/sys/log/uaa | ||
| property.log_pattern=[<%= timestamp_format %>] uaa%X{context} - %pid [%t] - [%X{traceId},%X{spanId}] .... %5p --- %c{1}: %replace{%m}{(?<=password=|client_secret=)([^&]*)}{<redacted>}%n | ||
| property.log_pattern=[<%= timestamp_format %>] uaa%X{context} - %pid [%t] - [%X{traceId},%X{spanId}] .... %5p --- %c{1}: %replace{%m}{(?<=password=|client_secret=|code=|access_token=|refresh_token=|id_token=)([^&\s]*)}{<redacted>}%n |
| uaa.logging_level: | ||
| description: Set UAA logging level. (e.g. TRACE, DEBUG, INFO) | ||
| default: DEBUG | ||
| default: INFO | ||
| uaa.logging.format.timestamp: |
| uaa.client.redirect_uri.matching_mode: | ||
| description: | | ||
| When set to `legacy`, allow unsafe matching of redirect URIs. | ||
| For example, https://example.com would also match all subdomains and all paths of https://example.com. | ||
| When set to `exact`, will provide OAuth2 spec-compliant (RFC6749) exact redirect URI matching. | ||
| default: legacy | ||
| NOTE: changing this from `legacy` to `exact` is a breaking change for clients that rely on | ||
| wildcard or subdomain redirect URI matching. Review all registered client redirect URIs before | ||
| enabling `exact` mode in existing deployments. | ||
| default: exact | ||
|
|
Changes span the pre-start script, log4j2 redaction pattern, redirect URI matching default, and release tooling.
Redirect URI matching (BREAKING CHANGE)
uaa.client.redirect_uri.matching_modenow defaults toexactrather thanlegacy. Deployments with clients that use subdomain or path wildcard redirect URIs must register exact URIs or explicitly setmatching_mode: legacyin their manifest before upgrading.Logging defaults
uaa.logging_levelnow defaults toINFO. The log4j2 redaction pattern is extended to covercode=,access_token=,refresh_token=, andid_token=values in addition topassword=andclient_secret=.cert-cache file ownership
pre-startnow callsresecure_cert_cache()afterconfigure_tomcatandconfigure_spring_boot, restoringcert-cachetoroot:root 0711and its contents toroot:root 0644. Thevcapprocess retains read-only access to the truststore.Release script temp file handling
perform-release.shnow usesmktempfor all temporary files and directories, with mode0600on credential-bearing files andtrap ... EXITfor cleanup.