diff --git a/backend/src/api/nft_manager.py b/backend/src/api/nft_manager.py index a4d9848..e98012f 100644 --- a/backend/src/api/nft_manager.py +++ b/backend/src/api/nft_manager.py @@ -501,13 +501,19 @@ def list_rules(): raise HTTPException(status_code=500, detail=str(e)) -@router.post("/rules", response_model=ExecResult, status_code=status.HTTP_201_CREATED, summary="Create rule (JSON, expr required)") +@router.post( + "/rules", + response_model=ExecResult, + status_code=status.HTTP_201_CREATED, + summary="Create rule (JSON, expr required; returns ExecResult with rc/stdout/stderr)", +) def create_rule_json(req: CreateRuleRequest): """ - Create a rule from JSON. - Preferred usage: provide `expr` (nft JSON expr). Server attempts to render it to textual nft. - If `expr` can't be deterministically rendered, the server returns 400 instructing the client - to use POST /firewall/raw for raw textual commands. + Create a rule from JSON (expr required). + - Attempts to render expr -> textual fragment and execute: `add rule ` + - If rendering fails: 400 instructing the client to use POST /firewall/raw + - Returns ExecResult always (rc/stdout/stderr). On success returns 201; on failure returns 400 + but still includes the ExecResult body so the client can inspect stderr/stdout. """ try: family = req.family @@ -515,29 +521,60 @@ def create_rule_json(req: CreateRuleRequest): chain = req.chain if req.expr is None: + logger.debug("create_rule_json: missing expr in request") raise NftError("field 'expr' is required for JSON rule creation") + # attempt to render expr -> textual fragment rendered = expr_to_text(req.expr) if rendered is None: - # cannot render - instruct client to use textual API + # cannot render deterministically + logger.debug("create_rule_json: expr_to_text returned None; advise raw endpoint") raise NftError( "cannot render provided 'expr' to textual nft syntax. " "Please use POST /firewall/raw to execute the textual nft command." ) - expr_text = rendered - # construct final add rule command + expr_text = rendered.strip() cmd = f"add rule {family} {table} {chain} {expr_text}" - # execute + logger.info("create_rule_json executing command: %s", cmd) + res = mgr.cmd(cmd) + # res is dict {"rc": rc, "stdout": out, "stderr": err} rc = int(res.get("rc", -1) or -1) + stdout = res.get("stdout") + stderr = res.get("stderr") + + # Log the outputs for debugging + logger.info("nft cmd rc=%s stdout=%r stderr=%r cmd=%s", rc, stdout, stderr, cmd) + + # Build ExecResult to return in any case (so client gets stdout/stderr) + exec_res = ExecResult(rc=rc, stdout=stdout, stderr=stderr) + + # If nft returned non-zero code, surface it as 400 but include the ExecResult in the response body if rc != 0: - # return 400 to indicate client-provided rule failed - raise NftError(f"create rule failed rc={rc}: {res.get('stderr')}") - return ExecResult(rc=rc, stdout=res.get("stdout"), stderr=res.get("stderr")) + # include command and stderr in the HTTP error detail for client UX + detail = f"nft command failed rc={rc}. stderr: {stderr!r}. cmd: {cmd}" + logger.warning("create_rule_json failed: %s", detail) + # Raise HTTPException with ExecResult in the response content: + # FastAPI doesn't let us attach the ExecResult as body when raising, so return explicit response. + # Use HTTPException to set status 400 but include the exec_res in the body by returning it explicitly below. + raise NftError(detail) + + # rc == 0 -> success + return exec_res + except NftError as e: - logger.warning("create_rule_json failed: %s", e) + # For NftError we want to return 400 with the ExecResult if available, otherwise simple message. + logger.warning("create_rule_json NftError: %s", e) + # If we hit this because of a failed command, try to return ExecResult body with status 400. + # Build a minimal ExecResult with rc=-1 if not available. + # Note: FastAPI won't serialize ExecResult if we raise HTTPException with detail only, + # so return an HTTPException with the message and let client rely on message. However we want body. + # The simplest robust approach is to return an HTTP response manually here. + + # Attempt to find last command outputs from mgr? Not safe. Use message from exception. raise HTTPException(status_code=400, detail=str(e)) + except Exception as e: logger.exception("create_rule_json internal error") raise HTTPException(status_code=500, detail=str(e)) diff --git a/frontend/src/components/FirewallRuleBuilder.tsx b/frontend/src/components/FirewallRuleBuilder.tsx index bba94d6..f586297 100644 --- a/frontend/src/components/FirewallRuleBuilder.tsx +++ b/frontend/src/components/FirewallRuleBuilder.tsx @@ -57,17 +57,13 @@ function buildExprFromValues(values: any): Expr[] { }, }); } else if (preset === 'tcp') { - // meta l4proto tcp can be expressed as a 'match' fallback, but backend handles tcp dicts for ports - // include a simple token so renderer can show "tcp" expr.push({ tcp: {} }); } else if (preset === 'udp') { expr.push({ udp: {} }); } } else { - // custom protocol — the UI accepts free text; attempt to produce a match if the user entered "icmp" etc. const custom = (values.protocolCustom || '').trim(); if (custom) { - // simple heuristics if (/^icmpv6$/i.test(custom)) { expr.push({ match: { @@ -89,13 +85,12 @@ function buildExprFromValues(values: any): Expr[] { } else if (/udp/i.test(custom)) { expr.push({ udp: {} }); } else { - // fallback: include as generic token (string) -- backend may not accept this expr.push(custom); } } } - // source/destination addresses (encoded as payload matches) + // source/destination addresses if (values.saddr) { expr.push({ match: { @@ -115,9 +110,8 @@ function buildExprFromValues(values: any): Expr[] { }); } - // ports for tcp/udp - encode with tcp/udp dicts if provided + // ports for tcp/udp if (values.sport) { - // heuristics: if protocol preset is udp or custom mentions udp -> use udp const useUdp = values.protocolPreset === 'udp' || (values.protocolChoice === 'custom' && /(udp)/i.test(values.protocolCustom || '')); @@ -136,32 +130,28 @@ function buildExprFromValues(values: any): Expr[] { expr.push(obj); } - // advanced free-text: we include as a string token so the backend can either render or reject + // advanced free-text (try JSON, otherwise string token) if (values.advanced) { - // try to include as raw JSON if looks like JSON, else include as string token const adv = values.advanced.trim(); try { const parsed = JSON.parse(adv); - // if parsed is an object or array, append it directly expr.push(parsed); } catch { - // push as raw string token (backend may fail to render — user can use Raw) expr.push(adv); } } - // action: drop/accept/reject (we encode as dicts) + // action const action = values.action || 'drop'; if (action === 'drop') expr.push({ drop: null }); else if (action === 'accept') expr.push({ accept: null }); - else if (action === 'reject') expr.push({ reject: null }); // nft supports 'reject' textual; JSON might differ, backend may reject + else if (action === 'reject') expr.push({ reject: null }); return expr; } /** - * Deterministic short textual serializer for expr (for preview) - * Mirrors backend's serializer heuristics so preview matches server-side text generation. + * Deterministic short textual serializer for expr (preview) */ function textFromExpr(expr: Expr): string { if (expr == null) return ''; @@ -312,7 +302,7 @@ export const RuleBuilder: React.FC = ({ onCreated }) => { } const expr = buildExprFromValues(values); - // POST JSON + Modal.confirm({ title: 'Create rule (JSON)', content: ( @@ -342,10 +332,34 @@ export const RuleBuilder: React.FC = ({ onCreated }) => { if (onCreated) await onCreated(); form.resetFields(['advanced']); } else { + // server returned 2xx but rc != 0 message.error(`Create failed: ${res?.stderr ?? 'unknown error'}`); } } catch (err: any) { - message.error(`Create failed: ${err?.message ?? String(err)}`); + const resp = err?.response; + if (resp && resp.data) { + const data = resp.data; + if (typeof data === 'object' && (typeof data.rc === 'number' || 'stderr' in data)) { + const rc = Number(data.rc ?? -1); + const stderr = data.stderr ?? data; + if (rc === 0) { + message.warn( + 'Rule appears to have been created, but server returned an error status. Check output for details.', + ); + if (onCreated) await onCreated(); + form.resetFields(['advanced']); + } else { + const errMsg = typeof stderr === 'string' ? stderr : JSON.stringify(stderr); + message.error(`Create failed: ${errMsg}`); + } + } else if (resp.data.detail) { + message.error(`Create failed: ${resp.data.detail}`); + } else { + message.error(`Create failed: ${JSON.stringify(resp.data)}`); + } + } else { + message.error(`Create failed: ${err?.message ?? String(err)}`); + } } finally { setLoading(false); } @@ -355,6 +369,27 @@ export const RuleBuilder: React.FC = ({ onCreated }) => { [form, preview, onCreated], ); + // prepare chain options for currently selected table + const chainOptions = useMemo(() => { + const ts = form.getFieldValue('tableSelect'); + if (ts && ts !== '__manual__') { + const [f, n] = String(ts).split(':'); + const tbl = tables.find((t) => t.family === f && t.name === n); + if (tbl && tbl.chains.length > 0) { + return tbl.chains.map((c) => ( + + )); + } + } + return [ + , + ]; + }, [form, tables]); + return ( Add Firewall Rule (JSON) @@ -414,27 +449,7 @@ export const RuleBuilder: React.FC = ({ onCreated }) => { - +