Skip to content

πŸ”’ security: v1.7 hygiene β€” icon integrity, base-image CVE bumps, dead SW rule - #680

Merged
scttbnsn merged 8 commits into
dev/v1.7from
fix/v1.7-security-hygiene
Aug 8, 2026
Merged

πŸ”’ security: v1.7 hygiene β€” icon integrity, base-image CVE bumps, dead SW rule#680
scttbnsn merged 8 commits into
dev/v1.7from
fix/v1.7-security-hygiene

Conversation

@scttbnsn

@scttbnsn scttbnsn commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Security/correctness hygiene batch from the 2026-08-08 sweep workflows.

Icon-bundle integrity

  • Restores the Audit nav icon (lucide:history was dropped by tabler/lucide bundle regeneration in πŸ—‘οΈ remove(triggers): drop DD_TRIGGER_* legacy aliases (Phase 4 removal)Β #674) and fixes 5 more stale icon references (iconoir history/key/gitlab/stack, lucide more-vertical)
  • Adds @iconify-json/fa6-brands devDep that the bundle referenced but never declared
  • New guard test ui/tests/icons.spec.ts: every iconMap key must resolve against the generated icon-bundle.json, so future bundle regenerations can't silently blank icons

Base image CVEs

Dead service-worker rule

  • The PWA runtimeCaching NetworkOnly rule for /api/** never matched: workbox's RegExpRoute tests the full url.href, so the ^\/api\// anchor can never hit. Replaced with an exported isApiRequest pathname matcher + tests. Currently masked (no other caching rules), but any future catch-all rule would have silently cached authenticated API responses.

All commits gate-green (biome, qlty, 100% coverage app+ui, build, e2e, playwright, zizmor).

Changelog

  • πŸ”’ Updated Node and Trivy base-image digests.
  • πŸ”’ Added the documented AVD-DS-0002 Trivy suppression for su-exec.
  • πŸ”’ Enabled Grype scans for pull requests targeting main and dev/**.
  • πŸ› Corrected stale navigation icon mappings.
  • ✨ Added @iconify-json/fa6-brands.
  • ✨ Added ten icons to icon-bundle.json.
  • ✨ Added iconMap bundle-integrity tests.
  • πŸ”§ Replaced the ineffective service-worker /api/** rule with the exported isApiRequest pathname matcher.
  • ✨ Added API request matcher tests.

Concerns

  • Confirm that the AVD-DS-0002 suppression remains valid if the Docker entrypoint or privilege model changes.
  • Confirm that excluding paths containing /api/ when they do not start with /api/ is intentional.

PR #668's icon-bundle regeneration against tabler 1.2.38 dropped
lucide:history because @iconify-json/lucide 1.2.121 no longer ships a
history glyph, leaving iconMap.audit.lucide pointing at a key the
bundle didn't have β€” a blank Audit nav icon for the Lucide preference.

- Repoint iconMap.audit.lucide to lucide:rotate-ccw-clock (same
  clock-with-counterclockwise-arrow concept as fa6-solid:clock-rotate-left
  / ph:clock-counter-clockwise already used for this entry)
- Fix the same class of stale reference found by re-running the
  extraction script: iconoir:history -> iconoir:clock-rotate-right,
  lucide:more-vertical -> lucide:ellipsis-vertical, iconoir:key-alt ->
  iconoir:key, iconoir:gitlab -> iconoir:gitlab-full, iconoir:stack ->
  iconoir:cube (all renamed upstream in the installed iconsets)
- Add @iconify-json/fa6-brands 1.2.4 as a devDependency β€” it was
  referenced by icons.ts (github/gitlab/google/microsoft brand icons
  under the fa6-solid preference) but never installed, so those four
  icons were silently missing from every bundle regeneration
- Regenerate ui/src/boot/icon-bundle.json (all 576 icon references now
  resolve, zero warnings from scripts/extract-icons.mjs)
- Add ui/tests/icons.spec.ts asserting every iconMap entry has a
  matching icon-bundle.json key, so this class of regression fails
  tests instead of shipping a blank icon
Atomic changelog entry for the icon-bundle fix committed in 73380f5d.
Both were resolved from the live Docker Hub registry (buildx imagetools
inspect + registry API cross-check), not guessed, and the resulting
image was verified with `node --version` / `trivy --version` inside a
built container plus a full `docker build` of the release stage.

- node:24-alpine: 24.18.0 -> 24.19.0, picking up Node's July 29 2026
  security release (3 HIGH + 5 MEDIUM CVEs fixed in 24.18.1)
  sha256:a0b9bf06... -> sha256:d32cdf619f...
- aquasec/trivy (trivy-bin build stage): 0.72.0 -> 0.73.0, resolving
  4 HIGH / 6 MEDIUM CVEs in vendored Go deps (go-git, x/text, grpc,
  oras-go, stdlib)
  sha256:cffe3f5161... -> sha256:7cced7cae5...
Adds root .trivyignore.yaml (trivy's documented misconfiguration
ignore format) suppressing AVD-DS-0002/DS-0002 (Dockerfile missing
USER) for the same reason the Dockerfile already carries a
checkov:skip=CKV_DOCKER_3 comment and .qlty/qlty.toml already has a
[[triage]] rule for trivy:DS002/DS-0002: Docker.entrypoint.sh drops
privileges at runtime via su-exec, so a static USER instruction isn't
needed. Unlike the qlty triage rule (only honored when trivy runs
through qlty's plugin wrapper), .trivyignore.yaml is trivy's own
native suppression mechanism, so it also covers a bare `trivy config`
invocation run directly against the Dockerfile.

Verified against the newly-pinned aquasec/trivy@sha256:7cced7cae5...
(0.73.0, see cd3b3e3e): `trivy config Dockerfile` reports 1 HIGH
(DS-0002) without the ignorefile and 0 misconfigurations with it.
workbox-routing tests a RegExpRoute's urlPattern against the full url.href
(always starting http://https://), never against the pathname alone, so
the ^-anchored /^\/api\// regex in ui/vite.config.ts's runtimeCaching
entry could never match and silently fell through. Harmless today only
because no other runtimeCaching rule exists to catch the fallthrough -
any future catch-all rule would have started caching authenticated /api
responses. Replaced with an exported isApiRequest match-callback that
tests url.pathname.startsWith('/api/') so the rule actually engages, and
covered it with direct unit tests.
@scttbnsn
scttbnsn requested a review from ALARGECOMPANY as a code owner August 8, 2026 21:25
@scttbnsn

scttbnsn commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
drydock-website Ready Ready Preview Aug 8, 2026 11:32pm
drydockdemo-website Ready Ready Preview Aug 8, 2026 11:32pm

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@scttbnsn, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 17 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
βš™οΈ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 29335bff-c031-4832-be64-f71495334f6a

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 31bce0d and f12546e.

πŸ“’ Files selected for processing (1)
  • e2e/playwright.config.ts
πŸ“ Walkthrough

Walkthrough

The PR updates pinned Trivy and Node 24 Alpine image digests and adds a scoped Trivy suppression. It refreshes UI icon dependencies, bundled definitions, and icon mappings. It adds icon bundle consistency coverage. It exports isApiRequest for Workbox API runtime caching and tests its path matching. It expands Grype pull-request scans to dev/** branches.

Possibly related PRs

πŸš₯ Pre-merge checks | βœ… 2
βœ… Passed checks (2 passed)
Check name Status Explanation
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.
✨ Finishing Touches
πŸ“ Generate docstrings
  • Create stacked PR
  • Commit on current branch
πŸ§ͺ Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/v1.7-security-hygiene

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

πŸ€– 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 `@Dockerfile`:
- Around line 4-7: Update the image-scanning workflow configuration in
security-grype.yml so pull requests targeting dev/v1.7 and changes to Dockerfile
trigger the required scan, while preserving existing main-branch and
package-manifest filters.
πŸͺ„ Autofix

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0413b945-dd69-4617-9324-f8e11910f4e1

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 93a2c8d and e4806a3.

β›” Files ignored due to path filters (2)
  • CHANGELOG.md is excluded by !CHANGELOG.md
  • ui/package-lock.json is excluded by !**/package-lock.json, !**/package-lock.json
πŸ“’ Files selected for processing (9)
  • .trivyignore.yaml
  • Dockerfile
  • app/configuration/dockerfile-defaults.test.ts
  • ui/package.json
  • ui/src/boot/icon-bundle.json
  • ui/src/icons.ts
  • ui/tests/config/vite.config.spec.ts
  • ui/tests/icons.spec.ts
  • ui/vite.config.ts

Comment thread Dockerfile

@biggest-littlest biggest-littlest left a comment

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.

LGTM

@biggest-littlest biggest-littlest left a comment

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.

LGTM

@scttbnsn
scttbnsn merged commit cee3a68 into dev/v1.7 Aug 8, 2026
29 checks passed
@scttbnsn
scttbnsn deleted the fix/v1.7-security-hygiene branch August 8, 2026 23:53
scttbnsn added a commit that referenced this pull request Aug 9, 2026
) (#686)

Forward-ports #683 to dev/v1.7. #680 fixed the symptom here (direct
icon-name renames) but not the root cause:
`ui/scripts/extract-icons.mjs` still dropped icons that iconify
collections demote to aliases (`collection.aliases[name].parent`), so
any future collection bump could silently ship broken icons again β€”
that's exactly how rc.12 shipped a blank Audit icon.

What's in the port:
- `extract-icons.mjs`: `resolveIcon()` follows alias parent chains
(depth-capped), merges alias overrides, refuses transform-requiring
aliases with a warning.
- `ui/tests/boot/icon-bundle.spec.ts`: guard test β€” every `prefix:name`
ref in `icons.ts` must exist in the bundle with a body.
- `iconoir:stack` β†’ `multiple-pages` (#680 had picked `cube`;
`multiple-pages` matches v1.6 and verified valid against locked iconoir
1.2.11).
- `@iconify-json/fa6-brands` 1.2.4 β†’ 1.2.6 exact-pinned
(devDependencies).
- Bundle regenerated: 576/576 refs, zero warnings, 229.8 KB; `npm run
build` leaves the tree clean.

4508 UI tests, 100% coverage, full pre-push gate green.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Changelog

- ✨ Added recursive Iconify alias resolution with parent-chain
traversal, override merging, depth limits, and transform warnings.
- ✨ Added a guard test for missing or empty icon bundle entries.
- πŸ”§ Replaced `iconoir:cube` with `iconoir:multiple-pages` for the
`stack` icon.
- πŸ”§ Pinned `@iconify-json/fa6-brands` to `1.2.6`.
- πŸ› Fixed extraction of indirect icon aliases.

## Concerns

- Verify aliases with rotation or flipping produce actionable warnings
and do not enter the body-only bundle.
- Keep the guard test aligned with all supported `icons.ts` reference
formats.
- Confirm the regenerated bundle contains all 576 references with
nonempty bodies.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants