deploy/podman/quadlet/grimmory-db.container (1)

16-17: Use separate secrets for root and application DB passwords.

Line 16 and Line 17 currently bind the same secret to both root and app credentials, which reduces blast-radius isolation.

🔐 Suggested hardening diff

-Secret=grimmory_db_pass,type=env,target=MYSQL_ROOT_PASSWORD
-Secret=grimmory_db_pass,type=env,target=MYSQL_PASSWORD
+Secret=grimmory_db_root_pass,type=env,target=MYSQL_ROOT_PASSWORD
+Secret=grimmory_db_app_pass,type=env,target=MYSQL_PASSWORD

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@deploy/podman/quadlet/grimmory-db.container` around lines 16 - 17, The two
Secret entries bind the same secret name (grimmory_db_pass) to both
MYSQL_ROOT_PASSWORD and MYSQL_PASSWORD, so create and use distinct secrets
(e.g., grimmory_db_root_pass and grimmory_db_app_pass) and update the two Secret
lines so the target=MYSQL_ROOT_PASSWORD uses the root secret and
target=MYSQL_PASSWORD uses the app secret; also ensure any secret creation/CI
manifests that currently create grimmory_db_pass are updated to create both
unique secrets and any code/config that reads those env vars is left unchanged.

.github/workflows/preview-image.yml (2)

71-76: Dual cache-to targets may cause redundant writes.

Writing to both type=gha and type=registry caches on every preview build could increase build times and storage costs. For preview builds (which are ephemeral), consider using only the GHA cache, reserving registry cache writes for release/nightly workflows.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/preview-image.yml around lines 71 - 76, The workflow
writes cache-to entries for both type=gha and type=registry in the
preview-image.yml (the cache-to block), causing redundant registry writes for
ephemeral preview builds; change the preview workflow to only write the GHA
cache (remove or conditionally disable the registry cache-to entry) so
preview-image.yml uses type=gha,mode=max cache-to while reserving the registry
cache-to (type=registry,ref=...,mode=max) for release/nightly workflows or
behind a conditional check.

---

33-44: Potential shell injection via workflow input.

The inputs.pr_number is directly interpolated into the shell script without sanitization. While workflow_dispatch inputs are manually triggered by authorized users, a malicious or careless input like "; rm -rf /; " could cause unintended behavior.

Consider validating the input or using environment variables to avoid direct interpolation:

♻️ Proposed fix using environment variable

      - name: Generate Preview Tag
+       env:
+         PR_NUMBER: ${{ inputs.pr_number }}
        run: |
          short_sha=$(git rev-parse --short HEAD)

-         if [ -n "${{ inputs.pr_number }}" ]; then
-           preview_tag="pr-${{ inputs.pr_number }}-${short_sha}"
+         if [ -n "$PR_NUMBER" ]; then
+           # Validate pr_number is numeric
+           if ! [[ "$PR_NUMBER" =~ ^[0-9]+$ ]]; then
+             echo "Invalid PR number: $PR_NUMBER"
+             exit 1
+           fi
+           preview_tag="pr-${PR_NUMBER}-${short_sha}"
          else
            preview_tag="preview-${short_sha}"
          fi

          echo "preview_tag=$preview_tag" >> "$GITHUB_ENV"
          echo "commit_sha=$(git rev-parse HEAD)" >> "$GITHUB_ENV"
          echo "Preview tag: $preview_tag"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/preview-image.yml around lines 33 - 44, The workflow
directly interpolates inputs.pr_number into the shell and assigns preview_tag,
which allows shell injection; instead, capture the input into a safe environment
variable (e.g., PR_NUMBER) and validate/sanitize it in the shell before using it
to build preview_tag (for example allow only digits or alphanumerics, reject or
strip anything else), always quote expansions (use "$PR_NUMBER") and use a safe
fallback path when validation fails so preview_tag is never constructed from
unsanitized input; update the logic that sets preview_tag and the uses of
inputs.pr_number to reference the sanitized PR_NUMBER variable and ensure
commit_sha/preview_tag are echoed from the sanitized values.

.github/workflows/publish-release.yml (1)

48-167: Significant code duplication with ci-validate.yml test jobs.

The backend-tests and frontend-tests jobs here are near-identical copies of those in ci-validate.yml. This creates maintenance burden—any fix must be applied in both places.

Consider extracting these into reusable workflows (similar to migrations-check.yml), or verify that the commit being released has already passed CI validation on main.

♻️ Alternative: Verify CI status instead of re-running tests

If the release workflow requires that the ref already passed CI on main, you could use the GitHub API to verify the commit status instead of re-running all tests:

  verify-ci-passed:
    name: Verify CI Status
    needs: [resolve_ref]
    runs-on: ubuntu-latest
    steps:
      - name: Check commit status
        env:
          GH_TOKEN: ${{ github.token }}
        run: |
          status=$(gh api repos/${{ github.repository }}/commits/${{ needs.resolve_ref.outputs.commit_sha }}/status --jq '.state')
          if [ "$status" != "success" ]; then
            echo "CI has not passed for this commit"
            exit 1
          fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/publish-release.yml around lines 48 - 167, The
backend-tests and frontend-tests jobs duplicate the same test logic already
present in ci-validate.yml; refactor by extracting these jobs into a reusable
workflow or calling an existing reusable workflow (similar to
migrations-check.yml) instead of duplicating steps, or replace them with a
Verify CI Status job that checks the commit status using the GitHub API; update
the workflow to remove the duplicated job definitions (backend-tests,
frontend-tests) and either reference the reusable workflow or add a
verify-ci-passed job that queries the commit status for
needs.resolve_ref.outputs.commit_sha and fails if not "success".

.github/workflows/ci-validate.yml (2)

101-103: npm audit may cause spurious CI failures on transitive dependency issues.

The npm audit --audit-level=high step will fail the job if any high-severity vulnerabilities are detected in dependencies, even if they're transitive and not directly exploitable in this codebase. Consider adding continue-on-error: true with a separate advisory step, or use --audit-level=critical for blocking failures.

♻️ Option: Make audit advisory rather than blocking

      - name: Audit Frontend Dependencies
        working-directory: ./booklore-ui
-       run: npm audit --audit-level=high
+       run: npm audit --audit-level=high || echo "::warning::npm audit found high-severity vulnerabilities"
+       continue-on-error: true

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci-validate.yml around lines 101 - 103, The CI step named
"Audit Frontend Dependencies" currently runs npm audit with "--audit-level=high"
which can cause spurious failures on transitive issues; update that step (the
block containing the name "Audit Frontend Dependencies" and the run command "npm
audit --audit-level=high") to either add "continue-on-error: true" so the audit
is advisory, or lower its strictness by changing the run to "npm audit
--audit-level=critical" so only critical issues fail the job; implement one of
these two fixes to avoid blocking CI on non-actionable transitive
vulnerabilities.

---

17-20: Verify base_ref fallback handles edge cases correctly.

The complex expression for base_ref uses format('{0}^', github.sha) as a fallback when github.event.before is the null SHA (new branch push). However, the migrations-check.yml workflow already has normalization logic (lines 44-50) that handles the null SHA case by deriving the parent.

This creates redundant fallback logic in two places. Consider simplifying by always passing the raw values and letting the reusable workflow handle normalization:

♻️ Simplify by delegating normalization to reusable workflow

      base_ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || (github.event.before != '0000000000000000000000000000000000000000' && github.event.before || format('{0}^', github.sha)) }}
+     # Could simplify to:
+     # base_ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }}
+     # The reusable workflow already handles null SHA normalization

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci-validate.yml around lines 17 - 20, The base_ref
expression includes a redundant fallback (format('{0}^', github.sha)) that
duplicates normalization logic performed in the reusable workflow; remove the
complex ternary and pass the raw event values so the called workflow handles
null-SHA normalization. Concretely, set base_ref to use the raw PR base or the
event.before (e.g. base_ref: ${{ github.event.pull_request.base.sha ||
github.event.before }}), and simplify head_ref and checkout_ref to use the raw
PR head or github.sha (e.g. head_ref: ${{ github.event.pull_request.head.sha ||
github.sha }} and checkout_ref: ${{ github.event.pull_request.head.sha ||
github.sha }}), leaving normalization to the reusable migrations-check.yml
logic.

deploy/podman/quadlet/grimmory.container (1)

8-20: Duplicated version in image tag and APP_VERSION environment variable.

The version v0.38.2 is hardcoded in both the Image= directive (line 8) and the APP_VERSION environment variable (line 20). This duplication creates maintenance overhead and risk of version drift.

Consider documenting that users should update both values together, or note that APP_VERSION can be derived at runtime if the application supports it.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@deploy/podman/quadlet/grimmory.container` around lines 8 - 20, The Image= and
Environment=APP_VERSION entries duplicate the same hardcoded version (v0.38.2);
remove the drift risk by either deleting the hardcoded APP_VERSION env var and
letting the application derive its version at runtime, or implement a single
source of truth (e.g., templating/replacement that sets both Image= and
APP_VERSION from one variable) so they cannot diverge; update the
grimmory.container manifest to use only one authoritative version source and add
a short comment explaining which approach is used.

.github/workflows/publish-nightly.yml (2)

33-36: Unnecessary pull-requests: write permission for nightly workflow.

This workflow runs on schedule and workflow_dispatch triggers only—there's no PR context to post comments to. The pull-requests: write permission can be removed to follow least-privilege principles.

♻️ Proposed fix

     permissions:
       contents: read
       checks: write
-      pull-requests: write

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/publish-nightly.yml around lines 33 - 36, Remove the
unnecessary pull-requests write permission from the GitHub Actions permissions
block to follow least-privilege principles: edit the permissions mapping where
keys are "contents", "checks", and "pull-requests" and delete the
"pull-requests: write" entry so the workflow only grants the required "contents:
read" and "checks: write" permissions for the scheduled/workflow_dispatch
nightly run.

---

88-91: Same unnecessary permission in frontend-tests job.

Same issue as backend-tests—remove pull-requests: write since there's no PR context in schedule/dispatch triggers.

♻️ Proposed fix

     permissions:
       contents: read
       checks: write
-      pull-requests: write

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/publish-nightly.yml around lines 88 - 91, In the
frontend-tests job remove the unnecessary pull-requests: write permission from
the permissions block (i.e., delete the "pull-requests: write" entry) because
this workflow runs on schedule/dispatch and has no PR context; update the
permissions list so it only contains contents: read and checks: write to match
the backend-tests fix.

Dockerfile (1)

6-7: Consider removing --force flag from npm ci.

The --force flag bypasses peer dependency validation, which can mask legitimate compatibility issues. If this is needed for specific peer dependency conflicts, consider documenting the reason in a comment or resolving the underlying conflicts.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 6 - 7, The Dockerfile RUN step uses "npm ci --force
--no-audit --no-fund" which bypasses peer dependency validation; remove the
"--force" flag from that npm ci invocation in the RUN command (or if you must
keep it for a documented reason, add an inline comment above the RUN explaining
the specific peer-dependency conflict and why --force is required). Update the
RUN line that contains the npm ci command accordingly and ensure any CI docs
mention the chosen approach.

README.md (1)

5-5: Make the predecessor name explicit for migration clarity.

Consider naming Booklore directly instead of “original project” so users can find migration context faster.

Suggested wording

-> Grimmory is the successor of the original project.
+> Grimmory is the successor of Booklore.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 5, Update the README sentence that currently reads
"Grimmory is the successor of the original project." to explicitly name the
predecessor by replacing "original project" with "Booklore" so it reads
something like "Grimmory is the successor to Booklore." This change should be
made where the project intro mentions Grimmory to help users locate migration
and compatibility information faster.

.github/workflows/publish-nightly.yml (1)

1-6: LGTM on the nightly workflow triggers.

The 4 AM UTC schedule is reasonable for nightly builds, and workflow_dispatch allows manual triggering when needed. Consider adding a concurrency block if simultaneous manual dispatches could cause issues:

concurrency:
  group: nightly-build
  cancel-in-progress: false

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/publish-nightly.yml around lines 1 - 6, Add a top-level
concurrency block to the workflow to prevent simultaneous runs: insert a
concurrency key with group set to "nightly-build" and cancel-in-progress set to
false (i.e., use concurrency, group: nightly-build, cancel-in-progress: false)
so manual dispatches don't trigger overlapping runs.

Dockerfile (2)

69-73: Acknowledge Trivy DS-0002: Consider documenting the runtime user model.

Trivy flags that no USER directive is specified. However, the use of su-exec (installed on line 69) indicates the entrypoint handles privilege dropping at runtime—a common pattern when the container needs to match host UID/GID for volume permissions.

Consider adding a comment in the Dockerfile explaining this design choice, which helps future maintainers understand why USER isn't set statically.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 69 - 73, Add a brief explanatory comment in the
Dockerfile near the installation of su-exec and the COPY of entrypoint.sh
indicating that user privileges are dropped at runtime by the entrypoint
(entrypoint.sh) using su-exec to allow matching host UID/GID for mounted
volumes, which is why no static USER directive is set in the image; reference
the su-exec installation and entrypoint.sh to make the runtime user model
explicit for future maintainers.

---

32-34: JAR discovery could be more robust.

The find | head -n 1 pattern will return an empty string if no JAR is found, which could lead to cp "" /workspace/booklore-api/app.jar failing with a confusing error. While set -eux will catch this, adding an explicit check would improve error messaging.

♻️ Proposed improvement for clearer failure

 RUN set -eux; \
     jar_path="$(find build/libs -maxdepth 1 -name '*.jar' ! -name '*plain.jar' | head -n 1)"; \
+    [ -n "$jar_path" ] || { echo "ERROR: No JAR found in build/libs"; exit 1; }; \
     cp "$jar_path" /workspace/booklore-api/app.jar

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 32 - 34, The RUN shell block that sets jar_path and
copies it is fragile: after jar_path="$(find build/libs -maxdepth 1 -name
'*.jar' ! -name '*plain.jar' | head -n 1)" add an explicit check that jar_path
is non-empty before running cp (e.g., test -n "$jar_path" || { echo "No JAR
found in build/libs" >&2; exit 1; }) so the COPY step fails with a clear error;
update the RUN sequence around set -eux, jar_path and cp to perform this
validation and emit a descriptive error message if no JAR is discovered.

.github/workflows/publish-release.yml (1)

35-41: Branch containment check may have edge cases.

The check git branch -r --contains HEAD | grep -q 'origin/main' verifies the commit is reachable from origin/main. However, git branch -r --contains can be slow on large repositories and may produce unexpected results if origin/main hasn't been fully fetched.

Consider using git merge-base --is-ancestor HEAD origin/main for a more direct ancestry check:

♻️ Alternative ancestry check

       - name: Confirm Release Ref Is On Main
         run: |
           git fetch origin main --tags --force
-          if ! git branch -r --contains HEAD | grep -q 'origin/main'; then
+          if ! git merge-base --is-ancestor HEAD origin/main; then
             echo "Selected ref is not contained by origin/main"
             exit 1
           fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/publish-release.yml around lines 35 - 41, Replace the
current slow/fragile containment check in the "Confirm Release Ref Is On Main"
step (which uses `git branch -r --contains HEAD | grep -q 'origin/main'`) with a
direct ancestry test using `git merge-base --is-ancestor HEAD origin/main`;
update the step to ensure `origin/main` is fetched first (e.g., `git fetch
origin main --tags --force`) and then call `git merge-base --is-ancestor HEAD
origin/main` and fail if it returns non-zero so the job exits when HEAD is not
an ancestor of origin/main.

.github/workflows/test-suite.yml (1)

24-25: Consider shallow clone for test jobs.

fetch-depth: 0 fetches full history, which adds overhead for jobs that only run tests. A shallow clone (fetch-depth: 1) would be faster. However, if the test suite or any tooling requires git history, keep the current setting.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/test-suite.yml around lines 24 - 25, The workflow
currently sets fetch-depth: 0 which performs a full git clone; change this to
fetch-depth: 1 in the job checkout step to perform a shallow clone for faster
test runs (unless your tests or tooling require full history), i.e., update the
checkout action configuration that contains fetch-depth and ref (the keys shown
as fetch-depth and ref: ${{ inputs.checkout_ref }}) to use fetch-depth: 1 or
document/guard the full-clone behavior when history is required.

.github/workflows/ci-validate.yml (1)

42-43: QEMU setup is unnecessary for amd64-only builds.

Since the smoke test only builds for linux/amd64 (line 53) and runs on ubuntu-latest (which is amd64), the QEMU setup step adds overhead without benefit. Consider removing it for faster CI runs.

♻️ Remove unnecessary QEMU step

-      - name: Set Up QEMU for Multi-Arch Builds
-        uses: docker/setup-qemu-action@c7c53464625b32c7a7e944ae62b3e17d2b600130 # v3
-
       - name: Set Up Docker Buildx

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci-validate.yml around lines 42 - 43, Remove the
unnecessary QEMU setup step named "Set Up QEMU for Multi-Arch Builds" (the use
of docker/setup-qemu-action@c7c53464625b32c7a7e944ae62b3e17d2b600130) because
the workflow only targets linux/amd64 and runs on ubuntu-latest; delete that
step so the job that builds for linux/amd64 (the smoke test) no longer runs the
QEMU action, reducing CI overhead and runtime.

.github/workflows/migrations-check.yml (1)

83-99: Consider cleaning MariaDB state between base and head migration runs.

After applying base migrations, the workflow checks out the head ref and applies head migrations on top. This correctly validates that head migrations apply cleanly over base. However, if a head migration modifies or removes an object created by a base migration in an incompatible way, this won't catch that until production. The current approach is reasonable for incremental migration validation but won't detect full schema drift from scratch.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/migrations-check.yml around lines 83 - 99, The workflow
applies base migrations then checks out head and runs head migrations without
resetting the MariaDB state; add an explicit DB reset between the "Apply
Migrations from Base Branch" step and the step that checks out/applies head
migrations so head runs start from a clean slate. Implement the reset by
invoking a Flyway clean (or issuing a DROP/CREATE for the test database) against
the same JDBC URL used in the "Apply Migrations from Base Branch" docker run,
ensuring the Flyway container/image (flyway/flyway:12.1.0) or a mysql client is
used with the same -url, -user and -password to remove the base-state before
running head migrations. Ensure the new reset step references the same migration
paths and credentials so the subsequent head migration run validates applying
migrations onto a fresh DB.