Skip to content

Upgrade to Java 27 & small improvements - #570

Open
MartelliEnrico wants to merge 8 commits into
mainfrom
java-27-improvements
Open

MartelliEnrico wants to merge 8 commits into
mainfrom
java-27-improvements

Conversation

@MartelliEnrico

@MartelliEnrico MartelliEnrico commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Compatibility

    • The application, build environment and setup instructions now require JDK 27 or later.
    • Windows startup diagnostics are directed to error output; the Java launch command is unchanged.
  • Media

    • Video conversion now uses a single FFmpeg filter thread for each conversion pass.
  • Bug Fixes

    • Improved startup handling when operating system information is unavailable, avoiding an error if that information is missing.

- Compact Object Headers are on by default, so the jre creation was simplified
- C1 is the new GC, and for the bot usage should be better than Generational Shenandoah
These checks enable the new jdk nullability data, so nullable lib methods are properly annotated now
@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

📝 Walkthrough

Walkthrough

The project now uses Java 27 in its build, CI, Docker builder, and setup instructions. The Gradle build updates jlink and nullability settings and packages an OpenTelemetry agent. Application code, the Windows launcher, and test helpers also change.

Changes

Project updates

Layer / File(s) Summary
Java 27 toolchain
.github/workflows/unit-test.yml, Dockerfile, README.md, buildSrc/build.gradle.kts, gradle/gradle-daemon-jvm.properties, build.gradle.kts
CI, the Docker builder, setup instructions, and Gradle toolchain configuration now select Java 27.
Gradle build and OpenTelemetry packaging
build.gradle.kts, buildSrc/src/main/java/com/github/stickerifier/stickerify/JlinkTask.java, buildSrc/src/main/java/com/github/stickerifier/stickerify/DownloadOpenTelemetryAgentTask.java, gradle/libs.versions.toml, Dockerfile
The build updates jlink options and nullability settings. It downloads and packages the OpenTelemetry agent and uses it in application JVM arguments. The Docker runtime no longer downloads or configures the agent.
Application runtime settings
src/main/java/com/github/stickerifier/stickerify/bot/Stickerify.java, src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java, src/main/java/com/github/stickerifier/stickerify/process/OsConstants.java
The bot token is declared nullable, the bot.answer span is marked as a consumer span, a missing OS name is handled, and FFmpeg filter processing is limited to one thread.
Windows launcher error paths
src/main/resources/customWindowsStartScript.txt
Java lookup error messages are redirected to standard error, and both error paths jump to :exitWithErrorLevel. The source reference and compatibility comment also change.
Test helper null handling
src/test/java/com/github/stickerifier/stickerify/ResourceHelper.java, src/test/java/com/github/stickerifier/stickerify/junit/TempFilesCleanupExtension.java, src/test/java/com/github/stickerifier/stickerify/media/MediaHelperTest.java
Test helpers handle absent class loaders, temporary-directory properties, path file names, and exception messages.

Priority: ⬇️ Low

Merge Risk: 🟡 Moderate · up to 5c9eb

The OpenTelemetry agent is now loaded by a relative path. Starting the app from a directory other than its install location, or through gradle run, can prevent it from starting. The new agent download can also hang a build indefinitely if the connection stalls. Fix the agent path before merging.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 5c9eb

The bundled agent is selected using a working-directory-relative filename rather than its installed location. Launching from another directory can fail or, if another actor controls that directory, load unintended code with the bot’s privileges. Agent execution also expands beyond Docker distributions. The default container layout limits the first risk, but packaged startup and failure-recovery behavior remain unverified.

Retained concerns

  • Medium · security · inferred: The new startup argument selects opentelemetry-javaagent.jar from the launch working directory, not explicitly from the installation. Both launcher templates forward JVM options without changing that directory. Consequently, a launch outside the distribution root can fail or load an attacker-supplied agent if another actor can write that directory. This weakens the previous Docker agent’s absolute-path binding; the normal container layout mitigates the issue.
  • Medium · security · inferred: Every produced application distribution now includes and configures startup execution of a GitHub-downloaded agent whose bytes are not checked against an independently trusted digest or signature. Docker already trusted an unverified HTTPS download, so this is not a newly introduced verification deficit there; the PR expands that executable-artifact trust to non-container distributions. Compromise of the trusted release source could consequently affect more bot deployments, although no compromised artifact is evidenced.
Security review details

Security Blast Radius

  • inferred — A substituted executable agent would run inside the bot JVM with the launching account’s authority, potentially accessing its token, readable files and permitted network destinations. The affected scope is the deployment that loads that JAR; broader host, tenant or service compromise depends on privileges and topology not supplied here.

Security Findings and Attack Paths

  • inferred — The source-supported substitution path requires an actor to place a valid malicious agent under the expected filename in the directory from which an operator launches the bot. The relative startup argument can select those bytes instead of the trusted installation’s agent. No Telegram-request-to-agent-write path or actual exploitation was established.

Trust Boundaries and Controls

  • observed — Artifact acquisition trusts the configured GitHub HTTPS release response without independent content verification. Version input and task-output wiring provide selection and intended build ordering, not cryptographic identity. The previous Docker acquisition also lacked an explicit checksum, but its runtime argument identified an absolute installed path.

Resilience and Maintainability Implications

  • inferred — Direct writes make interrupted acquisition externally visible as an incomplete final artifact. Failure propagation provides same-execution containment, so later publication of partial bytes is not established. Recovery correctness still depends on rerunning the producer and avoiding independent writers to the shared destination.

Hardening Proposals

  • proposed — Bind the agent argument to the installation root in both launcher variants, preserving quoting for paths with spaces. Validate startup from unrelated working directories, including one containing a same-named JAR.
  • proposed — Bind the release to an independently trusted digest or signature, download into a private temporary file, validate before atomic publication, and clean up failed attempts. Define retry and concurrent-writer behavior so distribution assembly consumes only a complete, validated output.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 10 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the Java 27 upgrade, the primary change in the pull request. “Small improvements” is broad but does not make the title misleading.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 10 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

"-c:v", "libvpx-" + VP9_CODEC,
"-row-mt", "1",
"-threads", "2",
"-filter_threads", "1",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why should we limit it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a parallel of the -row-mt 1 settings, which limit the processing to 1 cpu for the codec, while -filter_threads 1 limits the processing of the filters to 1 cpu. All this to avoid too many processes running inside the Docker image

Comment thread src/main/java/com/github/stickerifier/stickerify/process/OsConstants.java Outdated
Comment thread build.gradle.kts
Comment thread qodana.yaml
# bump: Qodana /qodana-jvm-community:([\d.]+)/ docker:jetbrains/qodana-jvm-community|/^[\d]{4}\.\d+$/|sort
linter: jetbrains/qodana-jvm-community:2026.2
projectJDK: temurin-26
projectJDK: temurin-27

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems like it isn't available yet on their side

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We just need to wait for Qodana to catch up

@rob93c rob93c added the enhancement New feature or request label Oct 3, 2026

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 0fd6f75e-b040-44b5-b29f-76dd03185ee2
📥 Commits

Reviewing files that changed from the base of the PR and between dab606c and 5c9eba0.

📒 Files selected for processing (6)
  • Dockerfile
  • build.gradle.kts
  • buildSrc/src/main/java/com/github/stickerifier/stickerify/DownloadOpenTelemetryAgentTask.java
  • gradle/libs.versions.toml
  • src/main/java/com/github/stickerifier/stickerify/bot/Stickerify.java
  • src/main/java/com/github/stickerifier/stickerify/process/OsConstants.java
💤 Files with no reviewable changes (1)
  • Dockerfile

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Comment thread build.gradle.kts
application {
mainClass = "com.github.stickerifier.stickerify.runner.Main"
applicationDefaultJvmArgs = listOf("-XX:+UseCompactObjectHeaders", "-XX:+UseShenandoahGC", "-XX:ShenandoahGCMode=generational", "--enable-final-field-mutation=ALL-UNNAMED")
applicationDefaultJvmArgs = listOf("--enable-final-field-mutation=ALL-UNNAMED", "-javaagent:" + openTelemetryAgent.get().destinationFile.get().asFile.name)

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.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Resolve the agent path for each launch mode.

applicationDefaultJvmArgs applies to gradle run and the distribution launchers. The agent JAR is added only to the distribution, while -javaagent:opentelemetry-javaagent.jar resolves relative to the process working directory. If gradle run starts outside that directory, or a user starts an installed launcher from elsewhere, the JVM cannot find the agent and the application does not start. Make run depend on the download task and give it the build output path. Resolve the packaged agent relative to the installation directory in the launch scripts. (docs.gradle.org)

var targetFile = getDestinationFile().get().getAsFile();

var _ = targetFile.getParentFile().mkdirs();
try (var inputStream = URI.create(urlString).toURL().openStream()) {

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.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Set connect and read timeouts for the agent download.

If the release connection stalls, openStream() can wait indefinitely because Java 27 leaves both timeouts disabled by default. This can leave the distribution build waiting until an operator cancels it. Open a connection and set both timeouts before reading the JAR. (docs.oracle.com)

Comment thread build.gradle.kts
}

val openTelemetryAgent = tasks.register<DownloadOpenTelemetryAgentTask>(DownloadOpenTelemetryAgentTask.DEFAULT_TASK_NAME) {
description = "Downloads the OpenTelemetry agent for the distribution package."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need to define the description both here and inside the task implementation?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In the task implementation is best practice, here is a warning if not provided

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants