Waf policy manager UI

[singamL887]

Description

A clear and concise description of the changes in this Pull Request.

Fixes / Implements: #[Issue Number]

---

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).

---

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

Describe the tests you ran to verify your changes:

1. make quick-check passes cleanly
2. make pre-push passes cleanly
3. Added unit/component tests for new logic

---

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).

[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.