refactor: split into core library and per-version test modules#36
Merged
Conversation
WalkthroughMaven build restructured from a single-module library into an aggregator with a shared core library and four version-specific test modules, each pinning a distinct Vaadin runtime (14, 23, 24, 25). Shared test configuration centralizes dependencies and test selection; version modules inherit it and pin their own Vaadin versions. A test is updated for cross-version compatibility. ChangesMulti-version test matrix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/pom.xml`:
- Around line 87-92: The POM declares jackson-databind:3.0.0 which requires Java
17 while the project compiles to Java 8 and also has compile-time imports of
tools.jackson.databind.* and tools.jackson.databind.node.* (causing hard
linkage); fix by either (A) downgrading the jackson-databind dependency to a
Java‑8 compatible 2.x release (e.g., replace version 3.0.0 with a 2.x line such
as 2.14.x and correct groupId to com.fasterxml.jackson.core if needed) or (B)
bump the project compilation target
(maven.compiler.target/maven.compiler.release) to Java 17 so Jackson 3 is
supported — update the dependency element for jackson-databind and/or the
maven.compiler.* properties accordingly and ensure code imports
(tools.jackson.databind.* / tools.jackson.databind.node.*) remain compatible
with the chosen Jackson version.
In `@pom.xml`:
- Around line 28-47: The pom currently only sets
maven.compiler.source/target=1.8 in the root properties, but runtime for tests
must match Vaadin/Jackson baselines; update the build to select JVMs per module
(tests-v24 → Java 17, tests-v25 → Java 21) by configuring Maven Toolchains or CI
per-module JVMs and ensure surefire/failsafe forks into those JDKs (adjust
maven-surefire-plugin configuration to fork using the toolchain/JVM), and remove
any assumption that changing maven.compiler.source/target alone fixes runtime
JDK; locate the root properties (maven.compiler.source/target), modules
tests-v24 and tests-v25, and the maven-surefire-plugin configuration to
implement the toolchains/CI JVM matrix so tests run on the required JDKs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea47fc9e-336d-470a-bf99-90a53934b56d
📒 Files selected for processing (118)
core/pom.xmlcore/src/main/java/com/flowingcode/vaadin/jsonmigration/ClassInstrumentationJacksonHelper.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/ClassInstrumentationUtil.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/ElementalArrayNode.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/ElementalBooleanNode.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/ElementalNodeAsmPostProcessor.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/ElementalNullNode.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/ElementalNumberNode.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/ElementalObjectNode.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/ElementalPendingJavaScriptResult.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/ElementalStringNode.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/InstrumentationViewInitializer.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/InstrumentedRoute.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/JsonCodec.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/JsonMigration.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/JsonMigrationHelper.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/JsonMigrationHelper25.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/JsonSerializer.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/LegacyJsonMigrationHelper.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/LitRendererAsmPostProcessor.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/LitRendererMigrationExtension.javacore/src/main/java/com/flowingcode/vaadin/jsonmigration/UnsupportedJsonValueImpl.javapom.xmltests-shared/pom.xmltests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/BaseClientCallable.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable_ArrayOfJsonObject__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable_D__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable_I__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable_JsonArray__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable_JsonBoolean__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable_JsonNull__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable_JsonNumber__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable_JsonObjectVarargs__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable_JsonObject__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable_JsonString__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable_JsonValue__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable_String__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable_Z__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable__D.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable__I.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable__Integer.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable__JsonArray.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable__JsonBoolean.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable__JsonNull.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable__JsonNumber.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable__JsonObject.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable__JsonString.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable__JsonValue.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallable__Z.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallablesTest.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallablesTest24.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ClientCallablesTest25.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/ExtendsLegacyClientCallablePrivate__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/JsonMigrationHelper25Test.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate_D__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate_I__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate_JsonArray__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate_JsonBoolean__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate_JsonNull__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate_JsonNumber__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate_JsonObject__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate_JsonString__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate_JsonValue__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate_String__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate_Z__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate__D.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate__I.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate__Integer.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate__JsonArray.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate__JsonBoolean.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate__JsonNull.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate__JsonNumber.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate__JsonObject.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate__JsonString.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate__JsonValue.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablePrivate__Z.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable_AllowInert__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable_ArrayOfJsonObject__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable_ArrayOfJsonString__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable_D__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable_I__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable_JsonArray__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable_JsonBoolean__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable_JsonNull__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable_JsonNumber__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable_JsonObjectVarargs__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable_JsonObject__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable_JsonString__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable_JsonValue__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable_String__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable_Z__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable__D.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable__I.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable__Integer.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable__JsonArray.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable__JsonBoolean.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable__JsonNull.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable__JsonNumber.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable__JsonObject.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable__JsonString.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable__JsonValue.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable__V.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallable__Z.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablesPrivateTest.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablesPrivateTest24.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablesPrivateTest25.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablesTest.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablesTest24.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LegacyClientCallablesTest25.javatests-shared/src/test/java/com/flowingcode/vaadin/jsonmigration/LitRendererMigrationExtensionTest.javatests-shared/src/test/java/com/vaadin/flow/component/internal/AllowInert.javatests-v14/pom.xmltests-v23/pom.xmltests-v24/pom.xmltests-v25/pom.xml
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/maven.yml:
- Around line 17-49: The job currently inherits default GITHUB_TOKEN scopes; add
a least-privilege permissions block to the build job to restrict token access
(e.g. add a permissions mapping under the "build" job such as permissions:
contents: read and any other required read-only scopes like packages: read) so
the workflow uses a read-only token instead of the default full-scoped token.
- Around line 39-45: Update the GitHub Actions workflow to harden third-party
action usage: replace the mutable actions/checkout@v3 and actions/setup-java@v3
references with their pinned commit SHAs (use the specific commit SHA for each
action), set actions/checkout to include persist-credentials: false, and add an
explicit least-privilege permissions block (e.g., permissions: contents: read)
at the workflow level to avoid relying on defaults; locate these changes around
the uses of actions/checkout and actions/setup-java in the maven.yml workflow
and apply them together.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1e9024fe-950b-43f1-851f-4d31e601fb4c
📒 Files selected for processing (1)
.github/workflows/maven.yml
Comment on lines
17
to
+49
| jobs: | ||
|
|
||
| build-vaadin24: | ||
| build: | ||
| runs-on: ubuntu-latest | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| # Each Vaadin runtime is exercised on the JDK that matches its baseline. | ||
| # Vaadin 14/23/24 run on Java 17; Vaadin 25 is compiled to Java 21 bytecode | ||
| # and only loads on Java 21, so it must be built and tested there. | ||
| # (maven.compiler.source/target=1.8 sets bytecode level only, not the runtime JDK.) | ||
| include: | ||
| - vaadin: '14' | ||
| java: '17' | ||
| - vaadin: '23' | ||
| java: '17' | ||
| - vaadin: '24' | ||
| java: '17' | ||
| - vaadin: '25' | ||
| java: '21' | ||
| name: Vaadin ${{ matrix.vaadin }} (JDK ${{ matrix.java }}) | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - name: Set up JDK | ||
| - name: Set up JDK ${{ matrix.java }} | ||
| uses: actions/setup-java@v3 | ||
| with: | ||
| java-version: '17' | ||
| java-version: ${{ matrix.java }} | ||
| distribution: 'temurin' | ||
| cache: maven | ||
| - name: Build (Vaadin 24) | ||
| run: mvn -B package --file pom.xml No newline at end of file | ||
| # Build only this version's test module plus its upstream (core, tests-shared) | ||
| # via -am, so the module compiles and its suite runs on the matching JDK. | ||
| - name: Build (Vaadin ${{ matrix.vaadin }}) | ||
| run: mvn -B package -pl tests-v${{ matrix.vaadin }} -am No newline at end of file |
There was a problem hiding this comment.
Add least-privilege permissions for GITHUB_TOKEN.
This job currently inherits default token scopes. Add explicit read-only permissions to reduce blast radius if any action/step is compromised.
Suggested patch
name: Java CI with Maven
on:
push:
branches: [ "master" ]
pull_request:
branches: [ "master" ]
+permissions:
+ contents: read
+
jobs:🧰 Tools
🪛 actionlint (1.7.12)
[error] 39-39: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
[error] 41-41: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 zizmor (1.25.2)
[warning] 39-39: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 19-49: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[error] 39-39: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 41-41: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/maven.yml around lines 17 - 49, The job currently inherits
default GITHUB_TOKEN scopes; add a least-privilege permissions block to the
build job to restrict token access (e.g. add a permissions mapping under the
"build" job such as permissions: contents: read and any other required read-only
scopes like packages: read) so the workflow uses a read-only token instead of
the default full-scoped token.
Source: Linters/SAST tools
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
gh issue create \
--repo FlowingCode/JsonMigrationHelper \
--assignee javier-godoy \
--title "ci: add least-privilege permissions block to GitHub Actions workflow" \
--body "## Summary
The \`.github/workflows/maven.yml\` build job currently inherits default \`GITHUB_TOKEN\` scopes, which is overly broad. A least-privilege \`permissions\` block should be added at the workflow (or job) level to reduce the blast radius if any action or step is compromised.
## Suggested fix
Add the following block to the workflow (top-level, after the \`on:\` section):
\`\`\`yaml
permissions:
contents: read
\`\`\`
If the workflow ever needs to read packages from GHPR, also add \`packages: read\`.
## References
- Flagged in PR `#36` — comment: https://github.com/FlowingCode/JsonMigrationHelper/pull/36#discussion_r3388417899
- Detected by [zizmor](https://github.com/woodruffw/zizmor) rule \`excessive-permissions\`
## Requested by
`@javier-godoy`"Length of output: 1058
@javier-godoy, the issue has been created: FlowingCode/JsonMigrationHelper#37 — ci: add least-privilege permissions block to GitHub Actions workflow.
Comment on lines
39
to
45
| - uses: actions/checkout@v3 | ||
| - name: Set up JDK | ||
| - name: Set up JDK ${{ matrix.java }} | ||
| uses: actions/setup-java@v3 | ||
| with: | ||
| java-version: '17' | ||
| java-version: ${{ matrix.java }} | ||
| distribution: 'temurin' | ||
| cache: maven |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification: resolve current supported major tags to immutable SHAs.
# Expected: choose a supported major tag and pin to its commit SHA in workflow YAML.
set -euo pipefail
for repo in actions/checkout actions/setup-java; do
echo "== $repo =="
for tag in v4 v5; do
code=$(curl -s -o /tmp/ref.json -w "%{http_code}" \
"https://api.github.com/repos/$repo/git/ref/tags/$tag")
if [ "$code" = "200" ]; then
sha=$(jq -r '.object.sha' /tmp/ref.json)
echo "$tag -> $sha"
fi
done
doneRepository: FlowingCode/JsonMigrationHelper
Length of output: 311
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the full workflow with line numbers but keep output bounded by showing only relevant sections
FILE=".github/workflows/maven.yml"
echo "== File: $FILE =="
nl -ba "$FILE" | sed -n '1,220p'Repository: FlowingCode/JsonMigrationHelper
Length of output: 176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/maven.yml"
echo "== grep uses/permissions/persist-credentials in $FILE =="
rg -n "uses:\s*actions/(checkout|setup-java)@|permissions:|persist-credentials" "$FILE" || true
echo
echo "== file excerpt around checkout/setup-java =="
awk 'NR>=30 && NR<=80 {printf "%5d %s\n", NR, $0}' "$FILE"
echo
echo "== full file (bounded) =="
awk '{printf "%5d %s\n", NR, $0}' "$FILE"Repository: FlowingCode/JsonMigrationHelper
Length of output: 3264
🌐 Web query:
actions/checkout persist-credentials default true write permissions later steps
💡 Result:
In the actions/checkout action, the persist-credentials input defaults to true [1][2]. When enabled, this setting configures the local Git repository with the authentication token or SSH key used to check out the code [1][3]. This allows subsequent steps in the same job to execute authenticated Git commands (such as git fetch or git push) automatically without requiring further configuration [1][4]. Regarding security and permissions: 1. Credential Security: In newer versions of actions/checkout, credentials are stored in a secure, separate file under $RUNNER_TEMP rather than directly in the.git/config file, which helps prevent accidental credential leakage (for example, if artifacts are published) [1][4][5]. The token is automatically removed during the post-job cleanup phase [1][3]. 2. Access Scope: While setting persist-credentials to true configures Git to use these credentials, it does not fundamentally change the accessibility of the GITHUB_TOKEN within a job [6][5]. The GITHUB_TOKEN is available to all steps within a job by default, regardless of the actions/checkout configuration [6][5]. Consequently, an action could technically access the token through other channels even if persist-credentials is set to false [5]. 3. Opting Out: If you prefer not to have the credentials persisted in the Git configuration for your workflow steps, you can explicitly set persist-credentials: false in your workflow file [1][3]. Top results: [1][3][4][5]
Citations:
- 1: https://github.com/actions/checkout/?tab=readme-ov-file
- 2: https://github.com/actions/checkout/blob/72f2cec99f417b1a1c5e2e88945068983b7965f9/action.yml
- 3: https://github.com/actions/checkout/blob/85e6279cec87321a52edac9c87bce653a07cf6c2/README.md
- 4: https://github.com/actions/checkout/tree/v6.0.2?tab=readme-ov-file
- 5: [security] Escalate concerning default
persist-credentials=trueplease actions/checkout#2312 - 6: Remove
persist-credentialsor change the default tofalseactions/checkout#485
🌐 Web query:
actions/setup-java cache: maven required workflow permissions contents actions write
💡 Result:
The actions/setup-java action does not require "contents: write" permissions for caching Maven dependencies [1][2]. The official documentation explicitly recommends setting permissions to "contents: read" to ensure proper functionality [1][3]. The "contents: read" permission is sufficient because the action only needs to read your dependency files (e.g., pom.xml) to calculate the cache key and access the internal GitHub Actions Cache service, which is managed by the workflow environment [1][2][4]. You do not need write access to your repository contents to manage these dependencies [1].
Citations:
- 1: https://github.com/actions/setup-java/blob/main/README.md
- 2: https://github.com/actions/setup-java
- 3: actions/setup-java@99d3141
- 4: https://deepwiki.com/actions/setup-java/3.1-dependency-caching
Harden third-party GitHub Actions in .github/workflows/maven.yml (pin + disable checkout credential persistence).
- At lines 39-41, replace mutable/older
actions/checkout@v3andactions/setup-java@v3with pinned commit SHAs. - Set
persist-credentials: falseforactions/checkout(defaults totrue). - Add an explicit least-privilege
permissions:block (e.g.,contents: read) instead of relying on defaults.
Suggested patch
- - uses: actions/checkout@v3
+ - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd
+ with:
+ persist-credentials: false
- name: Set up JDK ${{ matrix.java }}
- uses: actions/setup-java@v3
+ uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654🧰 Tools
🪛 actionlint (1.7.12)
[error] 39-39: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
[error] 41-41: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 zizmor (1.25.2)
[warning] 39-39: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 39-39: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 41-41: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/maven.yml around lines 39 - 45, Update the GitHub Actions
workflow to harden third-party action usage: replace the mutable
actions/checkout@v3 and actions/setup-java@v3 references with their pinned
commit SHAs (use the specific commit SHA for each action), set actions/checkout
to include persist-credentials: false, and add an explicit least-privilege
permissions block (e.g., permissions: contents: read) at the workflow level to
avoid relying on defaults; locate these changes around the uses of
actions/checkout and actions/setup-java in the maven.yml workflow and apply them
together.
Source: Linters/SAST tools
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
gh issue create \
--repo FlowingCode/JsonMigrationHelper \
--title "ci: harden GitHub Actions workflow (pin action SHAs, least-privilege permissions)" \
--assignee "javier-godoy" \
--body "## Summary
The \`.github/workflows/maven.yml\` workflow uses mutable version tags (\`actions/checkout@v3\`, \`actions/setup-java@v3\`) and relies on default permissions, which poses a supply-chain security risk.
## Findings
1. **Unpinned action references** – \`actions/checkout@v3\` and \`actions/setup-java@v3\` should be pinned to immutable commit SHAs to prevent unexpected changes or supply-chain attacks.
2. **Credential persistence** – \`actions/checkout\` defaults to \`persist-credentials: true\`, which persists the GITHUB_TOKEN in the Git config for the duration of the job. Setting \`persist-credentials: false\` is the safer default for jobs that do not need authenticated Git operations after checkout.
3. **Implicit permissions** – The workflow does not declare an explicit \`permissions:\` block. Adding \`permissions: contents: read\` (or the minimum required) follows the principle of least privilege.
Additionally, \`actionlint\` reports that the v3 runners for both actions are too old to run on GitHub Actions and should be updated.
## References
- Flagged during review of PR `#36` in comment https://github.com/FlowingCode/JsonMigrationHelper/pull/36#discussion_r3388417907
- Requested by `@javier-godoy`
"Length of output: 1546
@javier-godoy, the issue has been created: https://github.com/FlowingCode/JsonMigrationHelper/issues/38
paodb
approved these changes
Jun 10, 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.
Compile the library once against Vaadin 14 in a self-contained
coremodule, and run a single shared test suite against Vaadin 14/23/24/25 via dedicatedtest-v*modules. Each test module adds the shared sources fromtests-sharedvia build-helper and pins its own Vaadin runtime; surefire selection runs the legacy (*Test24) suites on 14/23/24 and the Jackson (*Test25) suites on 25.Close #34
Summary by CodeRabbit