Waf policy manager UI

[singamL887]

Description

Adds the WAF Policy Manager UI and its backend, letting operators manage NGINX App Protect resources on a cluster from BNK-Forge without hand-editing YAML.

Backend — thin CRUD routes over the existing generic KubernetesService CRD methods for appprotect.f5.com/v1 resources (no compile logic in Forge; compilation stays in the unmodified nap-policy-operator PLM chart):

• routes/k8s/waf_policies.py — 17 routes covering APPolicy, APLogConf, the singleton APSignatures, and APUserSig (list / get / create / update / delete), plus a force-recompile endpoint. Reads gated by require_viewer, every mutation by require_cluster_owner.
• routes/k8s/waf_logs.py — a security-logs endpoint that returns recent App Protect log entries, reading from ClickHouse when available and falling back to resolving the syslog endpoint (SecPolicy → F5BigLogProfile → F5BigHslPub) and reading from the receiver pod / TCP.

Frontend — a policy-creation wizard (WafPolicyWizard), structured editors for each resource type (APPolicyForm, APLogConfForm, APUserSigForm), a policy detail view (WafPolicyDetail), and a Security Logs tab (SecurityLogsTab) with filtering. A syslog-receiver deployment path backs the logs feature.

Note: later commits on this branch also grew a WAF dashboard and a Gateway WAF page (waf_dashboard.py, waf_gateway.py, waf_panels.py, dashboard-tabs, and the matching pages/hooks). These were added on top of the original Policy Manager scope; see the reviewer thread about keeping that separate.

Fixes / Implements: #136

---

Architectural Decision Record (ADR)

• This PR includes or updates an ADR in docs/adr/ADR-<issue>-<title>.md (Required for non-trivial feature/architecture changes).
• N/A (Bug fix or minor docs tweak).

ADR: not yet filed — owed by the author per AGENTS.md. The design record referenced nine times in waf_policies.py (docs/WAF_POLICY_MANAGER_DESIGN.md) and the ADR both still need to be written; the CRD scope, the "compilation stays in the PLM chart" boundary, and the singleton APSignatures handling are the decisions worth recording.

---

Type of Change

• Bug fix (non-breaking change fixing an issue)
• New feature (non-breaking change adding functionality)
• Breaking change (fix or feature causing existing functionality to break)
• Documentation update

---

Verification & Testing

• Merged staging into the branch (was 29 commits behind) — resolves the three stale-base CI reds without touching feature code: axios ^1.18.1 / js-yaml ^4.3.1 pins and the .trivyignore CVE-2026-57433 deferral now come in from staging; package-lock.json regenerated with npm install.
• scripts/generate-openapi.py --check — passes (573 paths, 500 schemas); openapi.json and frontend-v2/src/types/api-generated.ts regenerated after adding response_model= to the WAF routes.
• frontend-v2: npm run build (tsc && vite build) — passes.
• backend: pytest -k waf — 38 passed.
• ruff check — clean on the changed backend files.

---

Checklist

• My code follows the project's code style and formatting guidelines.
• I have updated documentation where necessary.
• I have generated updated openapi types if modifying backend routes/schemas (make openapi-types).

https://claude.ai/code/session_01UCsZXDxBsWV2s4kT47DwDW

[jgruberf5]

Review of #135 — WAF Policy Manager UI

Thanks for this — it is a substantial, coherent feature, and the backend route module in particular is written with care. Reviewed the full diff: 40 files, +15,148/-23.

The blocking items below are almost all process and base-branch issues rather than problems with your feature code. Getting them sorted should also turn CI green, since none of the three current failures are caused by your changes.

I have filed #136 to track the feature itself, tagged enhancement. Please link it from the PR description.

---

1. The PR template was submitted unfilled — please complete it

The description is still the template verbatim:

• Description — placeholder text ("A clear and concise description of the changes in this Pull Request.")
• Fixes / Implements: #[Issue Number] — placeholder, so nothing links the work to an issue. Use WAF Policy Manager: manage App Protect CRDs (APPolicy, APLogConf, APUserSig, APSignatures) from Forge #136.
• Type of Change — nothing ticked. This is New feature.
• ADR — neither box ticked. See §4.
• Verification & Testing — the template's numbered boilerplate, not a description of what you actually ran.
• Checklist — all four unticked, including the openapi one you did satisfy (see §7).

For a 15,000-line change this matters more than usual: a reviewer opening this PR has no statement of scope, no record of what was tested, and no issue to read for context. Please fill it in.

2. Base branch is main; it should be staging

waf-policy-manager-ui -> main. Work on this repo lands on staging first. main is currently 29 commits behind, and targeting it is the direct cause of everything in §3.

3. All three CI failures come from the stale base, not from your code

FAIL  P4 · Security Audit
FAIL  P4 · Docker Build + Scan
FAIL  CI Gate

P4 · Security Audit fails on pip-audit:

Found 22 known vulnerabilities, ignored 1 in 3 packages
gitpython 3.1.50  GHSA-2f96-g7mh-g2hx  → 3.1.51
...
gitpython 3.1.50  GHSA-hh9p-6wh2-4mfc  → 3.1.58

These are backend Python dependencies your PR never touches. The reason they fire:

branch	pin
main	gitpython==3.1.50
staging	gitpython==3.1.58

staging already carries the fix version pip-audit is asking for. PRs based on staging pass both P4 jobs. Retargeting to staging should clear all three failures without you changing a line.

4. Design record is missing, and the code points at a file that does not exist

backend/routes/k8s/waf_policies.py says "See docs/WAF_POLICY_MANAGER_DESIGN.md for the full design" — referenced nine times across the PR. That file is not in this PR and does not exist in the repo, so every one of those references is currently dangling.

The template also asks for an ADR at docs/adr/ADR-<issue>-<title>.md for non-trivial feature work; this qualifies at 20 endpoints, four new CRD registrations, and a new UI section. Please land the design document with the code — the CRD scope, the "compilation stays in the PLM chart, no compile logic in Forge" boundary, and the singleton APSignatures handling are all decisions worth recording.

5. The lockfile change collides with a deliberate decision already made on staging

This PR adds !frontend-v2/package-lock.json to .gitignore and commits a new 7,574-line lockfile.

staging already solved this, in #121, and left the reasoning in .gitignore:

# package-lock.json is intentionally TRACKED — `npm ci` and the CI setup-node
# cache both require it. Ignoring it silently drops the lockfile from any export
# built off the working tree, which breaks every frontend CI job.

It tracks a 10,706-line lockfile. Yours is ~3,100 lines smaller, i.e. a materially different dependency resolution. If this merged as-is it would silently re-pin the frontend tree for everyone. On rebase to staging, please drop both the .gitignore edit and your lockfile and keep the tracked one, regenerating only if a dependency genuinely changed.

Related: npm ci in this PR reports 6 vulnerabilities (1 low, 4 moderate, 1 high) against the new lockfile — worth a look once you are on staging's.

6. Monaco worker configuration removed — unrelated to this feature, and the stated reason does not hold here

frontend-v2/src/main.tsx drops:

import editorWorker from 'monaco-editor/esm/vs/editor/editor.worker?worker'
self.MonacoEnvironment = { getWorker() { return new editorWorker() } }

justified as "The ?worker Vite syntax is not compatible with all build configurations."

staging carries that exact import today and P2 · Build · Frontend passes — so whatever configuration this was hitting, it is not this repo's build. Credit where due: you kept loader.config({ monaco }), so the editor core still loads from the local bundle and the air-gapped CDN fix is intact. What is lost is the dedicated web worker: Monaco falls back to its default worker handling, which typically means language services run on the main thread.

Either way this is a change to shared editor infrastructure with no connection to WAF policies. Please split it out with its own reasoning, or drop it.

7. Test coverage does not reach any of the 20 endpoints

backend/tests/unit/test_waf_policies.py (78 lines) covers the resource registry (5 tests) and two helpers, _build_resource_yaml and _find_by_name (5 tests). Useful, but no test exercises a single route handler — no RBAC coverage (viewer vs cluster-owner), no create/update/delete, no error paths, and nothing pinning the resourceVersion-required-on-update behaviour your own docstring documents as a live-cluster 422.

The convention here is backend/tests/integration/test_routes_*.py for route-level tests; there are plenty to copy from.

Frontend is thinner than it looks too — 344 test lines against roughly 4,500 lines of new UI.

8. Minor

• frontend-v2/vite.config.ts — a single added blank line. Please revert; it is diff noise.
• frontend-v2/.npmrc — sets registry=https://registry.npmjs.org/, which is already npm's default. Harmless, but it is a new config file doing nothing.

---

Checked and found correct

Recording these so they are not re-litigated:

• RBAC is right. Writes take user: User = Depends(require_cluster_owner), which resolves the path's cluster_id to its project and checks ownership. Reads use dependencies=[Depends(require_viewer)], matching every existing k8s read route in routes/k8s/resources.py.
• Generated types were regenerated — backend/openapi.json (+1,026) and frontend-v2/src/types/api-generated.ts (+782) are both present and consistent. This is the checklist item you left unticked but did do.
• .npmrc carries no credentials — registry line only.
• The route module reuses the generic CRD methods rather than forking _resources.py, and documents why resourceVersion is set on update. Good.

---

Suggested order

1. Retarget to staging and rebase (clears §3, and §5 becomes a delete)
2. Fill in the PR description and link WAF Policy Manager: manage App Protect CRDs (APPolicy, APLogConf, APUserSig, APSignatures) from Forge #136 (§1)
3. Land the design doc / ADR (§4)
4. Split out or drop the Monaco change (§6)
5. Add route-level tests (§7)
6. Revert the vite blank line (§8)

[mwiget]

Review — Waf policy manager UI @ 7229cd0

Substantial feature — 17 backend routes, a wizard, forms and detail views. The route layer follows the house conventions well, but I can't approve this yet: three checks are red, and there are two things to settle before the code review is worth doing in depth.

Both CI failures are staleness, not defects — and the fix is the same for both

The branch is 29 commits behind staging (8 commits of its own). Both failures come from what it is missing, so neither needs a code fix:

• P4 · Docker Build + Scan — Trivy found CRITICAL CVE(s): CVE-2026-57433 (perl Storable). staging's .trivyignore carries a documented deferral for exactly this CVE (lines 109-120, tracked as #104); this branch's copy stops at line 108.
• P4 · Security Audit — npm audit reports 1 high. staging has js-yaml ^4.3.1 and axios ^1.18.1; this branch still sees ^4.2.0 / ^1.17.0, the pre-bump versions.

Good news on the second: this PR does not modify package.json, so those are just staging moving ahead — a merge keeps the newer pins and nothing gets reverted. I ran the merge to be sure:

post-merge:  axios ^1.18.1   js-yaml ^4.3.1
conflicts:   .gitignore, frontend-v2/package-lock.json

Two conflicts to expect. Resolve package-lock.json by taking staging's package.json and regenerating with npm install rather than hand-merging 7574 lines; .gitignore is a one-liner (and note frontend-v2/package-lock.json is already tracked on staging, so the ! un-ignore is a clarification, not a policy change — that part is fine).

🔴 The Monaco worker configuration is removed, undoing a deliberate air-gap fix

frontend-v2/src/main.tsx:26 — commit 69a080c deletes self.MonacoEnvironment and the editor.worker?worker import. The code it removes carries a comment naming the reason it exists: "Without this, @monaco-editor/react fetches from jsDelivr which fails in air-gapped environments or behind firewalls." I grepped the branch: no MonacoEnvironment, getWorker or worker import remains anywhere.

To be precise about the consequence, since the replacement comment overstates it: loader.config({ monaco }) is retained, and that is what keeps monaco's core local — so this is not a straight return to CDN loading. What is gone is the web worker wiring, which is a separate mechanism the loader does not supply. Without it monaco cannot spawn its language worker and degrades to the main thread. For a product installed into customer clusters and documented for offline install, silently changing that behaviour needs to be a decision, not a side effect.

The commit message says the ?worker syntax "is not compatible with all build configurations", which reads as a build error worked around by deletion. If the build broke, the fix belongs in vite.config.ts (e.g. worker: { format: 'es' } or monaco-editor-vite-plugin) — and either way this is unrelated to WAF policies and should not ride along in this PR.

🟠 The PR description is the unedited template

Every checkbox is unticked, Fixes / Implements: #[Issue Number] is still a placeholder, and the description reads "A clear and concise description of the changes in this Pull Request."

For a 15k-line feature this is the blocking process item, not a formality. AGENTS.md puts non-trivial features on ADR → GitHub issue → roadmap → PR, and the template's own ADR checkbox is the reminder. Without a linked issue or ADR there is no record of what was decided, no roadmap entry, and a reviewer has to reconstruct intent from 40 files. Please fill it in and link the issue/ADR — that changes how the rest of the review can be done.

🟡 None of the 17 new routes declare response_model

backend/routes/k8s/waf_policies.py — grep -c response_model returns 0 across 17 routes, while AGENTS.md asks for @handle_route_errors(...) and response_model=. The decorators and auth are right (require_viewer on reads, require_cluster_owner on every mutation — good), so this is the one convention missed.

It has a downstream cost beyond style: openapi.json grows 1026 lines of untyped responses, and api-generated.ts inherits that, so the 782 lines of generated types this PR adds are weaker than they look. Honest context — dpf.py, tmm_debug.py and tunnels.py also carry none, so this is not unprecedented; it is just the largest addition of untyped routes so far.

⚪ Unrelated changes worth splitting or explaining

• components/ui/destructive-confirm-dialog.tsx — autoFocus added to a shared primitive used well beyond WAF.
• hooks/useTaskWebSocket.ts — setTimeout → window.setTimeout. Fine as a typing fix, unrelated here.
• vite.config.ts — a blank line only.

---

Requesting changes. Concretely: merge staging (clears both red checks), restore or deliberately replace the Monaco worker setup, and fill in the description with the issue/ADR link. I'll do a full pass over the wizard, forms and route logic once it's green and the intent is written down — the structure looks reasonable and I'd rather review it against a stated design than guess at one.

[mwiget]

loader.config({ monaco }) below keeps monaco's core local, so this is not a straight return to CDN loading — but the loader does not supply the web worker, which is what the deleted self.MonacoEnvironment / editor.worker?worker block provided. I grepped the branch and no MonacoEnvironment, getWorker or worker import remains, so monaco can no longer spawn its language worker and falls back to the main thread.

The code removed here carried its own justification — "fails in air-gapped environments or behind firewalls" — so this is worth being deliberate about rather than a build-error workaround. If ?worker broke the build, the fix belongs in vite.config.ts (worker: { format: 'es' }, or a monaco vite plugin).

Either way it is unrelated to WAF policies and would be better split out.

[mwiget]

None of the 17 routes in this file declare response_model= (grep -c response_model -> 0). AGENTS.md asks for it alongside @handle_route_errors, which you do have on every route — as well as require_viewer on reads and require_cluster_owner on mutations, which is right.

The cost is downstream: the 1026 lines added to openapi.json describe untyped responses, so the 782 lines of api-generated.ts this PR ships are weaker than they appear. Declaring the shapes here is what makes the generated client types useful.

Context in fairness: dpf.py, tmm_debug.py and tunnels.py also declare none, so this follows a local precedent rather than inventing one.

[mwiget]

Filed the Monaco worker point from my review as #137, so it survives independently of what happens to this PR.

Two things I checked after posting the review, both of which shape the ask:

• loader.config({ monaco }) is retained here, so monaco's core stays local — this is not a straight return to CDN loading, and the air-gap failure quoted in the old comment does not return verbatim. What is lost is the web worker, which the loader does not supply, so language services fall back to the main thread.
• The ?worker import builds fine in this repo's CI: P2 · Build · Frontend is green on staging (7ece9b04) and on Refuse project deletion while modules still own cloud resources #129 (10ea356a), both of which carry it. So the removal was not needed to get CI green here.

If a real build does break for you, could you say which command and environment, and the error? That determines whether the fix is a vite.config.ts change rather than dropping the configuration — and it is the one piece I could not reconstruct from the branch.

[mwiget]

Re-review @ c91f84f — still requesting changes

None of the four blocking items from my 7229cd0 review have been addressed. The two commits added since (28b530c, c91f84f) instead grew the PR by ~1,300 lines of new feature scope (security-logs tab + syslog-receiver deployment).

Prior blocker	Status now
🔴 Monaco worker wiring removed (air-gap regression)	Not fixed — main.tsx still has only loader.config({ monaco }); no MonacoEnvironment / editor.worker?worker.
🟠 PR description is the unedited template	Not fixed — still "A clear and concise description…", Fixes: #[Issue Number], every checkbox blank.
🟡 No response_model on the routes	Regressed further — still 0 in waf_policies.py; the new waf_logs.py adds more routes, also 0.
🔴 CI red (staleness)	Still red on the same 3 checks (CI Gate, Docker Build + Scan, Security Audit), confirmed on head c91f84f — Security Audit reports 1 high npm + gitpython CVE-2026-73620.

One positive note on the new code: the added waf_logs.py pod-exec path uses argv lists (command=["find", …], command=["tail", …]) rather than a shell, and the tail target comes from find output rather than user input — so no obvious command-injection surface, and reads are gated by require_viewer. But that new syslog-receiver deployment path is exactly the kind of scope I said I'd review after the PR is green and has a stated design; it shouldn't be expanding while the blockers sit.

Recommendation, unchanged and now more pointed: please stop adding features. Get CI green, restore (or deliberately justify in vite.config.ts) the Monaco worker wiring, and fill in the description with a linked issue/ADR. Adding a syslog receiver on top of an already-blocked 16k-line PR makes it harder to land, not easier.

[jgruberf5]

Review + resolution of the CHANGES_REQUESTED items @ 55465e1c

Picking this up to get it back to a reviewable state. I've addressed every blocker from your two reviews, merged staging, and fixed several CI failures that only surfaced once the branch was current. Mapping each of your points to what changed:

Your blockers

Blocker (from 7229cd0 / c91f84f)	Status
🔴 Monaco worker wiring removed (air-gap regression)	Restored. frontend-v2/src/main.tsx again wires self.MonacoEnvironment.getWorker with the monaco-editor/esm/vs/editor/editor.worker?worker import, alongside the retained loader.config({ monaco }). The worker runs off the main thread again; nothing falls back to CDN. Your own note confirmed the ?worker import builds in this repo's CI, so no vite.config.ts workaround was needed — the deletion, not the import, was the problem.
🟠 PR description was the unedited template	Rewritten. Real description, Implements: #136 linked, and an explicit note that later commits grew a WAF dashboard + Gateway page beyond the original Policy-Manager scope (your scope point, surfaced rather than hidden).
🟡 No response_model on the routes	Fixed. 17 response_model= on waf_policies.py and 1 on waf_logs.py, with new Pydantic schemas in backend/schemas/waf.py; openapi.json + api-generated.ts regenerated so the generated types are real.
🔴 CI red (staleness)	Merged staging (.gitignore union; package-lock.json regenerated via npm install off staging's package.json, keeping axios 1.18.1 / js-yaml 4.3.1 — not hand-merged). Then fixed the failures below.
⚪ Unrelated changes	destructive-confirm-dialog.tsx autoFocus reverted; vite.config.ts blank line reverted. (useTaskWebSocket.ts window.setTimeout is a correct typing fix and is retained.)

CI failures found + fixed after the merge (whole-tree gates the changed-files checks missed)

1. Lint Backend — 15 ruff errors in the branch's own WAF routes/models (unsorted/unused imports, if x: return one-liners, ambiguous l, a dead items = []). Mechanical, no behavior change; ruff now clean.
2. Migration Collision — the staging merge left duplicate revision ids (the WAF migrations reused v2_143/v2_144, already taken on staging). Renumbered them to chain linearly after the current staging head v2_155 → v2_156 (ingestion-cursors) → v2_157 (panels) → v2_158 (dashboard-tabs). Single linear chain.
3. Migration Upgrade From Released (Postgres) — two real schema bugs that were masked while the collision gate stayed red and skipped this Postgres job:
   • waf_panels FK-referenced a non-existent table k8s_clusters (the clusters table is kubernetes_clusters); SQLite doesn't validate FK targets at CREATE so it was never caught locally. Dropped the FK to match the sibling WAF tables' plain indexed cluster_id and v2_158's own "no FK to keep schema portable, matches waf_panels" comment.
   • The three WAF tables were chain-only (created by migrations but not by create_all): WafPanel/WafDashboardTab were never imported in models/__init__.py, and waf_ingestion_cursors had no ORM model. Registered all three and reconciled the models to their migrations exactly (added the missing created_by column + the cluster indexes, added a WafIngestionCursor model). check-schema-parity.py now agrees on every table, column and index.
4. Lint Frontend — restricted useMutation imports (3 WAF hooks now use useAppMutation), two D-020 raw-palette colors (text-blue-500→text-info, bg-orange-400→bg-warning), a ternary-as-statement → if/else.
5. Unit Tests · Frontend — the SecurityLogsTab test still asserted the old free-text vs_name… input; the vs filter is now a <Select>. Updated to assert the rendered "All virtual servers" option. Full frontend suite green (2609 passed); backend WAF tests 34 passed.

Remaining red: the two repo-wide P4 security gates (not this PR's code)

P4 · Docker Build + Scan (Trivy CRITICAL CVE-2026-56854, x/crypto/ssh in helm 3.20.0) and P4 · Security Audit (gitpython 3.1.58) are environmental — the advisory/vuln DBs updated after staging last audited clean on 2026-08-24, so every open PR and staging itself would be red. Both are fixed in #215 (documented .trivyignore suppression for the un-fixable-yet helm CVE + a gitpython patch bump); this branch goes green on P4 once #215 merges and it rebases.

Still owed by the author (I deliberately did not invent these)

• ADR. #136 is now linked, but I won't fabricate an ADR / design record for a feature I didn't design — if AGENTS.md requires a formal ADR for something this size, that's yours to write.
• The scope-split decision. The dashboard / Gateway page / syslog-receiver that grew on top of the original Policy-Manager scope is a maintainer call — I've surfaced it in the description rather than unilaterally removing working features.

I can't dismiss the CHANGES_REQUESTED (that's yours), but the four blockers are addressed and the branch is green on P1–P3. Requesting re-review of the wizard/forms/route logic against #136, with the ADR/scope-split as the two items still needing your/the author's decision.

https://claude.ai/code/session_01UCsZXDxBsWV2s4kT47DwDW