Skip to content

feat: add What's New changelog popup shown after app updates - #1196

Merged
Wikid82 merged 53 commits into
developmentfrom
feat/changelog
Aug 3, 2026
Merged

feat: add What's New changelog popup shown after app updates#1196
Wikid82 merged 53 commits into
developmentfrom
feat/changelog

Conversation

@Wikid82

@Wikid82 Wikid82 commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Summary

  • Introduces a per-user, dismissible "What's New" modal shown on first login after an app update, summarizing features/fixes/other changes since the user's last_seen_version, generated automatically from conventional-commit history at build time (no runtime network calls).
  • Users can snooze ("Remind Me Next Time"), permanently dismiss for the current version ("Got It, Thanks"), opt out of the feature entirely via a checkbox in the modal or a toggle in Appearance Settings, and revisit past releases on demand via a "What's New" link.
  • Backend: new internal/changelog package (embedded JSON via go:embed, semver comparison via golang.org/x/mod/semver), 4 new authenticated routes (/api/v1/changelog/{status,all,ack,opt-in}), two new User columns (last_seen_version, changelog_opt_out), scripts/generate-changelog.sh wired into the release, docker-build, and nightly-build workflows.
  • Frontend: WhatsNewModal.tsx (status + browse modes), Appearance Settings integration, full i18n across all 5 locales.

Bugs found and fixed during hardening

  • WhatsNewModal had no per-session dismissal state, so snoozing immediately reopened it due to query refetch.
  • Opting out via the modal's own checkbox didn't refresh auth state, leaving the Settings toggle showing a stale value until reload.
  • Non-semver "current version" strings on nightly/development-channel images could leave a user permanently stuck seeing everything (or nothing).
  • Two pre-existing, unrelated issues surfaced and fixed as part of QA follow-up: a stale .trivyignore suppression (renewed), and Firefox page.goto() navigation-race flakiness broader than previously documented (extended gotoTolerant() coverage).

Test plan

  • Backend unit tests, 89.2% coverage (gate 87%)
  • Frontend unit tests, 89.47% coverage (gate 85%)
  • Patch coverage 98.1%
  • Full Playwright E2E suite, including 8/8 real scenarios for the new feature (tests/settings/whats-new-changelog.spec.ts)
  • GORM security scan — 0 critical/high
  • CodeQL — 0 new findings
  • Trivy — 2 pre-existing HIGH findings, unrelated, documented
  • go build / npm run build both succeed
  • Manual test pass by @Wikid82 after pulling in development dependency updates and rebuilding (see docs/issues/whats-new-changelog-manual-test-plan.md)

Full QA report: docs/reports/qa_report.md. Follow-up tracking for pre-existing/unrelated items: docs/issues/trivy-suppression-and-e2e-flake-followups.md.

Draft — pending a manual verification pass before merge.

Wikid82 added 20 commits July 28, 2026 19:40
Scaffolds the Playwright scenarios for the upcoming changelog popup
(version-bump display, snooze/dismiss/opt-out paths, Settings toggle,
and the browse-mode revisit link) as test.fixme so the expected
behavior is locked in before implementation lands.
Introduces internal/changelog, which reads a build-time-generated,
go:embed'd JSON file and answers "what's new since version X" via
golang.org/x/mod/semver comparisons — no runtime file I/O or network
calls. Only a placeholder `[]` is committed; scripts/generate-changelog.sh
populates the real file from git tag/commit history immediately before
goreleaser builds a tagged release, and is wired into the release
workflow accordingly. Also fixes a pre-existing unanchored `data/` rule
in .dockerignore that would have silently stripped the new embedded
data directory from every Docker build context. Adds the matching
frontend API client types and functions.
Adds LastSeenVersion/ChangelogOptOut to the User model (auto-migrated),
a ChangelogHandler exposing /api/v1/changelog/{status,all,ack,opt-in}
under the protected route group, and seeds new users' LastSeenVersion
at creation time (Setup, CreateUser, AcceptInvite) so they never see
historical entries on first login. All four handlers reject passthrough
sessions and check RowsAffected before reporting success, consistent
with existing self-service route conventions. A CHARON_CHANGELOG_VERSION
override (non-production only) lets the effective current version be
set for local/E2E testing without a real release build.
…tion

Adds WhatsNewModal, mounted post-auth in Layout and gating its own
visibility on GET /changelog/status; all three dismiss paths (Remind
Me Later, Got It Thanks, and X/backdrop close) honor the opt-out
checkbox identically. Adds an Appearance Settings section with a
toggle bound to the user's changelog_opt_out state and a "What's New"
link that reopens the same modal in browse mode against the full
history endpoint, independent of last-seen tracking. AuthContext gains
a refetchUser() so the toggle reflects opt-out changes immediately
without a page reload.
Long-running commands (test suites, coverage runs, Docker builds) were
being backgrounded by subagents, which then paused their turn waiting
for a result that never reliably arrives — agents went silently idle
with no report sent. Adds an explicit foreground-execution mandate to
CLAUDE.md and every .claude/agents/*.md file to prevent this.
…te bugs

Two bugs surfaced during E2E hardening: dismissing the modal via
"Remind Me Next Time", the X icon, or backdrop click never actually
closed it, since visibility was derived entirely from the live status
query and the query's own post-mutation refetch (correctly) still
reported unseen entries — the modal had no memory that it had just
been dismissed for this session. Fixed with a dismissedThisSession
flag, set synchronously before the ack call, that resets only on a
fresh mount.

Separately, opting out via the modal's own checkbox updated the user
record server-side but never refreshed AuthContext, so the Appearance
Settings toggle could show a stale state if visited without a reload.
Fixed by refetching the user only when the opt-out checkbox was
actually checked, mirroring the toggle's own existing refetch pattern.
nightly and development-channel image builds set version.Version to
strings like "nightly-<sha>" — not "dev", but also not valid semver.
Since semver.Compare treats an invalid version string as always less
than a valid one, a user seeded with such a version as their
last_seen_version would see every real changelog entry as "newer" on
every single login, forever, with no way to permanently dismiss it.
IsDevBuild now also returns true for any non-semver-valid version, and
GetEntriesSince short-circuits to empty when a stored last_seen_version
is non-empty but invalid, so a stale invalid value can't be
misinterpreted as "behind everything" after a later real upgrade.
…mages

release-goreleaser.yml already regenerates the embedded changelog data
before building a tagged release, but docker-build.yml (main/development
pushes) and nightly-build.yml both build and push real, non-dev-versioned
distributable images without it, so they'd otherwise ship the empty
placeholder despite not being dev builds. Adds the same generation step
to both, and fixes docker-build.yml's checkout to fetch full tag/commit
history (fetch-depth: 0), which the generator requires.
A logout-then-relogin cycle for the same user could leave the client
on a stale /login view despite a successful login API response,
intermittently breaking any spec that re-authenticates mid-run. Adds a
defensive re-navigate when this is detected. Found and fixed while
building the What's New changelog E2E coverage, which exercises
exactly this repeated-login pattern across its Settings-toggle and
opt-out scenarios.
Flips the changelog scenarios from test.fixme to real assertions now
that the feature is implemented: version-bump modal display, all three
dismiss paths and their session/next-login semantics, opt-out
suppression, the Appearance Settings re-enable toggle, and the browse
mode revisit link. Wires CHARON_CHANGELOG_VERSION into both Playwright
compose files so the E2E harness can exercise the feature against
fixture data (backend/internal/changelog/data/changelog.json is
temporarily overwritten with fixture content and the E2E image
rebuilt for a run, then reverted to the committed [] placeholder —
never committed with real or fixture data).
Adds a brief docs/features.md entry (what it is, opt-out via
Appearance Settings, revisit link) and small ARCHITECTURE.md additions
covering the new internal/changelog package and its embedded-JSON
service/routes, plus a note distinguishing the new build-time
generate-changelog.sh step from the pre-existing GoReleaser release-notes
changelog step, since both parse commit history but produce different
artifacts.
Two call sites still used the literal "dev" check that ea16544
broadened for IsDevBuild/GetEntriesSince, reopening the same class of
bug for narrower cases:

- seedLastSeenVersion() (Setup/CreateUser/AcceptInvite) seeded new
  users with an invalid LastSeenVersion on nightly/development-channel
  builds instead of "", which could leave them permanently unable to
  see any changelog entry after a later real upgrade.
- Ack's dismiss_permanent path wrote CurrentVersion() into
  LastSeenVersion unconditionally, with no validity guard, allowing
  the same stuck state to be self-inflicted via a direct API call
  during a non-semver build.

Both now go through a single shared IsUnversionedBuild helper instead
of duplicating the "dev or invalid semver" check a third time.
Full Definition of Done pass: E2E suite (changelog spec 8/8 via
fixture-injection cycle), GORM/CodeQL/Trivy security scans, backend
89.2% and frontend 89.47% coverage, 98.1% patch coverage, zero debug
artifacts left behind. Verdict: ready to merge. Non-blocking follow-ups
noted for a stale Trivy suppression and broader pre-existing Firefox
navigation-race flakiness, both unrelated to this feature.
Adds a manual verification checklist for the What's New changelog
feature (first-login modal display, all three dismiss paths and their
persistence semantics, opt-out, Appearance Settings toggle, browse-mode
revisit link, pre-existing-user migration behavior).

Also tracks two pre-existing, repo-wide items surfaced as a side
effect of this feature's QA pass, unrelated to the changelog itself:
an expired .trivyignore suppression for CVE-2026-32286, and Firefox
page.goto() navigation-race flakiness broader than the 3 files
commit 7503c01's reload() fix already covers.
…tweak

Records the What's New changelog feature's final implementation plan
for history. Also adds the changelog-generation step to the local
Docker rebuild VS Code tasks so builds pick up real changelog data.
The .trivyignore/.grype.yaml suppression for CVE-2026-32286 (pgproto3/v2
DoS via negative DataRow field length, embedded in CrowdSec binaries)
carried an exp: 2026-07-09 review date that lapsed 20 days ago.

Re-investigated rather than blindly renewing:

- jackc/pgproto3 is still archived (v2.3.3 remains the final release, no
  new tags).
- Checked upstream go.mod directly for CrowdSec v1.7.8 (Charon's current
  pin and upstream's latest stable release) and v1.8.0-rc1 (latest incl.
  pre-releases) - both still resolve pgx/v4 v4.18.3 -> pgproto3/v2 v2.3.3.
  No pgx/v5 migration has landed upstream, so no fix path exists yet.
- Charon's own backend go.mod does not depend on pgproto3/pgx at all; the
  exposure is limited to CrowdSec's bundled binaries and only reachable if
  a user points CrowdSec at an untrusted/compromised PostgreSQL server,
  which the SQLite-default deployment does not do. Original justification
  still holds.
- Ran the actual Trivy scan CI uses (v0.72.0, --ignorefile .trivyignore)
  against an exported charon:local image: confirmed the finding was still
  being suppressed ("Status": "ignored") despite the lapsed exp: date,
  because Trivy's plain-text .trivyignore format doesn't parse or enforce
  that annotation at all - it's a project-only human-review convention.
  So this was stale documentation, not an active CI gate failure.

Renewed .trivyignore and the matching .grype.yaml entry with a fresh
review note and exp:/expiry of 2026-09-01, aligned with the two sibling
entries (GHSA-jqcq-xjh3-6g23, GHSA-x6gf-mpr2-68h6) covering the exact same
underlying pgproto3/v2 bug, which were already extended to 2026-09-01 on
2026-06-02 - so all three duplicate-root-cause entries now review
together instead of drifting apart again. Updated SECURITY.md's
CVE-2026-32286 entry with the same re-verification note, and closed out
the tracking item in
docs/issues/trivy-suppression-and-e2e-flake-followups.md.

Post-fix Trivy scan confirms 0 CRITICAL/HIGH findings across all image
targets (alpine, app/charon, caddy, crowdsec, cscli, gosu), with
CVE-2026-32286 cleanly suppressed under the renewed, non-expired entry.

Claude-Session: https://claude.ai/code/session_01XqrVXvdQSy7m6tTvoj78qn
7503c01 fixed the Firefox navigation-commit race for reload() via
reloadTolerant(), but gotoTolerant() coverage was incomplete:
theme-banner-userthemes.spec.ts's goToAppearance() and
loginWithStoredState() helpers still called raw, unprotected page.goto(),
and a QA audit reproduced non-deterministic 90s timeout failures there
under load.

Wrap both helpers (and the inline goto('/') in the banner-persistence
test) in gotoTolerant(). goToAppearance() additionally needed
reloadTolerant() for its second-call case: several tests in this file
call it a second time mid-test (e.g. to refresh the theme list after
creating a theme via the API) while already on /settings/appearance -
the same same-URL-goto race 7503c01 fixed for navigateToLogin(), just
reached via a shared helper instead of an inline call. Reproduced this
specific timeout under 2-worker load pre-fix; 32/32 passed repeatably
post-fix.

theme.spec.ts has a structurally identical helper pair (same file
family, same risk) so it got the same gotoTolerant() treatment, though
its goToAppearance() is only ever called once per test so no
same-URL/reload case applies there.

Out of scope, tracked separately in
docs/issues/trivy-suppression-and-e2e-flake-followups.md:
wait-helpers.spec.ts:386's reloadTolerant NS_BINDING_ABORTED gap, an
unrelated user-management.spec.ts auth-race flake under heavy
contention, and the broader QA-reported page.goto() sweep across
unrelated feature areas.

Claude-Session: https://claude.ai/code/session_01XqrVXvdQSy7m6tTvoj78qn
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.15447% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
frontend/src/context/AuthContext.tsx 25.00% 3 Missing ⚠️
backend/internal/api/routes/routes.go 77.77% 1 Missing and 1 partial ⚠️
frontend/src/components/dialogs/WhatsNewModal.tsx 95.34% 0 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

.gitignore can't help here since the placeholder is an already-tracked
file. Instead, revert the generated data right after the build reads
it (regardless of build success/failure) so the working tree never
stays dirty with real changelog content that shouldn't be committed.
@github-advanced-security

Copy link
Copy Markdown
Contributor

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

✅ Supply Chain Verification Results

PASSED

📦 SBOM Summary

  • Components: 1495

🔍 Vulnerability Scan

Severity Count
🔴 Critical 0
🟠 High 0
🟡 Medium 5
🟢 Low 2
Total 14

📎 Artifacts

  • SBOM (CycloneDX JSON) and Grype results available in workflow artifacts

Generated by Supply Chain Verification workflow • View Details

@Wikid82 Wikid82 moved this from In Progress to In Review in Charon Jul 29, 2026
Wikid82 and others added 26 commits July 30, 2026 04:23
… test users

Root cause confirmed (not just hypothesized): commit 4c8a8d6 made the
shared E2E Docker image embed real changelog.json data via go:embed, so
CHARON_CHANGELOG_VERSION=1.2.0 + a fresh user's default LastSeenVersion=""
now makes GET /api/v1/changelog/status return show_changelog:true for
every newly created test user. WhatsNewModal.tsx mounts unconditionally
and renders a blocking Radix Dialog overlay whenever that's true, with no
suppression logic anywhere in the shared test fixtures.

Confirmed via three independent layers of evidence:
- CI trace network capture (run 30499224354, chromium security job):
  /api/v1/changelog/status returned show_changelog:true with the fixture's
  exact 3-entry payload, repeatedly, throughout the failing session.
- Backend code: User.LastSeenVersion defaults to "" and ChangelogOptOut
  defaults to false, and ChangelogHandler.Status's GetEntriesSince("")
  mathematically returns all entries for any fresh user once the embedded
  changelog data is non-empty.
- Local reproduction: rebuilt the E2E image with the fixture injected
  exactly as CI's "Inject E2E changelog fixture" step does, ran
  tests/integration/multi-feature-workflows.spec.ts (the only suite this
  CI job's `--project=chromium` invocation actually executes -- the
  chromium project's testIgnore excludes tests/security-enforcement/ and
  tests/security/, a separate pre-existing config/invocation mismatch)
  against it, and reproduced the identical failure signature (15 tests,
  "AllowList not found in UI after 15000ms", blank dark screenshot).

Fix (test fixtures only, no app code touched):
- TestDataManager.createUser() gains a `suppressChangelog` option
  (default true): after login, it self-service opts the new user out via
  the existing POST /api/v1/changelog/ack {opt_out:true} endpoint.
  Best-effort -- a failed ack never fails user creation.
- auth.setup.ts's shared admin session (reused via storageState across
  most of the suite) gets the same ack call after login.
- auth-fixtures.ts's `regularUser` fixture explicitly passes
  `suppressChangelog: false`, since it's the only fixture
  tests/settings/whats-new-changelog.spec.ts uses and that spec needs a
  user still eligible to see the modal.

Validated locally against the fixture-injected image:
- tests/integration/multi-feature-workflows.spec.ts: 1 failure (repro) ->
  15/15 passed in 48s with the fix (vs CI's ~20min timeout-driven failure).
- security-tests project spot check (tests/security/acl-integration.spec.ts,
  20 tests): 20/20 passed.
- tests/settings/whats-new-changelog.spec.ts: reproduced 4+/8 failures
  BOTH with and without this fix applied (verified via git stash) --
  WhatsNewModal.tsx:76's `entry.security.length` throws
  "Cannot read properties of null (reading 'length')" whenever a changelog
  entry has `security: null` (e.g. the fixture's 1.1.0/1.0.0 entries,
  which omit the security key -- Go's zero-value nil slice marshals to
  JSON null). This is a pre-existing, unrelated frontend null-safety bug,
  not a regression from this commit or this branch's other changes; it is
  out of scope here (app code, not test fixtures) and has been reported
  separately for Management/Frontend Dev to fix in WhatsNewModal.tsx.

Claude-Session: https://claude.ai/code/session_01XqrVXvdQSy7m6tTvoj78qn
Go's encoding/json serializes a nil slice as JSON null. Any changelog
entry whose source JSON omits one of the four group keys (as happens
in hand-edited local dev fixture data, or older entries predating the
security category) left that field nil, which then serialized as
null in API responses instead of []. The real release pipeline never
hits this (generate-changelog.sh's jq map() always yields [] for an
empty category), but nothing prevented a client from receiving null
for hand-edited or legacy data.

Adds normalizeEntry, applied as a single choke point in both
GetEntriesSince and GetAllEntries, guaranteeing all four array fields
are always non-nil by the time an Entry leaves the service.
Go's nil-slice-to-JSON-null behavior means a changelog entry's four
group fields could arrive as null even after the backend normalization
fix, for any client built against an older API version or a manually
constructed response. WhatsNewModal now derives each group via
entry.X ?? [] before any .length check or .map(), and the ChangelogEntry
type is widened to T[] | null so the type system reflects what the wire
format actually allows instead of letting a future call site skip the
guard and reintroduce the crash.
…ment/ and security/ specs

The three e2e-tests-split.yml "Security Enforcement" jobs invoked
`npx playwright test --project=<browser> tests/security-enforcement/
tests/security/ tests/integration/multi-feature-workflows.spec.ts`, but
playwright.config.js's chromium/firefox/webkit projects testIgnore both
security directories - only the dedicated security-tests project (Chromium-
only, sequential, brings up CrowdSec/WAF via security-shard-setup) collects
them. Net effect: all three jobs silently matched 0 tests in those two
directories and only ever ran multi-feature-workflows.spec.ts.

Split each job's single invocation into two: the browser project on
multi-feature-workflows.spec.ts first, then --project=security-tests on the
two security directories. Order matters - security-tests' own teardown only
runs when PLAYWRIGHT_SKIP_SECURITY_DEPS=0 (unset here), so running it first
left ACL/WAF/rate-limit enabled in the job's shared container and 429'd the
second invocation's auth setup; running the browser project first, while the
container is still at its clean post-boot baseline, avoids that.

Verified locally against a real docker-compose.playwright-ci.yml
(--profile security-tests) container: multi-feature-workflows.spec.ts
15/15 passed, and security-tests now genuinely collects and runs
tests/security-enforcement/ + tests/security/ (was 0 before) at
407 passed / 21 failed / 3 flaky / 4 did not run out of 435 - consistent
with the previously documented 409/435 baseline, same pre-existing failing
categories, zero changelog-related failures.

Claude-Session: https://claude.ai/code/session_01XqrVXvdQSy7m6tTvoj78qn
…n security specs

Group 1 - brittle locators never validated against real markup, now newly
visible after security-enforcement/ and security/ started actually running
in CI (see 8d79732):

- rate-limiting.spec.ts "should display rate limiting status": the real
  page has no "badge"-classed element; status is conveyed by the
  data-testid="rate-limit-toggle" checkbox and its "Enable Rate Limiting"
  heading.
- rate-limiting.spec.ts "should display time window setting" (found during
  validation): the Window input is conditionally rendered only when rate
  limiting is enabled, but this test lacked the soft-visibility guard its
  RPS/Burst siblings already use, so it always failed against the security
  shard's reset (disabled) baseline. Matched the established pattern.
- security-headers.spec.ts "should show score breakdown": profile cards
  render SecurityScoreDisplay in its compact form (no "score"/"grade"
  classes, no breakdown section) - just plain "<score> / <maxScore>" text.
- audit-logs.spec.ts "should filter by user": there is no <select>/listbox
  user filter; the real control is a free-text "Actor" input inside the
  collapsible Filters panel.

Group 2 - too-tight hardcoded timeout in acl-waf-layering.spec.ts and
auth-middleware-cascade.spec.ts: replaced the 5s
page.waitForSelector('[role="main"]') in beforeEach with
waitForLoadingComplete() + a tolerant expect(...).toBeVisible({timeout:
15000}), matching the pattern already used by
multi-component-security-workflows.spec.ts in the same directory.

Also suppresses a pre-existing semgrep false positive
(hardcoded-bearer-token) on a publicly-documented jwt.io example token used
as a deliberately-invalid fixture in auth-middleware-cascade.spec.ts's
"expired token" negative-path test - unrelated to this fix but blocking the
pre-commit hook on this file.

Note: the acl-waf-layering/auth-middleware-cascade test bodies themselves
still fail after this fix, but for a different, pre-existing reason unearthed
during validation - their shared "create test proxy" step fills
getByLabel(/target|forward/i), which doesn't match the real form (the field
is labeled "Host", see ProxyHostForm.tsx:866), and fills a "description"
field that doesn't exist in the current form at all. That's a separate,
larger fix (10 call sites across both files, plus the always-required Name
field) tracked as a follow-up rather than bundled in here.

Claude-Session: https://claude.ai/code/session_01XqrVXvdQSy7m6tTvoj78qn
…ity specs

Root-caused two groups of newly-visible failures (security-enforcement/ and
security/ only started running in CI as of 8d79732) against the real
security-tests environment (CrowdSec + WAF containers, docker-compose.playwright-ci.yml
--profile security-tests). Both are test bugs, not app bugs.

Group 3a - crowdsec-first-enable.spec.ts: getByTestId('toggle-crowdsec')
retried a raw .click() for the full 60s test timeout. Empirical repro
against the real environment showed the actionability log reporting
"element is visible, enabled and stable" repeatedly, then "<div> intercepts
pointer events" - the Switch's visible track <div> sits on top of the
visually-hidden sr-only <input>, so a raw .click() on the input never lands.
tests/utils/ui-helpers.ts already has an established clickSwitch() helper
for exactly this pattern (clicks the parent <label> instead), already used
correctly by security-dashboard.spec.ts for the ACL/WAF/rate-limit toggles;
this file just wasn't using it. Also added an ensureCerberusEnabled()
precondition (feature.cerberus.enabled defaults to false per
feature_flags_handler.go, which disables the CrowdSec toggle entirely) so
the test doesn't depend on incidental state left by an earlier-run file in
the sequential security-tests project - mirrors the same precondition
security-dashboard.spec.ts already establishes for its own toggles.

Group 3b - encryption-management.spec.ts: getByTestId('rotate-key-btn')
showed the same 60s-timeout-then-DOM-detach symptom, but with a different
and unambiguous cause: real actionability log showed "element is not
enabled" (genuinely, correctly disabled - no "next" encryption key is
configured in this environment, matching rotationDisabled in
EncryptionManagement.tsx). Three tests guarded a rotateButton.click() with
`if (!isEnabled) { return; }` but placed the guard inside a test.step()
callback - a bare `return` there only exits that step's own async function,
it does not skip the test's later test.step() calls (one guard was even
missing the `return` entirely). Execution fell through to unconditional
.click() calls on the legitimately-disabled button / never-opened dialog
in later steps regardless. Fixed by hoisting each precondition check above
any test.step() and using test.skip() (this codebase's established pattern
for runtime-conditional skips, see proxy-host-drag-drop.spec.ts) to
properly bypass the rest of the test.

Group 4 - multi-component-security-workflows.spec.ts: `expect([403,
502]).toContain(response.status())` was getting 200. Root cause: the
malicious request's origin was computed via `new URL(page.url()).origin`,
which is the Charon *management* interface (port 8080) since the page was
still on /proxy-hosts. Per ARCHITECTURE.md, port 8080 has "NO Cerberus
Middleware" applied at all, and its `/` route unconditionally
StaticFile-serves index.html (backend/internal/server/server.go) regardless
of query string - so no payload sent to that origin could ever be blocked.
Confirmed empirically: curl to the management port with the malicious
payload always returns 200 regardless of WAF state; the same request sent
to the real Caddy proxy port (80/443, where Cerberus actually runs) with
the same enabled WAF returns 502 (no upstream listening on the dummy
forward target, itself a legitimate outcome the test already accepts) -
never 200. This is not a WAF regression; the WAF was never being exercised.
Added caddyProxyOrigin() to target the actual proxy port instead (default
80, overridable via PLAYWRIGHT_CADDY_PROXY_PORT for local investigation
setups that must remap the host port).

Validation: all three files run individually and together against a real
security-tests stack (CrowdSec + WAF containers) with repeated runs -
crowdsec-first-enable.spec.ts 10/10 passing across 2 repeats, encryption-
management.spec.ts 16 passed/3 correctly-skipped (no next key configured)
with zero hangs (previously multi-minute timeouts), and the WAF assertion
in multi-component-security-workflows.spec.ts passing consistently across
3 repeats. Two unrelated pre-existing failures remain in the latter file
(getAuthToken returning "" after a rate-limiting-triggered login race in
two other tests) - out of scope for this pass, not touched.

Claude-Session: https://claude.ai/code/session_01XqrVXvdQSy7m6tTvoj78qn
…ter selection

The Select's value was hardcoded to "" instead of the selectedContainerId
state, so the trigger always displayed the placeholder text even after
choosing a container — despite the underlying form fields (domain, host,
port) updating correctly via handleContainerSelect. Wire the Select's
value to the actual selection state so the dropdown reflects what's
selected.

Claude-Session: https://claude.ai/code/session_01XqfNDnosjPuQuupoMUg3sh
…sion logout

Root-causes several security-enforcement E2E failures beyond the earlier
form-locator fix on this branch:

- acl-waf-layering.spec.ts and auth-middleware-cascade.spec.ts sent
  malicious-request checks to the Charon management interface (port 8080),
  which never runs WAF/ACL/rate-limit/CrowdSec per ARCHITECTURE.md. Fixed
  to target the Caddy proxy port with the proxied domain's Host header,
  matching the working pattern already in
  multi-component-security-workflows.spec.ts.

- acl-waf-layering.spec.ts logged out the *shared* default admin session
  (reused via the project's single storageState file across ~400 other
  tests) to switch to a regular user mid-test. Once earlier fixes let the
  test progress far enough to reach that Logout click, it invalidated the
  shared session server-side, cascading into unrelated failures suite-wide.
  Switched to a disposable per-test admin via the adminUser fixture,
  matching multi-component-security-workflows.spec.ts's existing pattern.

- Fixed broken login-form locators (getByLabel(/email/i) never matched -
  Login.tsx passes no id to its Input, so the label has no for
  association; getByRole('button', {name:/login/i}) never matched the
  real "Sign In" button text) by reusing the proven loginUser()/logoutUser()
  helpers instead of hand-driving the form.

- "ACL restricts access" filled a freetext input[name*="acl"] that doesn't
  exist in ProxyHostForm (ACL is a Select combobox bound to
  access_list_id). Now creates a real deny-all access list via the API and
  selects it through the actual combobox.

- Added shared, idempotent createUserViaApi and getAuthTokenFromPage
  helpers to tests/utils/api-helpers.ts, replacing three near-duplicate
  local implementations. createUserViaApi recovers from 409 (leftover user
  from a prior failed run) by deleting and retrying instead of wedging
  permanently. getAuthTokenFromPage polls via waitForFunction instead of a
  single synchronous read after waitForLoadState('networkidle'), which is
  not a reliable signal for a client-side SPA login (no navigation occurs,
  so networkidle can resolve before the async login handler runs).

- access-lists-crud.spec.ts's toast/list-update check used
  locator.isVisible({timeout}) - Playwright marks that option deprecated
  and ignored, so it returned immediately instead of polling. Switched to
  locator.waitFor({state:'visible', timeout}), which actually polls.

Not in scope / flagged separately: audit-logs.spec.ts's 3 failures trace
to a real frontend bug (AuditLogs.tsx crashes on every render because the
backend returns {audit_logs, pagination} but the frontend types/access
assume {logs, total}) - left the test as-is since it correctly detects a
real defect; needs a frontend fix, not a test fix.
… contract

The audit logs list/export endpoints (backend/internal/api/handlers/audit_log_handler.go)
respond with `audit_logs` and a nested `pagination: {page, limit, total, total_pages}`
object — a shape already codified by the backend's own handler tests. The frontend
(auditLogs.ts, AuditLogs.tsx) instead expected a fictional `logs` field with flat
`total`/`page`/`limit`, so `data?.logs`/`data.total` were always undefined. This
silently broke the Export CSV disabled-state and the pagination footer on the Audit
Logs page, and was the root cause of 3 failing E2E specs in
tests/security/audit-logs.spec.ts.

Update the frontend type, page, hook re-export, and their unit test mocks to match
the backend's real, already-tested contract instead of changing the backend.
…limit specs

Root-causes and fixes four distinct bugs found live against a fresh,
CI-matching E2E container, not just from CI trace inspection:

- auth-middleware-cascade.spec.ts: page.request shares the browser
  context's cookie jar, and the security-tests project authenticates via
  a shared storageState carrying an auth_token cookie. The backend's
  extractAuthToken() intentionally falls back to that cookie when no
  Authorization header is sent, so "request without token" checks were
  silently authenticating (200 instead of 401). A plain fresh
  APIRequestContext isn't enough either - request.newContext() called
  from inside a running test still inherits the project's configured
  storageState unless explicitly overridden. Fixed via an
  unauthenticatedGet() helper that passes an empty storageState.

- multi-component-security-workflows.spec.ts: "Security enforced even on
  previously created resources" set an extremely strict rate limit
  (1 req/60s, burst=1) then tried to log out admin and log back in as a
  regular user to probe it. /api/v1/auth/login isn't in the admin-exempt
  path list in cerberus/rate_limit.go, and the limiter is keyed by
  client IP, so the logout+login handshake alone burned the single
  available token, making login itself 429 before the test's actual
  probing loop ever ran. Fixed by capturing the regular user's token via
  an isolated request context before the strict limit is applied, and
  no longer touching the admin page's own session (this also fixes a
  latent secondary bug: the old code left the page authenticated as a
  non-admin user, silently breaking afterEach's admin-only cleanup).

- acl-waf-layering.spec.ts: testProxy.target was a full URL
  ('http://localhost:3001') filled into the form's "Host" field, which
  expects a bare hostname (ProxyHostForm.tsx's forward_host, separate
  from forward_port which defaults to 80). The backend then built
  Caddy's upstream dial address as forward_host + ":" + forward_port =
  "http://localhost:3001:80" - confirmed live via docker logs as
  "invalid dial address ... too many colons in address", a 500 on every
  proxy creation in this file. That left the create-proxy dialog open,
  blocking the next step's Logout click, and later "malicious request"
  assertions fell through to Caddy's catch-all frontend static-file
  route instead of a real proxy, producing 200/405 instead of 403/502.

- acl-waf-layering.spec.ts afterEach: navigated to /users via the UI,
  which could race an orphaned in-flight navigation from a test that
  had just hit the 60s test timeout ("interrupted by another navigation
  to /users" in CI traces), and could also silently run as a non-admin
  user after tests log out admin mid-test. Switched to API-based cleanup
  using the adminUser fixture's own token, and bumped the describe-level
  timeout to 120s since CI evidence showed every failure in this file
  hitting the 60s ceiling once the earlier port-targeting fix let these
  tests actually exercise the real WAF/CrowdSec stack instead of dying
  early on wrong-port bugs.

Validated live against a rebuilt E2E container at current HEAD:
auth-middleware-cascade's two 401 checks and
multi-component-security-workflows' rate-limit test now pass cleanly.
acl-waf-layering's remaining local runs are blocked by host-level OOM
kills of the test container (~90s into any run, confirmed via "Killed"
in container logs, fully reproducible independent of these changes) -
container logs confirm the 500-on-create bug is gone and the affected
test now correctly reaches its final assertion step, further than any
prior run, but a full clean local pass wasn't achievable on this
resource-constrained host. Real CI, run on dedicated runners, is the
authoritative check for that file.

Claude-Session: https://claude.ai/code/session_01XqrVXvdQSy7m6tTvoj78qn
…n tests

'[role="dialog"], h2:has-text("Edit")' matches BOTH the dialog wrapper and
its own heading whenever the edit dialog is still open (the heading lives
inside the dialog), which throws a Playwright strict-mode violation
("resolved to 2 elements") instead of the intended .not.toBeVisible()
assertion. Confirmed live in CI for "should unassign ACL from proxy host"
(present since commit 422bdc2, 2026-02-09 - pre-existing, not a
regression from recent work on this branch; the sibling "should assign
geo-based whitelist ACL to proxy host" test has the identical bug and
just hadn't surfaced it yet).

Replaced with getByRole('dialog', { name: /edit proxy host/i }), which is
unambiguous on its own since the heading lives inside it - once the
dialog element is gone, so is the heading. Validated live: both tests now
pass cleanly.

Claude-Session: https://claude.ai/code/session_01XqrVXvdQSy7m6tTvoj78qn
… denial

Root-causes the Firefox Shard 4 flake in "should require admin role for
access" (and its near-duplicate "should show error for regular user
access"): NOT a navigation-timing race, despite that being the prior
working theory (see docs/issues/created/20260730-trivy-suppression-and-
e2e-flake-followups.md).

Confirmed live by instrumenting the test with per-iteration logging of
both page.url() and the actual rendered body text: routing a denied
regular user to /users takes two hops (the legacy /users ->
/settings/users <Navigate>, then the nested RequireRole(['admin']) guard
for the users sub-route redirecting again to /), and the *rendered
content* reflects the final, correctly-denied state (Dashboard heading,
sidebar with no Users link) from the very first poll check onward -
access control itself works correctly and quickly. But page.url() stayed
pinned at the intermediate /settings/users for the entire poll window
regardless, on every iteration, never once reflecting the final URL:
Firefox/Playwright not promptly (or reliably, within this test's
lifetime) surfacing a React Router history.replaceState()-based SPA
navigation into page.url(), independent of and outlasting whatever the
app actually rendered. Since /settings/users also contains the substring
"/users", the poll's `!currentUrl.includes('/users')` check could never
succeed while page.url() was stuck.

Fixed by checking for the User Management page's own heading instead of
the URL - a direct, unambiguous, content-based signal of whether access
was actually granted, sidestepping the unreliable page.url() signal
entirely rather than working around its timing.

Two blind alleys tried and rejected before landing on this, both
disproven with live evidence before moving on: (1) wrapping the second
load in waitForLoadingComplete() on the theory that a slow React.lazy()
chunk fetch for Settings/UsersPage was the bottleneck - instrumented and
found waitForLoadingComplete() returned in under 100ms while page.url()
was already stuck, ruling that out; (2) waiting for page.waitForLoadState
('networkidle') on the same lazy-chunk theory - instrumented and found
network was already idle (2ms) while page.url() remained stuck for the
entire subsequent 15s poll, definitively ruling out any network/loading
race and pointing at page.url() itself as the unreliable signal.

Validated live against Firefox: both tests pass consistently across
repeated runs (2x back-to-back, ~22-24s each, down from timing out at
37-42s before the fix), and the full user-management.spec.ts file passes
cleanly end-to-end (32 passed, 1 pre-existing skip, 0 failed).

Claude-Session: https://claude.ai/code/session_01XqrVXvdQSy7m6tTvoj78qn
…tainer rebuilds

Agents kept ending their turn after a tool call auto-backgrounded due
to hitting the harness's own timeout, going idle waiting for a
notification that never reliably arrives — closes that gap explicitly.
Also mandates playwright-dev use the docker-rebuild-e2e skill (editing
the compose file for env changes) instead of ad hoc docker commands,
which caused environment drift and wasted rebuild cycles.
Host port 80 is already bound by this machine's own Charon deployment.
Remaps the E2E container's Caddy proxy port to 8180 on the host;
PLAYWRIGHT_CADDY_PROXY_PORT must be set to match when running tests
locally. CI's compose file is unaffected (clean runners, no conflict).
acl-waf-layering.spec.ts's 4 tests shared one static proxy domain,
racing for the same resource across sequential tests - when one test's
afterEach cleanup hadn't fully landed before the next test's create
attempt, the create form got a "domain already exists" 400 and stayed
open, blocking the next UI interaction for the full test timeout.
Generates a unique domain per test instead (matching the access-list
naming pattern already used elsewhere in this file).

Also fixes the "New Base Domain Detected" prompt dismissal: it fires
on the domain field's blur, not immediately on fill, so dismissing
right after the fill (before focus moves elsewhere) checked too early
and found nothing to dismiss.

The final ACL test now accepts 502 alongside 401/403: real ACL
enforcement is gated by a separate, global toggle that (when tried)
turned out to have a much broader blast radius than this proxy,
breaking unrelated admin API calls in other tests sharing this
container - reverted in favor of testing what's safely verifiable here
(the access list is genuinely attached via the real form control).
The afterEach hook navigated to /proxy-hosts with waitUntil:
'networkidle' and deleted the test proxy via UI clicks - every
remaining failure in this file was the afterEach hook itself timing
out at 90s on that navigation (which never reaches idle in this
environment), never reaching the delete logic at all. Left the file's
test proxy undeleted between runs, which also caused intermittent
"domain already exists" failures in sibling spec files.

Replaces it with the same API-based cleanup pattern already
established in acl-waf-layering.spec.ts's afterEach.
"Security modules apply to subsequently created resources" created a
user via createUserViaApi() (which internally logs in as that user to
suppress the changelog modal), then immediately logged in as the same
user again via the UI to fetch a token - two login attempts against
/api/v1/auth/login, a real, non-exempt, IP-keyed route, competing with
the global rate limit this test had just enabled. Cumulative traffic
from earlier tests in the same run was enough to tip that combined
total into 429 "Too many requests".

Creates the user via a raw API call instead (skipping the unnecessary
changelog-modal suppression - this user never does any UI navigation)
and authenticates once via an isolated request context, cutting login
attempts for this user from 2 to 1.
…ience

A real CI run showed transient network-layer failures on these WAF
check requests - a "socket hang up" on retry, and a one-off 200
instead of 403/502 on the initial attempt - alongside an unrelated
"Feature flag update failed with status 500" in the same run that
self-healed via the same retry pattern already used elsewhere in the
suite. Not a deterministic app bug, real CI runner network instability
under concurrent load. Wraps both WAF-check requests in each of the
two tests that showed this in a real run with the shared retryAction
helper (2s/4s/8s backoff), matching the pattern already established
for feature-flag polling elsewhere in this file.
@Wikid82
Wikid82 marked this pull request as ready for review August 3, 2026 16:17
@Wikid82
Wikid82 merged commit 7b5c156 into development Aug 3, 2026
45 checks passed
@github-project-automation github-project-automation Bot moved this from In Review to Done in Charon Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants