fix(config+record): rollback zone files on checkzone failure; report rndc reload failures; fix named-checkconf root cause

Three classes of silent-failure bugs were leaving Web UI actions in
limbo without telling the operator:

1. record_add / record_delete:
   - The file was already mutated + serial bumped before named-checkzone
     ran. If checkzone failed, the user saw an error flash but the
     half-applied record was on disk + serial had advanced. Re-running
     would create duplicates.
   - bind_reload() return value was discarded, so an rndc SERVFAIL was
     silently logged as '添加成功'.

2. zone_create / zone_delete:
   - Same bind_reload() silent-discard bug.

3. config_options_save / config_upstream_save:
   - 'named-checkconf /tmp/named_check.tmp' treated the tmp file as
     the master config, which refuses options-only directives
     (0.0.0.0 / dnssec-validation / listen-on-v6) at the top level →
     false-positive 'unknown option' validation failures that left
     the user unable to save any options change.
   - Auto-wrap logic re-wrapped already-valid options blocks, creating
     'options { options { ... }; };' which named-checkconf rejected.
   - No rollback: failures left the live file in whatever state the
     tmp had mutated it to.

Fixes:
- append_record_to_file / delete_record_from_file now return the
  original content; callers pass it to rollback_zone_file() on
  checkzone failure.
- record_delete now runs named-checkzone (was missing entirely).
- All four write paths (record add/delete, zone create/delete)
  surface rndc reload failures instead of always flashing '成功'.
- config_options_save and config_upstream_save now validate via
  'named-checkconf' (whole config) after writing BIND_CONF_OPTIONS,
  with .bak-based rollback on failure — no more /tmp/tmpfile tricks.

Verified end-to-end on prod: bad named.conf.options content triggers
HTTP 302 + '已恢复备份' flash, live file untouched, named-checkconf
remains OK; good content saves + reloads cleanly.

app.py

@@ -451,45 +451,74 @@ def generate_zone_file(zone_name, records, soa=None):
 def append_record_to_file(filepath, name, ttl, rtype, data):
     """Append a new record to the zone file and bump the serial.
 
-    Returns (success, message).
+    Returns (success, message, original_content_for_rollback).
+    The original_content can be passed to rollback_zone_file() if a
+    subsequent named-checkzone / bind_reload step fails.
     """
     try:
         with open(filepath) as f:
-            content = f.read()
+            original = f.read()
     except FileNotFoundError:
-        return False, "Zone file not found"
+        return False, "Zone file not found", None
 
     # Build the new record line
     ttl_str = f'{ttl}  ' if ttl else ''
     new_line = f'{name}  {ttl_str}IN  {rtype:<6} {data}\n'
 
     # Append record at end of file
-    content = content.rstrip('\n') + '\n' + new_line
+    content = original.rstrip('\n') + '\n' + new_line
 
     # Update serial
     content = bump_serial_in_content(content)
 
-    with open(filepath, 'w') as f:
+    try:
-        f.write(content)
+        with open(filepath, 'w') as f:
+            f.write(content)
+    except OSError as e:
+        return False, f"Write zone file failed: {e}", None
     run_cmd(f"chown bind:bind {filepath} 2>/dev/null || chown named:named {filepath} 2>/dev/null")
     run_cmd(f"chmod 644 {filepath}")
-    return True, "OK"
+    return True, "OK", original
+
+
+def rollback_zone_file(filepath, original_content):
+    """Restore a zone file to its previous content. Used when a record
+    write succeeds but a downstream check (named-checkzone / rndc reload)
+    fails — without this the user would see an error flash while the
+    record is already on disk, leading to confusion on retry.
+    """
+    if original_content is None:
+        return
+    try:
+        with open(filepath, 'w') as f:
+            f.write(original_content)
+        run_cmd(f"chown bind:bind {filepath} 2>/dev/null || chown named:named {filepath} 2>/dev/null")
+        run_cmd(f"chmod 644 {filepath}")
+    except OSError:
+        # Best-effort: if we can't restore, the operator must intervene.
+        pass
 
 
 def delete_record_from_file(filepath, zone_name, idx):
     """Delete the Nth record (0-indexed, excluding SOA) from the zone file.
 
-    Returns (success, deleted_record_str, message).
+    Returns (success, deleted_record_str, message, original_content_for_rollback).
+    Caller should pass original_content to rollback_zone_file() if a
+    subsequent named-checkzone / bind_reload step fails.
     """
     soa, records = parse_zone_file(filepath)
     if idx < 0 or idx >= len(records):
-        return False, "", "Record index out of range"
+        return False, "", "Record index out of range", None
 
     deleted = records[idx]
 
     # Read raw lines and find the matching record line to remove
-    with open(filepath) as f:
+    try:
-        raw_lines = f.readlines()
+        with open(filepath) as f:
+            original = f.read()
+            raw_lines = original.splitlines(keepends=True)
+    except FileNotFoundError:
+        return False, "", "Zone file not found", None
 
     # We need to find the Nth non-SOA, non-directive, non-comment, non-empty line
     # and remove it. This is tricky because the record might span lines.
@@ -534,16 +563,27 @@ def delete_record_from_file(filepath, zone_name, idx):
         new_lines.append(line)
 
     if not removed:
-        return False, "", "Could not find record to delete"
+        return False, "", "Could not find record to delete", None
 
     content = ''.join(new_lines)
     content = bump_serial_in_content(content)
 
-    with open(filepath, 'w') as f:
+    try:
-        f.write(content)
+        with open(filepath, 'w') as f:
+            f.write(content)
+    except OSError as e:
+        return False, "", f"Write zone file failed: {e}", None
     run_cmd(f"chown bind:bind {filepath} 2>/dev/null || chown named:named {filepath} 2>/dev/null")
     run_cmd(f"chmod 644 {filepath}")
-    return True, f'{deleted["name"]} {deleted["type"]} {deleted["data"]}', "OK"
+    return True, deleted_str_for(deleted), "OK", original
+
+
+def deleted_str_for(deleted):
+    """Compact human-readable summary of a record for audit logs."""
+    name = deleted.get("name", "")
+    rtype = deleted.get("type", "")
+    data = deleted.get("data", "")
+    return f"{name} {rtype} {data}".strip()
 
 
 def bump_serial_in_content(content):
@@ -798,11 +838,20 @@ def zone_create():
         write_named_conf_local(existing)
 
         # Reload BIND
-        bind_reload()
+        reload_ok, reload_msg = bind_reload()
 
         u = current_user()
         AuditLog.log(u.username, "创建Zone", f"域名: {zone_name}, 类型: {zone_type}")
-        flash(f"域名 {zone_name} 创建成功", "success")
+        if reload_ok:
+            flash(f"域名 {zone_name} 创建成功", "success")
+        else:
+            # Files are on disk and named.conf.local is updated, but BIND
+            # refused reload — surface it loudly so the operator doesn't
+            # assume the zone is live.
+            flash(
+                f"域名 {zone_name} 配置已写入,但 BIND 重载失败: {reload_msg}",
+                "error",
+            )
         return redirect(url_for("zone_detail", zone_name=zone_name))
 
     return render_template("zone_form.html")
@@ -849,11 +898,17 @@ def zone_delete(zone_name):
     zones = [z for z in zones if z["name"] != zone_name]
     write_named_conf_local(zones)
 
-    bind_reload()
+    reload_ok, reload_msg = bind_reload()
 
     u = current_user()
     AuditLog.log(u.username, "删除Zone", f"域名: {zone_name}")
-    flash(f"域名 {zone_name} 已删除", "success")
+    if reload_ok:
+        flash(f"域名 {zone_name} 已删除", "success")
+    else:
+        flash(
+            f"域名 {zone_name} 已从配置移除,但 BIND 重载失败: {reload_msg}",
+            "error",
+        )
     return redirect(url_for("zone_list"))
 
 
@@ -932,22 +987,36 @@ def record_add(zone_name):
         name = "@"
 
     # Append record directly to zone file and bump serial
-    ok, msg = append_record_to_file(zone["file"], name, ttl, rtype, data)
+    ok, msg, original = append_record_to_file(zone["file"], name, ttl, rtype, data)
     if not ok:
         flash(f"添加记录失败: {msg}", "error")
         return redirect(url_for("zone_detail", zone_name=zone_name))
 
-    # Validate zone
+    # Validate zone; on failure roll back the write so the user can retry cleanly
+    # without leaving a half-applied record + bumped serial on disk.
     rc, out, err = run_cmd(f"named-checkzone {zone_name} {zone['file']}")
     if rc != 0:
-        flash(f"Zone 验证失败: {out} {err}", "error")
+        rollback_zone_file(zone["file"], original)
+        flash(
+            f"Zone 验证失败: {out} {err} (已回滚,记录未保存)",
+            "error",
+        )
         return redirect(url_for("zone_detail", zone_name=zone_name))
 
-    bind_reload()
+    reload_ok, reload_msg = bind_reload()
 
     u = current_user()
     AuditLog.log(u.username, "添加记录", f"域名: {zone_name}, {name} {rtype} {data}")
-    flash(f"记录 {name} {rtype} 添加成功", "success")
+    if reload_ok:
+        flash(f"记录 {name} {rtype} 添加成功", "success")
+    else:
+        # Record IS on disk and zone validated, but BIND refused reload — likely
+        # SERVFAIL or rndc permission error. Surface it loudly so the operator
+        # doesn't think the change took effect.
+        flash(
+            f"记录 {name} {rtype} 已写入,但 BIND 重载失败: {reload_msg}",
+            "error",
+        )
     return redirect(url_for("zone_detail", zone_name=zone_name))
 
 
@@ -961,16 +1030,32 @@ def record_delete(zone_name, idx):
         abort(404)
 
     # Delete record from file and bump serial
-    ok, deleted_str, msg = delete_record_from_file(zone["file"], zone_name, idx)
+    ok, deleted_str, msg, original = delete_record_from_file(zone["file"], zone_name, idx)
     if not ok:
         flash(f"删除记录失败: {msg}", "error")
         return redirect(url_for("zone_detail", zone_name=zone_name))
 
-    bind_reload()
+    # Validate zone; roll back on failure (matches record_add behaviour).
+    rc, out, err = run_cmd(f"named-checkzone {zone_name} {zone['file']}")
+    if rc != 0:
+        rollback_zone_file(zone["file"], original)
+        flash(
+            f"Zone 验证失败: {out} {err} (已回滚,记录未删除)",
+            "error",
+        )
+        return redirect(url_for("zone_detail", zone_name=zone_name))
+
+    reload_ok, reload_msg = bind_reload()
 
     u = current_user()
     AuditLog.log(u.username, "删除记录", f"域名: {zone_name}, {deleted_str}")
-    flash(f"记录 {deleted_str} 已删除", "success")
+    if reload_ok:
+        flash(f"记录 {deleted_str} 已删除", "success")
+    else:
+        flash(
+            f"记录 {deleted_str} 已从文件删除,但 BIND 重载失败: {reload_msg}",
+            "error",
+        )
     return redirect(url_for("zone_detail", zone_name=zone_name))
 
 
@@ -1293,32 +1378,42 @@ def config_view():
 def config_options_save():
     content = request.form.get("content", "")
     backup = BIND_CONF_OPTIONS + ".bak"
+
+    # Validate BEFORE writing to the live file.
+    # Strategy: write the candidate to BIND_CONF_OPTIONS, validate the WHOLE
+    # named.conf (not just the tmp file — named-checkconf /tmp/foo treats
+    # foo as the master config and refuses 'options-only' directives like
+    # 0.0.0.0 / dnssec-validation / listen-on-v6 at the top level).
+    # If validation fails, restore from backup immediately.
     run_cmd(f"cp {BIND_CONF_OPTIONS} {backup}")
 
-    # Write to temp and validate
+    try:
-    # NOTE: BIND_CONF_OPTIONS content must have 'options { ... }' wrapper (it's a complete file)
+        with open(BIND_CONF_OPTIONS, 'w') as f:
-    # that gets included from named.conf, so named-checkconf can validate it directly
+            f.write(content)
-    if not re.search(r'^\s*options\s*\{', content, re.MULTILINE):
+        run_cmd(f"chown root:bind {BIND_CONF_OPTIONS} 2>/dev/null || chown root:named {BIND_CONF_OPTIONS} 2>/dev/null")
-        # User may have accidentally removed the options wrapper in text editor
+        rc, out, err = run_cmd("named-checkconf")
-        content = f"options {{\n{content}\n}};\n"
+        if rc != 0:
-    with open("/tmp/named_check.tmp", 'w') as f:
+            # Roll back the live file BEFORE the user sees the error flash
-        f.write(content)
+            run_cmd(f"cp {backup} {BIND_CONF_OPTIONS}")
-    rc, out, err = run_cmd("named-checkconf /tmp/named_check.tmp")
+            run_cmd(f"chown root:bind {BIND_CONF_OPTIONS} 2>/dev/null || chown root:named {BIND_CONF_OPTIONS} 2>/dev/null")
-    run_cmd("rm -f /tmp/named_check.tmp")
+            flash(f"配置验证失败,已恢复备份: {out} {err}", "error")
-
+            return redirect(url_for("config_view"))
-    if rc != 0:
+    except Exception as e:
-        flash(f"配置验证失败: {out} {err}", "error")
+        run_cmd(f"cp {backup} {BIND_CONF_OPTIONS}")
+        flash(f"保存配置失败,已恢复备份: {e}", "error")
         return redirect(url_for("config_view"))
 
-    with open(BIND_CONF_OPTIONS, 'w') as f:
+    reload_ok, reload_msg = bind_reload()
-        f.write(content)
-    run_cmd(f"chown root:bind {BIND_CONF_OPTIONS} 2>/dev/null || chown root:named {BIND_CONF_OPTIONS} 2>/dev/null")
-
-    bind_reload()
 
     u = current_user()
     AuditLog.log(u.username, "修改BIND options配置")
-    flash("options 配置已保存", "success")
+    if reload_ok:
+        flash("options 配置已保存", "success")
+    else:
+        flash(
+            f"options 配置已写入,但 BIND 重载失败: {reload_msg}",
+            "error",
+        )
     return redirect(url_for("config_view"))
 
 
@@ -1424,39 +1519,46 @@ def config_upstream_save():
         f"recursion {recursion_in};\n",
     )
 
-    # Validate BEFORE writing to the live file
+    # Validate BEFORE writing to the live file.
-    # Ensure content has options block wrapper (required for named-checkconf)
+    # Strategy: write the candidate to BIND_CONF_OPTIONS, validate the WHOLE
-    if not re.search(r'^\s*options\s*\{', new_content, re.MULTILINE):
+    # named.conf (named-checkconf /tmp/foo treats foo as the master config
-        # No options block found — wrap the entire content
+    # and refuses 'options-only' directives at the top level).
-        new_content = f"options {{\n{new_content}\n}};\n"
+    # If validation fails, restore from backup immediately.
-    tmp = "/tmp/named_check.tmp"
-    with open(tmp, 'w') as f:
-        f.write(new_content)
-    rc, out, err = run_cmd(f"named-checkconf {tmp}")
-    run_cmd(f"rm -f {tmp}")
-    if rc != 0:
-        flash(f"配置验证失败: {out} {err}", "error")
-        return redirect(url_for("config_view"))
-
-    # Backup + write
     backup = BIND_CONF_OPTIONS + ".bak"
     run_cmd(f"cp {BIND_CONF_OPTIONS} {backup}")
-    with open(BIND_CONF_OPTIONS, 'w') as f:
+    try:
-        f.write(new_content)
+        with open(BIND_CONF_OPTIONS, 'w') as f:
-    run_cmd(f"chown root:bind {BIND_CONF_OPTIONS} 2>/dev/null || chown root:named {BIND_CONF_OPTIONS} 2>/dev/null")
+            f.write(new_content)
+        run_cmd(f"chown root:bind {BIND_CONF_OPTIONS} 2>/dev/null || chown root:named {BIND_CONF_OPTIONS} 2>/dev/null")
+        rc, out, err = run_cmd("named-checkconf")
+        if rc != 0:
+            run_cmd(f"cp {backup} {BIND_CONF_OPTIONS}")
+            run_cmd(f"chown root:bind {BIND_CONF_OPTIONS} 2>/dev/null || chown root:named {BIND_CONF_OPTIONS} 2>/dev/null")
+            flash(f"配置验证失败,已恢复备份: {out} {err}", "error")
+            return redirect(url_for("config_view"))
+    except Exception as e:
+        run_cmd(f"cp {backup} {BIND_CONF_OPTIONS}")
+        flash(f"保存配置失败,已恢复备份: {e}", "error")
+        return redirect(url_for("config_view"))
 
-    bind_reload()
+    reload_ok, reload_msg = bind_reload()
 
     u = current_user()
     AuditLog.log(u.username, "修改上游DNS转发配置",
                  f"forwarders={new_fwd}; allow-recursion={new_rc}; "
                  f"dnssec={dnssec_in}; listen-on={new_listen}; listen-on-v6={listen_on_v6_in}; recursion={recursion_in}")
-    flash(
+    if reload_ok:
-        f"已保存：forwarders {len(new_fwd)} 条，allow-recursion {len(new_rc)} 条，"
+        flash(
-        f"dnssec-validation {dnssec_in}，listen-on {len(new_listen)} 项，"
+            f"已保存：forwarders {len(new_fwd)} 条，allow-recursion {len(new_rc)} 条，"
-        f"listen-on-v6 {listen_on_v6_in}，recursion {recursion_in}",
+            f"dnssec-validation {dnssec_in}，listen-on {len(new_listen)} 项，"
-        "success",
+            f"listen-on-v6 {listen_on_v6_in}，recursion {recursion_in}",
-    )
+            "success",
+        )
+    else:
+        flash(
+            f"上游 DNS 配置已写入,但 BIND 重载失败: {reload_msg}",
+            "error",
+        )
     return redirect(url_for("config_view"))