akanealw/Charon
1
0
Fork 0
Code Issues Pull Requests Actions 16 Packages Projects Releases Wiki Activity

fix: ensure ACL and Security Headers dropdown selections persist correctly in Proxy Host form

Browse Source
This commit is contained in:
GitHub Actions
2026-02-27 21:57:05 +00:00
parent feaae052ac
commit 75d945f706
7 changed files with 636 additions and 178 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- Fixed: Added robust validation and debug logging for Docker image tags to prevent invalid reference errors.
- Fixed: Removed log masking for image references and added manifest validation to debug CI failures.
- **Proxy Hosts**: Fixed ACL and Security Headers dropdown selections so create/edit saves now keep the selected values (including clearing to none) after submit and reload.
- **CI**: Fixed Docker image reference output so integration jobs never pull an empty image ref
- **E2E Test Reliability**: Resolved test timeout issues affecting CI/CD pipeline stability
- Fixed config reload overlay blocking test interactions

52
backend/internal/api/handlers/proxy_host_handler.go
View File

@@ -453,54 +453,12 @@ func (h *ProxyHostHandler) Update(c *gin.Context) {
// Security Header Profile: update only if provided
if v, ok := payload["security_header_profile_id"]; ok {
logger := middleware.GetRequestLogger(c)
// Sanitize user-provided values for log injection protection (CWE-117)
safeUUID := sanitizeForLog(uuidStr)
logger.WithField("host_uuid", safeUUID).WithField("raw_value", sanitizeForLog(fmt.Sprintf("%v", v))).Debug("Processing security_header_profile_id update")
if v == nil {
logger.WithField("host_uuid", safeUUID).Debug("Setting security_header_profile_id to nil")
host.SecurityHeaderProfileID = nil
} else {
conversionSuccess := false
switch t := v.(type) {
case float64:
logger.Debug("Received security_header_profile_id as float64")
if id, ok := safeFloat64ToUint(t); ok {
host.SecurityHeaderProfileID = &id
conversionSuccess = true
logger.Info("Successfully converted security_header_profile_id from float64")
} else {
logger.Warn("Failed to convert security_header_profile_id from float64: value is negative or not a valid uint")
}
case int:
logger.Debug("Received security_header_profile_id as int")
if id, ok := safeIntToUint(t); ok {
host.SecurityHeaderProfileID = &id
conversionSuccess = true
logger.Info("Successfully converted security_header_profile_id from int")
} else {
logger.Warn("Failed to convert security_header_profile_id from int: value is negative")
}
case string:
logger.Debug("Received security_header_profile_id as string")
if n, err := strconv.ParseUint(t, 10, 32); err == nil {
id := uint(n)
host.SecurityHeaderProfileID = &id
conversionSuccess = true
logger.WithField("host_uuid", safeUUID).WithField("profile_id", id).Info("Successfully converted security_header_profile_id from string")
} else {
logger.Warn("Failed to parse security_header_profile_id from string")
}
default:
logger.Warn("Unsupported type for security_header_profile_id")
}
if !conversionSuccess {
c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("invalid security_header_profile_id: unable to convert value %v of type %T to uint", v, v)})
return
}
parsedID, _, parseErr := parseNullableUintField(v, "security_header_profile_id")
if parseErr != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": parseErr.Error()})
return
}
host.SecurityHeaderProfileID = parsedID
}
// Locations: replace only if provided

139
backend/internal/api/handlers/proxy_host_handler_update_test.go
View File

@@ -75,6 +75,145 @@ func createTestSecurityHeaderProfile(t *testing.T, db *gorm.DB, name string) mod
return profile
}
// createTestAccessList creates an access list for testing.
func createTestAccessList(t *testing.T, db *gorm.DB, name string) models.AccessList {
t.Helper()
acl := models.AccessList{
UUID: uuid.NewString(),
Name: name,
Type: "ip",
Enabled: true,
}
require.NoError(t, db.Create(&acl).Error)
return acl
}
func TestProxyHostUpdate_AccessListID_Transitions_NoUnrelatedMutation(t *testing.T) {
t.Parallel()
router, db := setupUpdateTestRouter(t)
aclOne := createTestAccessList(t, db, "ACL One")
aclTwo := createTestAccessList(t, db, "ACL Two")
host := models.ProxyHost{
UUID: uuid.NewString(),
Name: "Access List Transition Host",
DomainNames: "acl-transition.test.com",
ForwardScheme: "http",
ForwardHost: "localhost",
ForwardPort: 8080,
Enabled: true,
SSLForced: true,
Application: "none",
AccessListID: &aclOne.ID,
}
require.NoError(t, db.Create(&host).Error)
assertUnrelatedFields := func(t *testing.T, current models.ProxyHost) {
t.Helper()
assert.Equal(t, "Access List Transition Host", current.Name)
assert.Equal(t, "acl-transition.test.com", current.DomainNames)
assert.Equal(t, "localhost", current.ForwardHost)
assert.Equal(t, 8080, current.ForwardPort)
assert.True(t, current.SSLForced)
assert.Equal(t, "none", current.Application)
}
runUpdate := func(t *testing.T, update map[string]any) {
t.Helper()
body, _ := json.Marshal(update)
req := httptest.NewRequest(http.MethodPut, "/api/v1/proxy-hosts/"+host.UUID, bytes.NewReader(body))
req.Header.Set("Content-Type", "application/json")
resp := httptest.NewRecorder()
router.ServeHTTP(resp, req)
require.Equal(t, http.StatusOK, resp.Code)
}
// value -> value
runUpdate(t, map[string]any{"access_list_id": aclTwo.ID})
var updated models.ProxyHost
require.NoError(t, db.First(&updated, "uuid = ?", host.UUID).Error)
require.NotNil(t, updated.AccessListID)
assert.Equal(t, aclTwo.ID, *updated.AccessListID)
assertUnrelatedFields(t, updated)
// value -> null
runUpdate(t, map[string]any{"access_list_id": nil})
require.NoError(t, db.First(&updated, "uuid = ?", host.UUID).Error)
assert.Nil(t, updated.AccessListID)
assertUnrelatedFields(t, updated)
// null -> value
runUpdate(t, map[string]any{"access_list_id": aclOne.ID})
require.NoError(t, db.First(&updated, "uuid = ?", host.UUID).Error)
require.NotNil(t, updated.AccessListID)
assert.Equal(t, aclOne.ID, *updated.AccessListID)
assertUnrelatedFields(t, updated)
}
func TestProxyHostUpdate_SecurityHeaderProfileID_Transitions_NoUnrelatedMutation(t *testing.T) {
t.Parallel()
router, db := setupUpdateTestRouter(t)
profileOne := createTestSecurityHeaderProfile(t, db, "Security Profile One")
profileTwo := createTestSecurityHeaderProfile(t, db, "Security Profile Two")
host := models.ProxyHost{
UUID: uuid.NewString(),
Name: "Security Profile Transition Host",
DomainNames: "security-transition.test.com",
ForwardScheme: "http",
ForwardHost: "localhost",
ForwardPort: 9090,
Enabled: true,
SSLForced: true,
Application: "none",
SecurityHeaderProfileID: &profileOne.ID,
}
require.NoError(t, db.Create(&host).Error)
assertUnrelatedFields := func(t *testing.T, current models.ProxyHost) {
t.Helper()
assert.Equal(t, "Security Profile Transition Host", current.Name)
assert.Equal(t, "security-transition.test.com", current.DomainNames)
assert.Equal(t, "localhost", current.ForwardHost)
assert.Equal(t, 9090, current.ForwardPort)
assert.True(t, current.SSLForced)
assert.Equal(t, "none", current.Application)
}
runUpdate := func(t *testing.T, update map[string]any) {
t.Helper()
body, _ := json.Marshal(update)
req := httptest.NewRequest(http.MethodPut, "/api/v1/proxy-hosts/"+host.UUID, bytes.NewReader(body))
req.Header.Set("Content-Type", "application/json")
resp := httptest.NewRecorder()
router.ServeHTTP(resp, req)
require.Equal(t, http.StatusOK, resp.Code)
}
// value -> value
runUpdate(t, map[string]any{"security_header_profile_id": fmt.Sprintf("%d", profileTwo.ID)})
var updated models.ProxyHost
require.NoError(t, db.First(&updated, "uuid = ?", host.UUID).Error)
require.NotNil(t, updated.SecurityHeaderProfileID)
assert.Equal(t, profileTwo.ID, *updated.SecurityHeaderProfileID)
assertUnrelatedFields(t, updated)
// value -> null
runUpdate(t, map[string]any{"security_header_profile_id": ""})
require.NoError(t, db.First(&updated, "uuid = ?", host.UUID).Error)
assert.Nil(t, updated.SecurityHeaderProfileID)
assertUnrelatedFields(t, updated)
// null -> value
runUpdate(t, map[string]any{"security_header_profile_id": fmt.Sprintf("%d", profileOne.ID)})
require.NoError(t, db.First(&updated, "uuid = ?", host.UUID).Error)
require.NotNil(t, updated.SecurityHeaderProfileID)
assert.Equal(t, profileOne.ID, *updated.SecurityHeaderProfileID)
assertUnrelatedFields(t, updated)
}
// TestProxyHostUpdate_EnableStandardHeaders_Null tests updating enable_standard_headers to null.
func TestProxyHostUpdate_EnableStandardHeaders_Null(t *testing.T) {
t.Parallel()

77
docs/issues/manual_test_acl_security_headers_dropdown_hotfix.md Normal file
View File

@@ -0,0 +1,77 @@
## Manual Test Plan — ACL + Security Headers Dropdown Hotfix
- Date: 2026-02-27
- Scope: Proxy Host create/edit dropdown persistence
- Goal: Confirm ACL and Security Headers selections save correctly, can be changed, and can be cleared without regressions.
## Preconditions
- [ ] Charon is running and reachable in browser
- [ ] At least 2 Access Lists exist
- [ ] At least 2 Security Headers profiles exist
- [ ] Tester has permission to create and edit Proxy Hosts
## Test Cases
### TC-001 — Create Host With Both Dropdowns Set
- Steps:
1. Open Proxy Hosts and start creating a new host.
2. Fill required host fields.
3. Select any Access List.
4. Select any Security Headers profile.
5. Save.
6. Reopen the same host in edit mode.
- Expected:
- The selected Access List remains selected.
- The selected Security Headers profile remains selected.
### TC-002 — Edit Host And Change Both Selections
- Steps:
1. Open an existing host that already has both values set.
2. Change Access List to a different option.
3. Change Security Headers to a different option.
4. Save.
5. Reopen the host.
- Expected:
- New Access List is persisted.
- New Security Headers profile is persisted.
- Previous values are not shown.
### TC-003 — Clear Access List
- Steps:
1. Open an existing host with an Access List selected.
2. Set Access List to no selection.
3. Save.
4. Reopen the host.
- Expected:
- Access List is empty (none).
- No old Access List value returns.
### TC-004 — Clear Security Headers
- Steps:
1. Open an existing host with a Security Headers profile selected.
2. Set Security Headers to no selection.
3. Save.
4. Reopen the host.
- Expected:
- Security Headers is empty (none).
- No old profile value returns.
### TC-005 — Regression Guard: Repeated Edit Cycles
- Steps:
1. Repeat edit/save cycle 3 times on one host.
2. Alternate between selecting values and clearing values for both dropdowns.
3. After each save, reopen the host.
- Expected:
- Last saved choice is always what appears after reopen.
- No mismatch between what was selected and what is shown.
## Execution Notes
- Targeted tests for this hotfix are already passing.
- Full-suite, security, and coverage gates are deferred to CI/end pass.

361
docs/plans/current_spec.md
View File

@@ -1,155 +1,270 @@
# ACL + Security Headers Hotfix Plan (Proxy Host Create/Edit)
## 1. Introduction
### Overview
Compatibility rollout for Caddy `2.11.1` is already reflected in the build
default (`Dockerfile` currently sets `ARG CADDY_VERSION=2.11.1`).
Hotfix request: Proxy Host form dropdown selections for Access Control List (ACL) and Security Headers are not being applied/persisted for new or edited hosts.
This plan is now focused on rollout verification and regression-proofing, not
changing the default ARG.
Reported behavior:
1. Existing hosts with previously assigned ACL/Security Header profile retain old values.
2. Users cannot reliably remove or change those values in UI.
3. Newly created hosts cannot reliably apply ACL/Security Header profile.
### Objective
Establish deterministic, evidence-backed gates that prove published images and
security artifacts are fresh, digest-bound, and aligned across registries for
the Caddy `2.11.1` rollout.
Deliver an urgent but correct root-cause fix across frontend binding and backend persistence flow, with minimum user interruption and full validation gates.
## 2. Current State (Verified)
## 2. Research Findings (Current Architecture + Touchpoints)
1. `Dockerfile` default is already `CADDY_VERSION=2.11.1`.
2. `ARCHITECTURE.md` now reports Caddy `2.11.1`.
3. Existing scan artifacts can become stale if not explicitly tied to pushed
digests.
### Frontend Entry Points
1. `frontend/src/pages/ProxyHosts.tsx`
- `handleSubmit(data)` calls `updateHost(editingHost.uuid, data)` or `createHost(data)`.
- Renders `ProxyHostForm` modal for create/edit flows.
2. `frontend/src/components/ProxyHostForm.tsx`
- Local form state initializes `access_list_id` and `security_header_profile_id`.
- ACL control uses `AccessListSelector`.
- Security Headers control uses `Select` with `security_header_profile_id` mapping.
- Submission path: `handleSubmit` -> `onSubmit(payloadWithoutUptime)`.
3. `frontend/src/components/AccessListSelector.tsx`
- Converts select values between `string` and `number | null`.
## 3. Technical Specification (EARS)
### Frontend API/Hooks
1. `frontend/src/hooks/useProxyHosts.ts`
- `createHost` -> `createProxyHost`.
- `updateHost` -> `updateProxyHost`.
2. `frontend/src/api/proxyHosts.ts`
- `createProxyHost(host: Partial<ProxyHost>)` -> `POST /api/v1/proxy-hosts`.
- `updateProxyHost(uuid, host)` -> `PUT /api/v1/proxy-hosts/:uuid`.
- Contract fields: `access_list_id`, `security_header_profile_id`.
1. WHEN image builds run without an explicit `CADDY_VERSION` override, THE
SYSTEM SHALL continue producing Caddy `2.11.1`.
2. WHEN an image tag is pushed, THE SYSTEM SHALL validate index digest parity
between GHCR and Docker Hub for that same tag.
3. WHEN multi-arch images are published, THE SYSTEM SHALL validate per-arch
digest parity across GHCR and Docker Hub for each platform present.
4. WHEN vulnerability and SBOM scans execute, THE SYSTEM SHALL scan
`image@sha256:<index-digest>` instead of mutable tags.
5. WHEN scan artifacts are generated, THE SYSTEM SHALL prove artifacts were
produced after the push event in the same validation run.
6. IF a verification gate fails, THEN THE SYSTEM SHALL block rollout sign-off
until all gates pass.
### Backend Entry/Transformation/Persistence
1. Route registration
- `backend/internal/api/routes/routes.go`: `proxyHostHandler.RegisterRoutes(protected)`.
2. Handler
- `backend/internal/api/handlers/proxy_host_handler.go`
- `Create(c)` uses `ShouldBindJSON(&models.ProxyHost{})`.
- `Update(c)` uses `map[string]any` partial update parsing.
- Target fields:
- `payload["access_list_id"]` -> `parseNullableUintField` -> `host.AccessListID`
- `payload["security_header_profile_id"]` -> typed conversion -> `host.SecurityHeaderProfileID`
3. Service
- `backend/internal/services/proxyhost_service.go`
- `Create(host)` validates + `db.Create(host)`.
- `Update(host)` validates + `db.Model(...).Select("*").Updates(host)`.
4. Model
- `backend/internal/models/proxy_host.go`
- `AccessListID *uint \`json:"access_list_id"\``
- `SecurityHeaderProfileID *uint \`json:"security_header_profile_id"\``
## 4. Scope and Planned Edits
### Existing Tests Relevant to Incident
1. Frontend unit regression coverage already exists:
- `frontend/src/components/__tests__/ProxyHostForm-dropdown-changes.test.tsx`
2. E2E regression spec exists:
- `tests/proxy-host-dropdown-fix.spec.ts`
3. Backend update and security-header tests exist:
- `backend/internal/api/handlers/proxy_host_handler_update_test.go`
- `backend/internal/api/handlers/proxy_host_handler_security_headers_test.go`
### In scope
1. `docs/plans/current_spec.md` (this plan refresh).
2. `ARCHITECTURE.md` version sync is already complete (`2.11.1`); no pending
update is required in this plan.
3. Verification workflow/checklist updates needed to enforce deterministic gates.
## 3. Root-Cause-First Trace
### Out of scope
1. No functional Caddy build logic changes unless a verification failure proves
they are required.
2. No plugin list or patch-scenario refactors.
### Trace Model (Mandatory)
1. Entry Point:
- UI dropdown interactions in `ProxyHostForm` and `AccessListSelector`.
2. Transformation:
- Form state conversion (`string` <-> `number | null`) and payload construction in `ProxyHostForm`.
- API serialization via `frontend/src/api/proxyHosts.ts`.
3. Persistence:
- Backend `Update` parser (`proxy_host_handler.go`) and `ProxyHostService.Update` persistence.
4. Exit Point:
- Response body consumed by React Query invalidation/refetch in `useProxyHosts`.
- UI reflects updated values in table/form.
## 5. Deterministic Acceptance Gates
### Most Likely Failure Zones
1. Frontend select binding/conversion drift (top candidate)
- Shared symptom across ACL and Security Headers points to form/select layer.
- Candidate files:
- `frontend/src/components/ProxyHostForm.tsx`
- `frontend/src/components/AccessListSelector.tsx`
- `frontend/src/components/ui/Select.tsx`
2. Payload mutation or stale form object behavior
- Ensure payload carries updated `access_list_id` / `security_header_profile_id` values at submit time.
3. Backend partial-update parser edge behavior
- Ensure `nil`, numeric string, and number conversions are consistent between ACL and security header profile paths.
### Gate 1: Digest Freshness (pre/post push)
1. Capture pre-push index digest for target tag on GHCR and Docker Hub.
2. Push image.
3. Capture post-push index digest on GHCR and Docker Hub.
4. Pass criteria:
- Post-push index digest changed as expected from pre-push (or matches
intended new digest when creating new tag).
- GHCR and Docker Hub index digests are identical for the tag.
- Per-arch digests are identical across registries for each published
platform.
### Investigation Decision
Root-cause verification will be instrumented through failing-first Playwright scenario and targeted handler tests before applying code changes.
### Gate 2: Digest-Bound Rescan
1. Resolve the post-push index digest.
2. Run all security scans against immutable ref:
- `ghcr.io/<owner>/<repo>@sha256:<index-digest>`
- Optional mirror check against Docker Hub digest ref.
3. Pass criteria:
- No scan uses mutable tags as the primary target.
- Artifact metadata and logs show digest reference.
## 4. EARS Requirements
### Gate 3: Artifact Freshness
1. Record push timestamp and digest capture timestamp.
2. Generate SBOM and vuln artifacts after push in the same run.
3. Pass criteria:
- Artifact generation timestamps are greater than push timestamp.
- Artifacts are newly created/overwritten in this run.
- Evidence ties each artifact to the scanned digest.
1. WHEN a user selects an ACL in the Proxy Host create/edit form, THE SYSTEM SHALL persist `access_list_id` and return it in API response.
2. WHEN a user changes ACL from one value to another, THE SYSTEM SHALL replace prior `access_list_id` with the new value.
3. WHEN a user selects "No Access Control", THE SYSTEM SHALL persist `access_list_id = null`.
4. WHEN a user selects a Security Headers profile in the Proxy Host create/edit form, THE SYSTEM SHALL persist `security_header_profile_id` and return it in API response.
5. WHEN a user changes Security Headers profile from one value to another, THE SYSTEM SHALL replace prior `security_header_profile_id` with the new value.
6. WHEN a user selects "None" for Security Headers, THE SYSTEM SHALL persist `security_header_profile_id = null`.
7. IF dropdown interaction fails to update internal form state, THEN THE SYSTEM SHALL prevent stale values from being persisted.
8. WHILE updating Proxy Host settings, THE SYSTEM SHALL maintain existing behavior for unrelated fields and not regress certificate, DNS challenge, or uptime-linked updates.
### Gate 4: Evidence Block (mandatory)
Every validation run must include a structured evidence block with:
1. Tag name.
2. Index digest.
3. Per-arch digests.
4. Scan tool versions.
5. Push and scan timestamps.
6. Artifact file names produced in this run.
Note: User-visible blocking error behavior is deferred unless required by confirmed root cause.
## 6. Implementation Plan
## 5. Technical Specification (Hotfix Scope)
### Phase 1: Baseline Capture
1. Confirm current `Dockerfile` default remains `2.11.1`.
2. Capture pre-push digest state for target tag across both registries.
### API Contract (No Breaking Change)
1. `POST /api/v1/proxy-hosts`
- Request fields include `access_list_id`, `security_header_profile_id` as nullable numeric fields.
2. `PUT /api/v1/proxy-hosts/:uuid`
- Partial payload accepts nullable updates for both fields.
3. Response must echo persisted values in snake_case:
- `access_list_id`
- `security_header_profile_id`
### Phase 2: Docs Sync
1. Confirm `ARCHITECTURE.md` remains synced at Caddy `2.11.1`.
### Data Model/DB
No schema migration expected. Existing nullable FK fields in `backend/internal/models/proxy_host.go` are sufficient.
### Phase 3: Push and Verification
1. Push validation tag.
2. Execute Gate 1 (digest freshness and parity).
3. Execute Gate 2 (digest-bound rescan).
4. Execute Gate 3 (artifact freshness).
5. Produce Gate 4 evidence block.
### Targeted Code Areas for Fix
1. Frontend
- `frontend/src/components/ProxyHostForm.tsx`
- `frontend/src/components/AccessListSelector.tsx`
- `frontend/src/components/ui/Select.tsx` (only if click/select propagation issue confirmed)
- `frontend/src/api/proxyHosts.ts` (only if serialization issue confirmed)
2. Backend
- `backend/internal/api/handlers/proxy_host_handler.go` (only if parsing/persistence mismatch confirmed)
- `backend/internal/services/proxyhost_service.go` (only if update write path proves incorrect)
### Phase 4: Sign-off
1. Mark rollout verified only when all gates pass.
2. If any gate fails, open follow-up remediation task before merge.
## 6. Edge Cases
## 7. Acceptance Criteria
1. Edit host with existing ACL/profile and switch to another value.
2. Edit host with existing ACL/profile and clear to null.
3. Create new host with ACL/profile set before first save.
4. Submit with stringified numeric values (defensive compatibility).
5. Submit with null values for both fields simultaneously.
6. Missing/deleted profile or ACL IDs in backend (validation errors).
7. Multiple rapid dropdown changes before save (last selection wins).
1. Plan and execution no longer assume Dockerfile default is beta.
2. Objective is rollout verification/regression-proofing for Caddy `2.11.1`.
3. `ARCHITECTURE.md` version metadata is included in required docs sync.
4. Digest freshness gate passes:
- Pre/post push validation completed.
- GHCR and Docker Hub index digest parity confirmed.
- Per-arch digest parity confirmed.
5. Digest-bound rescan gate passes with `image@sha256` scan targets.
6. Artifact freshness gate passes with artifacts produced after push in the same
run.
7. Evidence block is present and complete with:
- Tag
- Index digest
- Per-arch digests
- Scan tool versions
- Timestamps
- Artifact names
## 7. Risk Analysis
## 8. PR Slicing Strategy
### High Risk
1. Silent stale-state submission from form controls.
2. Regressing other Proxy Host settings due to broad payload mutation.
### Medium Risk
1. Partial-update parser divergence between ACL and security profile behavior.
2. UI select portal/z-index interaction causing non-deterministic click handling.
### Mitigations
1. Reproduce with Playwright first and capture exact failing action path.
2. Add/strengthen focused frontend tests around create/edit/clear flows.
3. Add/strengthen backend tests for nullable + conversion paths.
4. Keep hotfix minimal and avoid unrelated refactors.
## 8. Implementation Plan (Urgent, Minimal Interruption)
### Phase 1: Reproduction + Guardrails (Playwright First)
1. Execute targeted E2E spec for dropdown flow and create/edit persistence behavior.
2. Capture exact failure step and confirm whether failure is click binding, payload value, or backend persistence.
3. Add/adjust failing-first test if current suite does not capture observed production regression.
### Phase 2: Frontend Fix
1. Patch select binding/state mapping for ACL and Security Headers in `ProxyHostForm`/`AccessListSelector`.
2. If needed, patch `ui/Select` interaction layering.
3. Ensure payload contains correct final `access_list_id` and `security_header_profile_id` values at submit.
4. Extend `ProxyHostForm` tests for create/edit/change/remove flows.
### Phase 3: Backend Hardening (Conditional)
1. Only if frontend payload is correct but persistence is wrong:
- Backend fix MUST use field-scoped partial-update semantics for `access_list_id` and `security_header_profile_id` only (unless separately justified).
- Ensure write path persists null transitions reliably.
2. Add/adjust handler/service regression tests proving no unintended mutation of unrelated proxy host fields during these targeted updates.
### Phase 4: Integration + Regression
1. Run complete targeted Proxy Host UI flow tests.
2. Validate list refresh and modal reopen reflect persisted values.
3. Validate no regressions in bulk ACL / bulk security-header operations.
### Phase 5: Documentation + Handoff
1. Update changelog/release notes only for hotfix behavior.
2. Keep architecture docs unchanged unless root cause requires architectural note.
3. Handoff to Supervisor agent for review after plan approval and implementation.
## 9. Acceptance Criteria
1. ACL dropdown selection persists on create and edit.
2. Security Headers dropdown selection persists on create and edit.
3. Clearing ACL persists `null` and is reflected after reload.
4. Clearing Security Headers persists `null` and is reflected after reload.
5. Existing hosts can change from one ACL/profile to another without stale value retention.
6. New hosts can apply ACL/profile at creation time.
7. No regressions in unrelated proxy host fields.
8. All validation gates in Section 11 pass.
9. API create response returns persisted `access_list_id` and `security_header_profile_id` matching submitted values (including `null`).
10. API update response returns persisted `access_list_id` and `security_header_profile_id` after `value->value`, `value->null`, and `null->value` transitions.
11. Backend persistence verification confirms unrelated proxy host fields remain unchanged for targeted updates.
## 10. PR Slicing Strategy
### Decision
Single PR.
Single PR (hotfix-first), with contingency split only if backend root cause is confirmed late.
### Trigger Reasons
1. Scope is narrow and cross-cutting risk is low.
2. Verification logic and docs sync are tightly coupled.
3. Review size remains small and rollback is straightforward.
### Rationale
1. Incident impact is immediate user-facing and concentrated in one feature path.
2. Frontend + targeted backend/test changes are tightly coupled for verification.
3. Single PR minimizes release coordination and user interruption.
### PR-1
1. Scope:
- Refresh `docs/plans/current_spec.md` to verification-focused plan.
- Sync `ARCHITECTURE.md` Caddy version metadata.
- Add/adjust verification checklist content needed for gates.
2. Dependencies:
- Existing publish/scanning pipeline availability.
3. Validation gates:
- Gate 1 through Gate 4 all required.
### Contingency (Only if split becomes necessary)
1. PR-1: Frontend binding + tests
- Scope: `ProxyHostForm`, `AccessListSelector`, `ui/Select` (if required), related tests.
- Dependency: none.
- Acceptance: UI submit payload verified correct in unit + Playwright.
2. PR-2: Backend parser/persistence + tests (conditional)
- Scope: `proxy_host_handler.go`, `proxyhost_service.go`, handler/service tests.
- Dependency: PR-1 merged or rebased for aligned contract.
- Acceptance: API update/create persist both nullable IDs correctly.
3. PR-3: Regression hardening + docs
- Scope: extra regression coverage, release-note hotfix entry.
- Dependency: PR-1/PR-2.
- Acceptance: full DoD validation sequence passes.
## 9. Rollback and Contingency
## 11. Validation Plan (Mandatory Sequence)
1. If verification updates are incorrect or incomplete, revert PR-1.
2. If rollout evidence fails, hold release sign-off and keep last known-good
digest as active reference.
3. Re-run verification with corrected commands/artifacts before reattempting
sign-off.
0. E2E environment prerequisite
- Determine rebuild necessity per testing policy: if application/runtime or Docker input changes are present, rebuild is required.
- If rebuild is required or the container is unhealthy, run `.github/skills/scripts/skill-runner.sh docker-rebuild-e2e`.
- Record container health outcome before executing tests.
1. Playwright first
- Run targeted Proxy Host dropdown and create/edit persistence scenarios.
2. Local patch coverage preflight
- Generate `test-results/local-patch-report.md` and `test-results/local-patch-report.json`.
3. Unit and coverage
- Backend coverage run (threshold >= 85%).
- Frontend coverage run (threshold >= 85%).
4. Type checks
- Frontend TypeScript check.
5. Pre-commit
- `pre-commit run --all-files` with zero blocking failures.
6. Security scans
- CodeQL Go + JS (security-and-quality).
- Findings check gate.
- Trivy scan.
- Conditional GORM security scan if model/DB-layer changes are made.
7. Build verification
- Backend build + frontend build pass.
## 12. File Review: `.gitignore`, `codecov.yml`, `.dockerignore`, `Dockerfile`
Assessment for this hotfix:
1. `.gitignore`: no required change for ACL/Security Headers hotfix.
2. `codecov.yml`: no required change; current exclusions/thresholds are compatible.
3. `.dockerignore`: no required change unless new hotfix-only artifact paths are introduced.
4. `Dockerfile`: no required change; incident is application logic/UI binding, not image build pipeline.
If implementation introduces new persistent test artifacts, update ignore files in the same PR.
## 13. Rollback and Contingency
1. If hotfix causes regression in proxy host save flow, revert hotfix commit and redeploy prior stable build.
2. If frontend-only fix is insufficient, activate conditional backend phase immediately.
3. If validation gates fail on security/coverage, hold merge until fixed; no partial exception for this incident.
4. Post-rollback smoke checks:
- Create host with ACL/profile.
- Edit to different ACL/profile values.
- Clear both values to `null`.
- Verify persisted values in API response and after UI reload.

58
frontend/src/components/ProxyHostForm.tsx
View File

@@ -101,9 +101,12 @@ interface ProxyHostFormProps {
onCancel: () => void
}
export default function ProxyHostForm({ host, onSubmit, onCancel }: ProxyHostFormProps) {
type ProxyHostFormState = Partial<ProxyHost> & { addUptime?: boolean; uptimeInterval?: number; uptimeMaxRetries?: number }
const [formData, setFormData] = useState<ProxyHostFormState>({
function buildInitialFormData(host?: ProxyHost): Partial<ProxyHost> & {
addUptime?: boolean
uptimeInterval?: number
uptimeMaxRetries?: number
} {
return {
name: host?.name || '',
domain_names: host?.domain_names || '',
forward_scheme: host?.forward_scheme || 'http',
@@ -123,7 +126,42 @@ export default function ProxyHostForm({ host, onSubmit, onCancel }: ProxyHostFor
access_list_id: host?.access_list_id,
security_header_profile_id: host?.security_header_profile_id,
dns_provider_id: host?.dns_provider_id || null,
})
}
}
function normalizeNullableID(value: unknown): number | null | undefined {
if (value === undefined) {
return undefined
}
if (value === null) {
return null
}
if (typeof value === 'number') {
return Number.isFinite(value) ? value : null
}
if (typeof value === 'string') {
const trimmed = value.trim()
if (trimmed === '') {
return null
}
const parsed = Number.parseInt(trimmed, 10)
return Number.isNaN(parsed) ? null : parsed
}
return null
}
export default function ProxyHostForm({ host, onSubmit, onCancel }: ProxyHostFormProps) {
type ProxyHostFormState = Partial<ProxyHost> & { addUptime?: boolean; uptimeInterval?: number; uptimeMaxRetries?: number }
const [formData, setFormData] = useState<ProxyHostFormState>(buildInitialFormData(host))
useEffect(() => {
setFormData(buildInitialFormData(host))
}, [host?.uuid])
// Charon internal IP for config helpers (previously CPMP internal IP)
const [charonInternalIP, setCharonInternalIP] = useState<string>('')
@@ -420,6 +458,10 @@ export default function ProxyHostForm({ host, onSubmit, onCancel }: ProxyHostFor
// strip temporary uptime-only flags from payload by destructuring
const { addUptime: _addUptime, uptimeInterval: _uptimeInterval, uptimeMaxRetries: _uptimeMaxRetries, ...payloadWithoutUptime } = payload as ProxyHostFormState
void _addUptime; void _uptimeInterval; void _uptimeMaxRetries;
payloadWithoutUptime.access_list_id = normalizeNullableID(payloadWithoutUptime.access_list_id)
payloadWithoutUptime.security_header_profile_id = normalizeNullableID(payloadWithoutUptime.security_header_profile_id)
const res = await onSubmit(payloadWithoutUptime)
// if user asked to add uptime, request server to sync monitors
@@ -824,7 +866,7 @@ export default function ProxyHostForm({ host, onSubmit, onCancel }: ProxyHostFor
{/* Access Control List */}
<AccessListSelector
value={formData.access_list_id || null}
value={formData.access_list_id ?? null}
onChange={id => setFormData(prev => ({ ...prev, access_list_id: id }))}
/>
@@ -836,9 +878,9 @@ export default function ProxyHostForm({ host, onSubmit, onCancel }: ProxyHostFor
</label>
<Select
value={String(formData.security_header_profile_id || 0)}
value={formData.security_header_profile_id == null ? 'none' : String(formData.security_header_profile_id)}
onValueChange={e => {
const value = e === "0" ? null : parseInt(e) || null
const value = e === 'none' ? null : normalizeNullableID(e)
setFormData(prev => ({ ...prev, security_header_profile_id: value }))
}}
>
@@ -846,7 +888,7 @@ export default function ProxyHostForm({ host, onSubmit, onCancel }: ProxyHostFor
<SelectValue />
</SelectTrigger>
<SelectContent>
<SelectItem value="0">None (No Security Headers)</SelectItem>
<SelectItem value="none">None (No Security Headers)</SelectItem>
{securityProfiles
?.filter(p => p.is_preset)
.sort((a, b) => a.security_score - b.security_score)

126
frontend/src/components/__tests__/ProxyHostForm-dropdown-changes.test.tsx
View File

@@ -410,4 +410,130 @@ describe('ProxyHostForm Dropdown Change Bug Fix', () => {
)
})
})
it('persists null to value transitions for ACL and security headers in edit flow', async () => {
const user = userEvent.setup()
const Wrapper = createWrapper()
const existingHostWithNulls: ProxyHost = {
uuid: 'host-uuid-null-fields',
name: 'Existing Null Fields',
domain_names: 'existing-null.com',
forward_scheme: 'http',
forward_host: 'localhost',
forward_port: 8080,
ssl_forced: true,
http2_support: true,
hsts_enabled: true,
hsts_subdomains: true,
block_exploits: true,
websocket_support: false,
enable_standard_headers: true,
application: 'none',
advanced_config: '',
enabled: true,
locations: [],
certificate_id: null,
access_list_id: null,
security_header_profile_id: null,
dns_provider_id: null,
created_at: '2024-01-01',
updated_at: '2024-01-01',
}
render(
<Wrapper>
<ProxyHostForm host={existingHostWithNulls} onSubmit={mockOnSubmit} onCancel={mockOnCancel} />
</Wrapper>
)
const aclTrigger = screen.getByRole('combobox', { name: /Access Control List/i })
await user.click(aclTrigger)
await user.click(await screen.findByRole('option', { name: /Office Network/i }))
const headersTrigger = screen.getByRole('combobox', { name: /Security Headers/i })
await user.click(headersTrigger)
await user.click(await screen.findByRole('option', { name: /Strict Security/i }))
await user.click(screen.getByRole('button', { name: /Save/i }))
await waitFor(() => {
expect(mockOnSubmit).toHaveBeenCalledWith(
expect.objectContaining({
access_list_id: 1,
security_header_profile_id: 2,
})
)
})
})
it('resets ACL/security header form state when editing target host changes', async () => {
const user = userEvent.setup()
const Wrapper = createWrapper()
const firstHost: ProxyHost = {
uuid: 'host-uuid-first',
name: 'First Host',
domain_names: 'first.example.com',
forward_scheme: 'http',
forward_host: 'localhost',
forward_port: 8080,
ssl_forced: true,
http2_support: true,
hsts_enabled: true,
hsts_subdomains: true,
block_exploits: true,
websocket_support: false,
enable_standard_headers: true,
application: 'none',
advanced_config: '',
enabled: true,
locations: [],
certificate_id: null,
access_list_id: 1,
security_header_profile_id: 1,
dns_provider_id: null,
created_at: '2024-01-01',
updated_at: '2024-01-01',
}
const secondHost: ProxyHost = {
...firstHost,
uuid: 'host-uuid-second',
name: 'Second Host',
domain_names: 'second.example.com',
access_list_id: null,
security_header_profile_id: null,
}
const { rerender } = render(
<Wrapper>
<ProxyHostForm host={firstHost} onSubmit={mockOnSubmit} onCancel={mockOnCancel} />
</Wrapper>
)
// Mutate first host state in the form before switching targets.
await user.click(screen.getByRole('combobox', { name: /Access Control List/i }))
await user.click(await screen.findByRole('option', { name: /VPN Users/i }))
await user.click(screen.getByRole('combobox', { name: /Security Headers/i }))
await user.click(await screen.findByRole('option', { name: /Strict Security/i }))
rerender(
<Wrapper>
<ProxyHostForm host={secondHost} onSubmit={mockOnSubmit} onCancel={mockOnCancel} />
</Wrapper>
)
await user.click(screen.getByRole('button', { name: /Save/i }))
await waitFor(() => {
expect(mockOnSubmit).toHaveBeenCalledWith(
expect.objectContaining({
access_list_id: null,
security_header_profile_id: null,
})
)
})
})
})