bindcar 0.8.1

// Copyright (c) 2025 Erick Bourgeois, firestoned
// SPDX-License-Identifier: MIT

//! Zone management API handlers
//!
//! This module implements HTTP handlers for all zone-related operations:
//! - Creating zones (with zone file creation)
//! - Deleting zones
//! - Reloading zones
//! - Getting zone status
//! - Freezing/thawing zones
//! - Notifying secondaries

use axum::{
    extract::{Path, State},
    http::StatusCode,
    Json,
};
use std::path::PathBuf;
use tracing::{debug, error, info, warn};

use crate::{
    metrics,
    types::{ApiError, AppState},
};

// The pure zone data types live in `zones_types` so that a library consumer can
// take them without the HTTP stack (roadmap 02, phase 1). They are re-exported
// here unchanged, so `bindcar::zones::ZoneConfig` and friends keep resolving.
pub use crate::zones_types::{
    CheckdsRequest, CreateZoneRequest, DnsRecord, DsRecordView, DsSetResponse, ModifyZoneRequest,
    ServerStatusResponse, SoaRecord, ZoneConfig, ZoneInfo, ZoneListResponse, ZoneResponse,
    ZoneStatusResponse, ZONE_TYPE_PRIMARY, ZONE_TYPE_SECONDARY,
};

/// Maximum length of a DNS zone name (RFC 1035 total name length).
const MAX_ZONE_NAME_LEN: usize = 253;

/// Maximum length of an RNDC configuration identifier (TSIG key / policy name).
const MAX_RNDC_IDENTIFIER_LEN: usize = 253;

/// Validate a zone name against a strict DNS-name grammar.
///
/// The zone name is interpolated into both a filesystem path (`<zone>.zone`) and
/// RNDC `addzone`/`delzone` commands, so it must be tightly constrained to prevent
/// path traversal (B-1) and command injection. Only the DNS character set
/// `[A-Za-z0-9._-]` is permitted, the name must start with an alphanumeric
/// character, and parent-directory references (`..`), path separators, whitespace,
/// NUL, and other control characters are rejected.
///
/// # Errors
/// Returns [`ApiError::InvalidRequest`] (HTTP 400) when the name is empty, too
/// long, or contains a disallowed character or sequence.
pub(crate) fn validate_zone_name(zone_name: &str) -> Result<(), ApiError> {
    if zone_name.is_empty() {
        return Err(ApiError::InvalidRequest(
            "Zone name cannot be empty".to_string(),
        ));
    }

    if zone_name.len() > MAX_ZONE_NAME_LEN {
        return Err(ApiError::InvalidRequest(format!(
            "Zone name exceeds maximum length of {} characters",
            MAX_ZONE_NAME_LEN
        )));
    }

    // Reject parent-directory references outright (defense-in-depth against
    // path traversal even though '/' is already rejected below).
    if zone_name.contains("..") {
        return Err(ApiError::InvalidRequest(
            "Zone name must not contain '..'".to_string(),
        ));
    }

    // The first character must be alphanumeric (no leading dot, hyphen, or
    // separator that could anchor a traversal or empty label).
    let starts_alphanumeric = zone_name
        .chars()
        .next()
        .is_some_and(|c| c.is_ascii_alphanumeric());
    if !starts_alphanumeric {
        return Err(ApiError::InvalidRequest(
            "Zone name must start with an alphanumeric character".to_string(),
        ));
    }

    // Every character must be in the permitted DNS set. This rejects '/', '\\',
    // whitespace, NUL, quotes, semicolons, braces, and all control characters.
    for c in zone_name.chars() {
        if !(c.is_ascii_alphanumeric() || c == '.' || c == '-' || c == '_') {
            return Err(ApiError::InvalidRequest(format!(
                "Zone name contains invalid character: {:?}",
                c
            )));
        }
    }

    Ok(())
}

/// Validate an RNDC configuration identifier such as a TSIG key name or a
/// `dnssec-policy` name.
///
/// These values are interpolated into quoted RNDC `addzone` config literals
/// (e.g. `allow-update {{ key "<name>"; }}`), so any `"`, `;`, `{`, `}`,
/// whitespace, or control character could break out of the quoted context and
/// inject arbitrary BIND configuration (B-3). Only the safe identifier set
/// `[A-Za-z0-9._-]` is permitted.
///
/// # Arguments
/// * `field` - Human-readable field name used in error messages.
/// * `value` - The identifier value to validate.
///
/// # Errors
/// Returns [`ApiError::InvalidRequest`] (HTTP 400) when the identifier is empty,
/// too long, or contains a disallowed character.
pub(crate) fn validate_rndc_identifier(field: &str, value: &str) -> Result<(), ApiError> {
    if value.is_empty() {
        return Err(ApiError::InvalidRequest(format!(
            "{} cannot be empty",
            field
        )));
    }

    if value.len() > MAX_RNDC_IDENTIFIER_LEN {
        return Err(ApiError::InvalidRequest(format!(
            "{} exceeds maximum length of {} characters",
            field, MAX_RNDC_IDENTIFIER_LEN
        )));
    }

    for c in value.chars() {
        if !(c.is_ascii_alphanumeric() || c == '.' || c == '-' || c == '_') {
            return Err(ApiError::InvalidRequest(format!(
                "{} contains invalid character: {:?}",
                field, c
            )));
        }
    }

    Ok(())
}

/// Validate that every entry in an IP-address list parses as an [`IpAddr`].
///
/// `create_zone` interpolates these values directly into the `rndc addzone`
/// configuration literal — `primaries {{ .. }}`, `also-notify {{ .. }}`,
/// `allow-transfer {{ .. }}`. Without this check an entry such as
/// `"1.2.3.4; }; zone \"x\" { type primary; ..."` would close the brace block
/// and inject arbitrary configuration into the running `named` (B-8 / C-1).
/// Requiring a strict `IpAddr` parse rejects every metacharacter (`}`, `{`,
/// `;`, `"`, whitespace) since none can appear in a valid IPv4/IPv6 literal.
///
/// [`IpAddr`]: std::net::IpAddr
///
/// # Errors
/// Returns [`ApiError::InvalidRequest`] (HTTP 400) if any entry is not a valid
/// IP address.
pub(crate) fn validate_ip_list(field: &str, ips: &[String]) -> Result<(), ApiError> {
    for ip in ips {
        if ip.parse::<std::net::IpAddr>().is_err() {
            return Err(ApiError::InvalidRequest(format!(
                "{} contains an invalid IP address: {:?}",
                field, ip
            )));
        }
    }

    Ok(())
}

/// Parse an IP address entry with optional port.
///
/// Accepts:
/// - Bare IPv4: `192.0.2.1`
/// - Bare IPv6: `2001:db8::1`
/// - IPv4 with port: `192.0.2.2:5353`
/// - IPv6 with port (bracketed): `[2001:db8::1]:5353`
///
/// Returns `(address, port)` where port is `None` when not specified.
/// Returns `None` for invalid input.
pub(crate) fn parse_ip_port_entry(s: &str) -> Option<(&str, Option<u16>)> {
    // Bracketed IPv6: [::1] or [::1]:5353
    if let Some(inner) = s.strip_prefix('[') {
        let (addr, rest) = inner.split_once(']')?;
        addr.parse::<std::net::Ipv6Addr>().ok()?;
        let port = match rest.strip_prefix(':') {
            Some(p) => Some(p.parse::<u16>().ok()?),
            None if rest.is_empty() => None,
            _ => return None,
        };
        return Some((addr, port));
    }

    // Try last-colon split for ipv4:port
    let mut parts = s.rsplitn(2, ':');
    let last = parts.next().unwrap();
    let prefix = parts.next();

    if let Some(prefix) = prefix {
        if let Ok(port) = last.parse::<u16>() {
            if prefix.parse::<std::net::Ipv4Addr>().is_ok() {
                return Some((prefix, Some(port)));
            }
        }
    }

    // Bare IP (v4 or v6)
    s.parse::<std::net::IpAddr>().ok()?;
    Some((s, None))
}

/// Render an IP:port entry into BIND config syntax (`ip port N`).
pub(crate) fn render_ip_port_entry(s: &str) -> String {
    if let Some((addr, Some(port))) = parse_ip_port_entry(s) {
        format!("{} port {}; ", addr, port)
    } else {
        format!("{}; ", s)
    }
}

/// Validate transfer endpoints rendered into BIND `primaries` / `also-notify` blocks.
///
/// Accepts either a bare IP address (`192.0.2.1`) or the compact `ip:port`
/// syntax (`192.0.2.2:5353`, `[2001:db8::1]:5353`). This keeps bindcar backward
/// compatible with existing bare-IP requests while allowing callers to run
/// `named` on an unprivileged transfer port.
///
/// Entries are rendered to BIND's `port` keyword syntax internally.
/// `allow-transfer` does not support per-endpoint ports and must use bare IPs.
pub(crate) fn validate_ip_port_list(field: &str, entries: &[String]) -> Result<(), ApiError> {
    for entry in entries {
        if parse_ip_port_entry(entry).is_none() {
            return Err(ApiError::InvalidRequest(format!(
                "{} contains an invalid entry: {:?} (expected \"<ip>\" or \"<ip>:<port>\")",
                field, entry
            )));
        }
    }

    Ok(())
}

/// Validate a zone-file hostname field against the strict DNS name character set.
///
/// [`ZoneConfig::to_zone_file`] interpolates these fields directly into the zone
/// file — and glue hostnames (`nameServerIps` keys) are rendered at the **start
/// of a line**, exactly where BIND master-file directives (`$INCLUDE`,
/// `$GENERATE`, `$ORIGIN`, `$TTL`) are recognized. A control-char-only check is
/// therefore insufficient: a value like `"$INCLUDE /etc/bind/rndc.key ;"`
/// contains no control character yet plants a directive that reads an arbitrary
/// file into the zone (disclosure via AXFR/query) or exhausts resources (B-8 /
/// C-2). Restricting to `[A-Za-z0-9._-]` rejects whitespace, `$`, `;`, quotes,
/// and parens, closing the directive-injection surface while still accepting
/// every legitimate hostname/SOA-email form (FQDNs with a trailing dot).
///
/// # Errors
/// Returns [`ApiError::InvalidRequest`] (HTTP 400) if `value` is empty or contains
/// any character outside the permitted set.
fn validate_zone_file_hostname(field: &str, value: &str) -> Result<(), ApiError> {
    if value.is_empty() {
        return Err(ApiError::InvalidRequest(format!(
            "{} cannot be empty",
            field
        )));
    }

    if let Some(bad) = value
        .chars()
        .find(|&c| !(c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | '_')))
    {
        return Err(ApiError::InvalidRequest(format!(
            "{} contains an illegal character: {:?} (allowed: A-Z a-z 0-9 . - _)",
            field, bad
        )));
    }

    Ok(())
}

/// Validate the structured zone configuration before it is rendered into a zone
/// file by [`ZoneConfig::to_zone_file`].
///
/// Every field that `to_zone_file` interpolates is checked here so that no
/// user-controlled value can inject extra zone-file lines or directives (B-8 /
/// C-2). This closes the gap where records embedded in a `create_zone` request
/// reached the zone file without passing the per-request validation applied by
/// the add/update/remove-record endpoints.
///
/// - SOA `primary_ns` / `admin_email`, name-server names, and glue hostnames must
///   match the strict DNS name charset `[A-Za-z0-9._-]`
///   ([`validate_zone_file_hostname`]) — rejecting whitespace/`$`/`;` so a value
///   rendered at the start of a zone-file line cannot plant a `$INCLUDE` /
///   `$GENERATE` master-file directive.
/// - Glue record IPs must parse as an [`IpAddr`](std::net::IpAddr).
/// - Each embedded record is held to the same rules as the add-record endpoint
///   ([`validate_record_type`](crate::records::validate_record_type),
///   [`validate_record_name`](crate::records::validate_record_name),
///   [`validate_record_value`](crate::records::validate_record_value)).
///
/// # Errors
/// Returns [`ApiError::InvalidRequest`] or [`ApiError::InvalidRecord`] (both
/// HTTP 400) for the first field that fails validation.
pub(crate) fn validate_zone_config_content(config: &ZoneConfig) -> Result<(), ApiError> {
    validate_zone_file_hostname("soa.primaryNs", &config.soa.primary_ns)?;
    validate_zone_file_hostname("soa.adminEmail", &config.soa.admin_email)?;

    for ns in &config.name_servers {
        validate_zone_file_hostname("nameServers entry", ns)?;
    }

    for (host, ip) in &config.name_server_ips {
        validate_zone_file_hostname("nameServerIps key", host)?;
        if ip.parse::<std::net::IpAddr>().is_err() {
            return Err(ApiError::InvalidRequest(format!(
                "nameServerIps[{:?}] is not a valid IP address: {:?}",
                host, ip
            )));
        }
    }

    for record in &config.records {
        crate::records::validate_record_type(&record.record_type)?;
        crate::records::validate_record_name(&record.name)?;
        crate::records::validate_record_value(&record.record_type, &record.value)?;
    }

    Ok(())
}

/// Resolve and validate the configured zone directory at startup.
///
/// The zone directory comes from the `BIND_ZONE_DIR` environment variable, which
/// static analysis (CodeQL `rust/path-injection`) treats as untrusted input.
/// Canonicalizing it once at startup both hardens the server and removes that
/// taint before the path ever reaches a filesystem sink (`read_dir`/`metadata`):
///
/// * Symlinks and `..` segments are resolved against the real filesystem, so the
///   value stored in [`AppState`] is an absolute, fully-normalized path.
/// * A missing path or a path that does not resolve to a directory is rejected
///   up front, turning a late runtime failure into a clear startup error.
///
/// This is the configuration-time counterpart to [`validate_zone_name`], which
/// guards the per-request zone names that are joined onto this directory (B-1).
///
/// # Arguments
/// * `raw_dir` - The configured zone directory path (e.g. from `BIND_ZONE_DIR`).
///
/// # Returns
/// The canonicalized directory path as a UTF-8 `String`.
///
/// # Errors
/// Returns [`ApiError::InternalError`] if the path cannot be canonicalized (for
/// example it does not exist), does not resolve to a directory, or is not valid
/// UTF-8.
pub fn resolve_zone_dir(raw_dir: &str) -> Result<String, ApiError> {
    let canonical = std::fs::canonicalize(raw_dir).map_err(|e| {
        ApiError::InternalError(format!(
            "zone directory {:?} could not be resolved: {}",
            raw_dir, e
        ))
    })?;

    if !canonical.is_dir() {
        return Err(ApiError::InternalError(format!(
            "zone directory {:?} does not resolve to a directory",
            raw_dir
        )));
    }

    canonical.into_os_string().into_string().map_err(|_| {
        ApiError::InternalError(format!(
            "zone directory {:?} resolves to a non-UTF-8 path",
            raw_dir
        ))
    })
}

/// Returns `true` if `path` is an absolute, fully-normalized directory path.
///
/// "Fully-normalized" means the path is absolute and contains no `..`
/// (parent-directory) or `.` (current-directory) components — exactly the shape
/// that [`resolve_zone_dir`] guarantees at startup.
///
/// # Why this exists
///
/// The configured zone directory is canonicalized once at startup, but it is
/// read back inside HTTP handlers through the axum `State` extractor. CodeQL
/// (`rust/path-injection`) models any value reaching a handler via `State` as
/// untrusted, so it re-taints the already-safe path where it feeds a filesystem
/// sink (e.g. `tokio::fs::metadata` in the readiness probe, `tokio::fs::read_dir`
/// when listing zones). Calling this guard
/// immediately before such a sink is a defense-in-depth barrier: it re-asserts
/// the [`resolve_zone_dir`] invariant at the point of use and rejects any path
/// that is unexpectedly relative or contains traversal components, rather than
/// touching an unintended location on the filesystem.
///
/// # Guard shape (required)
///
/// CodeQL's barrier guard is intra-procedural and only takes effect when the
/// guard is an **early return** that leaves the sink in straight-line code,
/// paired with an inline `contains("..")` check:
///
/// ```ignore
/// if !is_normalized_zone_dir(dir) || dir.contains("..") {
///     return /* not ready / error */;
/// }
/// // sink here, in straight-line code
/// tokio::fs::read_dir(dir).await
/// ```
///
/// Writing the same condition as `if bad { .. } else { /* sink */ }` does *not*
/// sanitize the flow — that shape left the readiness probe flagged as
/// code-scanning alert #7 on PR #114, while the early-return form in
/// [`list_zones`] cleared. Keep guard and sink in the same function; calling
/// this helper from a caller of the sink's function does not count.
///
/// # Arguments
/// * `path` - The configured zone directory path to check.
///
/// # Returns
/// `true` if the path is absolute and free of `..`/`.` components, else `false`.
pub fn is_normalized_zone_dir(path: &str) -> bool {
    use std::path::{Component, Path};

    let path = Path::new(path);
    if !path.is_absolute() {
        return false;
    }

    !path
        .components()
        .any(|component| matches!(component, Component::ParentDir | Component::CurDir))
}

/// Create a new zone
///
/// This endpoint:
/// 1. Generates zone file from structured configuration
/// 2. Writes the zone file to disk
/// 3. Executes `rndc addzone` to add the zone to BIND9
#[utoipa::path(
    post,
    path = "/api/v1/zones",
    request_body = CreateZoneRequest,
    responses(
        (status = 201, description = "Zone created successfully", body = ZoneResponse),
        (status = 400, description = "Invalid request"),
        (status = 409, description = "Zone already exists"),
        (status = 500, description = "RNDC command failed"),
        (status = 500, description = "Internal server error")
    ),
    tag = "zones"
)]
pub async fn create_zone(
    State(state): State<AppState>,
    Json(request): Json<CreateZoneRequest>,
) -> Result<(StatusCode, Json<ZoneResponse>), ApiError> {
    info!("Creating zone: {}", request.zone_name);

    // Debug log the full request payload
    if let Ok(json_payload) = serde_json::to_string_pretty(&request) {
        debug!("POST /api/v1/zones payload: {}", json_payload);
    }

    // Validate zone name (strict DNS grammar; prevents path traversal into the
    // zone directory and command injection into rndc addzone).
    if let Err(e) = validate_zone_name(&request.zone_name) {
        metrics::record_zone_operation("create", false);
        return Err(e);
    }

    // Validate zone type
    if request.zone_type != ZONE_TYPE_PRIMARY && request.zone_type != ZONE_TYPE_SECONDARY {
        metrics::record_zone_operation("create", false);
        return Err(ApiError::InvalidRequest(format!(
            "Invalid zone type: {}. Must be '{}' or '{}'",
            request.zone_type, ZONE_TYPE_PRIMARY, ZONE_TYPE_SECONDARY
        )));
    }

    // Validate secondary zone requirements
    if request.zone_type == ZONE_TYPE_SECONDARY
        && request
            .zone_config
            .primaries
            .as_ref()
            .is_none_or(|p| p.is_empty())
    {
        metrics::record_zone_operation("create", false);
        return Err(ApiError::InvalidRequest(
            "Secondary zones require at least one primary server in 'primaries' field".to_string(),
        ));
    }

    // Validate optional RNDC identifiers up front, before any filesystem writes,
    // so an injection attempt never reaches the rndc addzone config literal.
    if let Some(key_name) = &request.update_key_name {
        if let Err(e) = validate_rndc_identifier("updateKeyName", key_name) {
            metrics::record_zone_operation("create", false);
            return Err(e);
        }
    }
    if let Some(dnssec_policy) = &request.zone_config.dnssec_policy {
        if let Err(e) = validate_rndc_identifier("dnssecPolicy", dnssec_policy) {
            metrics::record_zone_operation("create", false);
            return Err(e);
        }
    }

    if let Some(primaries) = &request.zone_config.primaries {
        if let Err(e) = validate_ip_port_list("primaries", primaries) {
            metrics::record_zone_operation("create", false);
            return Err(e);
        }
    }
    if let Some(also_notify) = &request.zone_config.also_notify {
        if let Err(e) = validate_ip_port_list("also-notify", also_notify) {
            metrics::record_zone_operation("create", false);
            return Err(e);
        }
    }
    if let Some(allow_transfer) = &request.zone_config.allow_transfer {
        if let Err(e) = validate_ip_list("allow-transfer", allow_transfer) {
            metrics::record_zone_operation("create", false);
            return Err(e);
        }
    }

    // Validate the zone-file content fields before rendering (C-2). Records
    // embedded in the create request never passed the per-request validation
    // applied by the add-record endpoint, so a control character in any field
    // could inject extra zone-file lines / directives ($INCLUDE, $GENERATE).
    if request.zone_type == ZONE_TYPE_PRIMARY {
        if let Err(e) = validate_zone_config_content(&request.zone_config) {
            metrics::record_zone_operation("create", false);
            return Err(e);
        }
    }

    // Generate zone file content from structured configuration (only for primary zones)
    let zone_content = if request.zone_type == ZONE_TYPE_PRIMARY {
        request.zone_config.to_zone_file()
    } else {
        String::new() // Secondary zones don't need zone files
    };

    // Only write zone file for primary zones
    let zone_file_name = format!("{}.zone", request.zone_name);
    let zone_file_path = PathBuf::from(&state.zone_dir).join(&zone_file_name);

    if request.zone_type == ZONE_TYPE_PRIMARY {
        info!(
            "Generated zone file content for {}: {} bytes",
            request.zone_name,
            zone_content.len()
        );

        // Clean up any existing journal file to prevent sync issues
        let journal_file_name = format!("{}.zone.jnl", request.zone_name);
        let journal_file_path = PathBuf::from(&state.zone_dir).join(&journal_file_name);
        if journal_file_path.exists() {
            if let Err(e) = tokio::fs::remove_file(&journal_file_path).await {
                error!(
                    "Failed to remove old journal file {}: {}",
                    journal_file_path.display(),
                    e
                );
            } else {
                info!("Removed old journal file: {}", journal_file_path.display());
            }
        }

        tokio::fs::write(&zone_file_path, &zone_content)
            .await
            .map_err(|e| {
                error!(
                    "Failed to write zone file {}: {}",
                    zone_file_path.display(),
                    e
                );
                metrics::record_zone_operation("create", false);
                ApiError::ZoneFileError(format!("Failed to write zone file: {}", e))
            })?;

        info!("Wrote zone file: {}", zone_file_path.display());
    }

    // Build zone configuration for rndc addzone
    let mut config_parts = vec![format!(r#"type {}"#, request.zone_type)];

    // Add file path for primary zones
    if request.zone_type == ZONE_TYPE_PRIMARY {
        let zone_file_full_path = format!("{}/{}", state.zone_dir, zone_file_name);
        config_parts.push(format!(r#"file "{}""#, zone_file_full_path));
    }

    // Add primaries for secondary zones
    if request.zone_type == ZONE_TYPE_SECONDARY {
        if let Some(primaries) = &request.zone_config.primaries {
            if !primaries.is_empty() {
                let primaries_list = primaries
                    .iter()
                    .map(|s| render_ip_port_entry(s))
                    .collect::<String>();
                config_parts.push(format!(r#"primaries {{ {} }}"#, primaries_list));
            }
        }
    }

    // Add allow-update if TSIG key is provided
    if let Some(key_name) = &request.update_key_name {
        config_parts.push(format!(r#"allow-update {{ key "{}"; }}"#, key_name));
    }

    // Add also-notify if secondary IPs are provided
    if let Some(also_notify) = &request.zone_config.also_notify {
        if !also_notify.is_empty() {
            let notify_list = also_notify
                .iter()
                .map(|s| render_ip_port_entry(s))
                .collect::<String>();
            config_parts.push(format!(r#"also-notify {{ {} }}"#, notify_list));
        }
    }

    // Add allow-transfer if secondary IPs are provided
    if let Some(allow_transfer) = &request.zone_config.allow_transfer {
        if !allow_transfer.is_empty() {
            let transfer_list = allow_transfer
                .iter()
                .map(|ip| format!("{}; ", ip))
                .collect::<String>();
            config_parts.push(format!(r#"allow-transfer {{ {} }}"#, transfer_list));
        }
    }

    // Add DNSSEC policy if specified (BIND9 9.16+)
    if let Some(dnssec_policy) = &request.zone_config.dnssec_policy {
        config_parts.push(format!(r#"dnssec-policy "{}""#, dnssec_policy));
    }

    // Add inline-signing if specified (required for DNSSEC with dynamic zones)
    if let Some(inline_signing) = request.zone_config.inline_signing {
        config_parts.push(format!(
            r#"inline-signing {}"#,
            if inline_signing { "yes" } else { "no" }
        ));
    }

    // Join all parts into final configuration
    let zone_config = format!("{{ {}; }};", config_parts.join("; "));

    // Execute rndc addzone
    let output = state
        .rndc
        .addzone(&request.zone_name, &zone_config)
        .await
        .map_err(|e| {
            error!("RNDC addzone failed for {}: {}", request.zone_name, e);
            metrics::record_zone_operation("create", false);

            // Check if zone already exists
            let error_msg = e.to_string();
            if error_msg.contains("already exists") {
                ApiError::ZoneAlreadyExists(request.zone_name.clone())
            } else {
                ApiError::RndcError(error_msg)
            }
        })?;

    info!("Zone {} created successfully", request.zone_name);
    metrics::record_zone_operation("create", true);

    Ok((
        StatusCode::CREATED,
        Json(ZoneResponse {
            success: true,
            message: format!("Zone {} created successfully", request.zone_name),
            details: Some(output),
        }),
    ))
}

/// Delete a zone
#[utoipa::path(
    delete,
    path = "/api/v1/zones/{name}",
    params(
        ("name" = String, Path, description = "Zone name to delete")
    ),
    responses(
        (status = 200, description = "Zone deleted successfully", body = ZoneResponse),
        (status = 500, description = "RNDC command failed")
    ),
    tag = "zones"
)]
pub async fn delete_zone(
    State(state): State<AppState>,
    Path(zone_name): Path<String>,
) -> Result<Json<ZoneResponse>, ApiError> {
    info!("Deleting zone: {}", zone_name);

    // Validate zone name before it reaches rndc delzone or any filesystem path
    // (prevents path traversal deleting arbitrary *.zone files).
    if let Err(e) = validate_zone_name(&zone_name) {
        metrics::record_zone_operation("delete", false);
        return Err(e);
    }

    // Execute rndc delzone
    let output = state.rndc.delzone(&zone_name).await.map_err(|e| {
        error!("RNDC delzone failed for {}: {}", zone_name, e);
        metrics::record_zone_operation("delete", false);
        ApiError::RndcError(e.to_string())
    })?;

    // Delete zone file
    let zone_file_name = format!("{}.zone", zone_name);
    let zone_file_path = PathBuf::from(&state.zone_dir).join(&zone_file_name);

    if zone_file_path.exists() {
        if let Err(e) = tokio::fs::remove_file(&zone_file_path).await {
            error!(
                "Failed to delete zone file {}: {}",
                zone_file_path.display(),
                e
            );
            // Don't fail the request if file deletion fails - zone is already removed from BIND9
        } else {
            info!("Deleted zone file: {}", zone_file_path.display());
        }
    }

    // Delete journal file if it exists
    let journal_file_name = format!("{}.zone.jnl", zone_name);
    let journal_file_path = PathBuf::from(&state.zone_dir).join(&journal_file_name);

    if journal_file_path.exists() {
        if let Err(e) = tokio::fs::remove_file(&journal_file_path).await {
            error!(
                "Failed to delete journal file {}: {}",
                journal_file_path.display(),
                e
            );
            // Don't fail the request if file deletion fails
        } else {
            info!("Deleted journal file: {}", journal_file_path.display());
        }
    }

    info!("Zone {} deleted successfully", zone_name);
    metrics::record_zone_operation("delete", true);

    Ok(Json(ZoneResponse {
        success: true,
        message: format!("Zone {} deleted successfully", zone_name),
        details: Some(output),
    }))
}

/// Reload a zone
#[utoipa::path(
    post,
    path = "/api/v1/zones/{name}/reload",
    params(
        ("name" = String, Path, description = "Zone name to reload")
    ),
    responses(
        (status = 200, description = "Zone reloaded successfully", body = ZoneResponse),
        (status = 500, description = "RNDC command failed")
    ),
    tag = "zones"
)]
pub async fn reload_zone(
    State(state): State<AppState>,
    Path(zone_name): Path<String>,
) -> Result<Json<ZoneResponse>, ApiError> {
    info!("Reloading zone: {}", zone_name);

    // Validate the caller-supplied zone name before it reaches rndc (defense
    // against rndc command / path injection via the {name} path parameter).
    if let Err(e) = validate_zone_name(&zone_name) {
        metrics::record_zone_operation("reload", false);
        return Err(e);
    }

    let output = state.rndc.reload(&zone_name).await.map_err(|e| {
        error!("RNDC reload failed for {}: {}", zone_name, e);
        metrics::record_zone_operation("reload", false);
        ApiError::RndcError(e.to_string())
    })?;

    info!("Zone {} reloaded successfully", zone_name);
    metrics::record_zone_operation("reload", true);

    Ok(Json(ZoneResponse {
        success: true,
        message: format!("Zone {} reloaded successfully", zone_name),
        details: Some(output),
    }))
}

/// Get zone status
///
/// Returns the raw `rndc zonestatus` text plus a typed `dnssec` block parsed
/// from `rndc dnssec -status` (ADR-0001): policy, signed flag, and per-key
/// role, states and rollover timing. The block is omitted (never an error)
/// when the signing state cannot be determined, so the endpoint stays cheap
/// and robust to poll.
#[utoipa::path(
    get,
    path = "/api/v1/zones/{name}/status",
    params(
        ("name" = String, Path, description = "Zone name")
    ),
    responses(
        (status = 200, description = "Zone status retrieved", body = ZoneStatusResponse),
        (status = 404, description = "Zone not found"),
        (status = 500, description = "RNDC command failed")
    ),
    tag = "zones"
)]
pub async fn zone_status(
    State(state): State<AppState>,
    Path(zone_name): Path<String>,
) -> Result<Json<ZoneStatusResponse>, ApiError> {
    info!("Getting status for zone: {}", zone_name);

    // Validate the caller-supplied zone name before it reaches rndc (defense
    // against rndc command / path injection via the {name} path parameter).
    validate_zone_name(&zone_name)?;

    let output = state.rndc.zonestatus(&zone_name).await.map_err(|e| {
        error!("RNDC zonestatus failed for {}: {}", zone_name, e);
        if e.to_string().contains("not found") {
            ApiError::ZoneNotFound(zone_name.clone())
        } else {
            ApiError::RndcError(e.to_string())
        }
    })?;

    // Best-effort DNSSEC block: a zone without a policy answers with the
    // literal "Zone does not have dnssec-policy" (parsed to signed: false),
    // and an rndc failure only drops the block.
    let dnssec = match state.rndc.dnssec_status(&zone_name).await {
        Ok(status_output) => Some(crate::dnssec::parse_dnssec_status(&status_output)),
        Err(e) => {
            warn!("RNDC dnssec -status failed for {}: {}", zone_name, e);
            None
        }
    };

    Ok(Json(ZoneStatusResponse {
        success: true,
        message: format!("Zone {} status retrieved", zone_name),
        details: Some(output),
        dnssec,
    }))
}

/// Get the DS records of a signed zone (ADR-0001)
///
/// Computes DS records (RFC 4509, SHA-256) in-process from the public
/// `K<zone>.+<alg>+<tag>.key` files in the configured key directory,
/// restricted to keys that `rndc dnssec -status` reports in a key-signing
/// role. Delegation material for the parent zone / registrar.
#[utoipa::path(
    get,
    path = "/api/v1/zones/{name}/ds",
    params(
        ("name" = String, Path, description = "Zone name")
    ),
    responses(
        (status = 200, description = "DS records computed", body = DsSetResponse),
        (status = 404, description = "Zone not found or not signed"),
        (status = 501, description = "Key directory not configured"),
        (status = 500, description = "RNDC command failed")
    ),
    tag = "zones"
)]
pub async fn get_zone_ds(
    State(state): State<AppState>,
    Path(zone_name): Path<String>,
) -> Result<Json<DsSetResponse>, ApiError> {
    info!("Computing DS records for zone: {}", zone_name);

    validate_zone_name(&zone_name)?;

    let Some(key_dir) = state.key_dir.as_deref() else {
        return Err(ApiError::NotConfigured(
            "DS retrieval requires the DNSSEC key directory to be mounted read-only \
             and configured via BIND_KEY_DIR"
                .to_string(),
        ));
    };

    // Defense-in-depth (mirrors the zone_dir sinks, bug-076): the directory
    // was canonicalized at startup, so re-check the invariant at the sink.
    if !is_normalized_zone_dir(key_dir) || key_dir.contains("..") {
        error!("key directory {:?} failed the normalization guard", key_dir);
        return Err(ApiError::InternalError(
            "key directory misconfigured".to_string(),
        ));
    }

    let status_output = state.rndc.dnssec_status(&zone_name).await.map_err(|e| {
        error!("RNDC dnssec -status failed for {}: {}", zone_name, e);
        if e.to_string().contains("not found") {
            ApiError::ZoneNotFound(zone_name.clone())
        } else {
            ApiError::RndcError(e.to_string())
        }
    })?;
    let status = crate::dnssec::parse_dnssec_status(&status_output);

    if status.policy.is_none() {
        return Err(ApiError::DsNotAvailable(format!(
            "zone {} has no dnssec-policy; there is nothing to delegate",
            zone_name
        )));
    }

    let ksk_tags = status.ksk_tags();
    if ksk_tags.is_empty() {
        return Err(ApiError::DsNotAvailable(format!(
            "zone {} reports no key-signing keys",
            zone_name
        )));
    }

    // Public key files only: `K<zone>.+<alg>+<tag>.key`. Filenames come from
    // the directory listing, never from the request; `.private`/`.state`
    // files are never read (ADR-0001).
    let file_prefix = format!("K{}.+", zone_name);
    let mut ds_records: Vec<DsRecordView> = Vec::new();
    let mut entries = tokio::fs::read_dir(key_dir).await.map_err(|e| {
        error!("failed to read key directory {:?}: {}", key_dir, e);
        ApiError::InternalError("key directory unreadable".to_string())
    })?;
    while let Some(entry) = entries.next_entry().await.map_err(|e| {
        error!("failed to iterate key directory {:?}: {}", key_dir, e);
        ApiError::InternalError("key directory unreadable".to_string())
    })? {
        let file_name = entry.file_name();
        let Some(name) = file_name.to_str() else {
            continue;
        };
        if !name.starts_with(&file_prefix) || !name.ends_with(".key") {
            continue;
        }
        let contents = match tokio::fs::read_to_string(entry.path()).await {
            Ok(contents) => contents,
            Err(e) => {
                warn!("skipping unreadable key file {:?}: {}", name, e);
                continue;
            }
        };
        let dnskey = match crate::dnssec::parse_key_file(&contents) {
            Ok(dnskey) => dnskey,
            Err(e) => {
                warn!("skipping unparseable key file {:?}: {}", name, e);
                continue;
            }
        };
        if !ksk_tags.contains(&dnskey.key_tag()) {
            continue;
        }
        let ds = dnskey.ds_record().map_err(|e| {
            error!("DS computation failed for key file {:?}: {}", name, e);
            ApiError::InternalError("DS computation failed".to_string())
        })?;
        let rr = format!("{} IN DS {}", dnskey.owner, ds.rdata());
        ds_records.push(DsRecordView {
            key_tag: ds.key_tag,
            algorithm: ds.algorithm,
            digest_type: ds.digest_type,
            digest: ds.digest,
            rr,
        });
    }

    if ds_records.is_empty() {
        return Err(ApiError::DsNotAvailable(format!(
            "no public key files for the key-signing keys of zone {} were found; \
             is the key directory mounted?",
            zone_name
        )));
    }
    ds_records.sort_by_key(|r| r.key_tag);

    Ok(Json(DsSetResponse {
        success: true,
        zone: zone_name,
        ds_records,
    }))
}

/// Report a DS change at the parent zone (ADR-0001)
///
/// Forwards `rndc dnssec -checkds` so key rollovers and the graceful
/// `insecure` transition can complete without host access when no parental
/// agents are configured.
#[utoipa::path(
    post,
    path = "/api/v1/zones/{name}/dnssec/checkds",
    request_body = CheckdsRequest,
    params(
        ("name" = String, Path, description = "Zone name")
    ),
    responses(
        (status = 200, description = "DS state recorded", body = ZoneResponse),
        (status = 404, description = "Zone not found"),
        (status = 500, description = "RNDC command failed")
    ),
    tag = "zones"
)]
pub async fn checkds_zone(
    State(state): State<AppState>,
    Path(zone_name): Path<String>,
    Json(request): Json<CheckdsRequest>,
) -> Result<Json<ZoneResponse>, ApiError> {
    info!(
        "Recording DS {} for zone {} key tag {}",
        request.ds.as_str(),
        zone_name,
        request.key_tag
    );

    validate_zone_name(&zone_name)?;

    let output = state
        .rndc
        .dnssec_checkds(&zone_name, request.key_tag, request.ds)
        .await
        .map_err(|e| {
            error!("RNDC dnssec -checkds failed for {}: {}", zone_name, e);
            if e.to_string().contains("not found") {
                ApiError::ZoneNotFound(zone_name.clone())
            } else {
                ApiError::RndcError(e.to_string())
            }
        })?;

    Ok(Json(ZoneResponse {
        success: true,
        message: format!(
            "DS {} recorded for zone {} key tag {}",
            request.ds.as_str(),
            zone_name,
            request.key_tag
        ),
        details: Some(output),
    }))
}

/// Freeze a zone (disable dynamic updates)
#[utoipa::path(
    post,
    path = "/api/v1/zones/{name}/freeze",
    params(
        ("name" = String, Path, description = "Zone name to freeze")
    ),
    responses(
        (status = 200, description = "Zone frozen successfully", body = ZoneResponse),
        (status = 500, description = "RNDC command failed")
    ),
    tag = "zones"
)]
pub async fn freeze_zone(
    State(state): State<AppState>,
    Path(zone_name): Path<String>,
) -> Result<Json<ZoneResponse>, ApiError> {
    info!("Freezing zone: {}", zone_name);

    // Validate the caller-supplied zone name before it reaches rndc (defense
    // against rndc command / path injection via the {name} path parameter).
    if let Err(e) = validate_zone_name(&zone_name) {
        metrics::record_zone_operation("freeze", false);
        return Err(e);
    }

    let output = state.rndc.freeze(&zone_name).await.map_err(|e| {
        error!("RNDC freeze failed for {}: {}", zone_name, e);
        metrics::record_zone_operation("freeze", false);
        ApiError::RndcError(e.to_string())
    })?;

    info!("Zone {} frozen successfully", zone_name);
    metrics::record_zone_operation("freeze", true);

    Ok(Json(ZoneResponse {
        success: true,
        message: format!("Zone {} frozen successfully", zone_name),
        details: Some(output),
    }))
}

/// Thaw a zone (enable dynamic updates)
#[utoipa::path(
    post,
    path = "/api/v1/zones/{name}/thaw",
    params(
        ("name" = String, Path, description = "Zone name to thaw")
    ),
    responses(
        (status = 200, description = "Zone thawed successfully", body = ZoneResponse),
        (status = 500, description = "RNDC command failed")
    ),
    tag = "zones"
)]
pub async fn thaw_zone(
    State(state): State<AppState>,
    Path(zone_name): Path<String>,
) -> Result<Json<ZoneResponse>, ApiError> {
    info!("Thawing zone: {}", zone_name);

    // Validate the caller-supplied zone name before it reaches rndc (defense
    // against rndc command / path injection via the {name} path parameter).
    if let Err(e) = validate_zone_name(&zone_name) {
        metrics::record_zone_operation("thaw", false);
        return Err(e);
    }

    let output = state.rndc.thaw(&zone_name).await.map_err(|e| {
        error!("RNDC thaw failed for {}: {}", zone_name, e);
        metrics::record_zone_operation("thaw", false);
        ApiError::RndcError(e.to_string())
    })?;

    info!("Zone {} thawed successfully", zone_name);
    metrics::record_zone_operation("thaw", true);

    Ok(Json(ZoneResponse {
        success: true,
        message: format!("Zone {} thawed successfully", zone_name),
        details: Some(output),
    }))
}

/// Notify secondaries about zone changes
#[utoipa::path(
    post,
    path = "/api/v1/zones/{name}/notify",
    params(
        ("name" = String, Path, description = "Zone name")
    ),
    responses(
        (status = 200, description = "Notify sent successfully", body = ZoneResponse),
        (status = 500, description = "RNDC command failed")
    ),
    tag = "zones"
)]
pub async fn notify_zone(
    State(state): State<AppState>,
    Path(zone_name): Path<String>,
) -> Result<Json<ZoneResponse>, ApiError> {
    info!("Notifying secondaries for zone: {}", zone_name);

    // Validate the caller-supplied zone name before it reaches rndc (defense
    // against rndc command / path injection via the {name} path parameter).
    if let Err(e) = validate_zone_name(&zone_name) {
        metrics::record_zone_operation("notify", false);
        return Err(e);
    }

    let output = state.rndc.notify(&zone_name).await.map_err(|e| {
        error!("RNDC notify failed for {}: {}", zone_name, e);
        metrics::record_zone_operation("notify", false);
        ApiError::RndcError(e.to_string())
    })?;

    info!("Zone {} notify sent successfully", zone_name);
    metrics::record_zone_operation("notify", true);

    Ok(Json(ZoneResponse {
        success: true,
        message: format!("Notify sent for zone {}", zone_name),
        details: Some(output),
    }))
}

/// Force a zone retransfer from primary
#[utoipa::path(
    post,
    path = "/api/v1/zones/{name}/retransfer",
    params(
        ("name" = String, Path, description = "Zone name to retransfer")
    ),
    responses(
        (status = 200, description = "Zone retransfer initiated", body = ZoneResponse),
        (status = 500, description = "RNDC command failed")
    ),
    tag = "zones"
)]
pub async fn retransfer_zone(
    State(state): State<AppState>,
    Path(zone_name): Path<String>,
) -> Result<Json<ZoneResponse>, ApiError> {
    info!("Retransferring zone: {}", zone_name);

    // Validate the caller-supplied zone name before it reaches rndc (defense
    // against rndc command / path injection via the {name} path parameter).
    if let Err(e) = validate_zone_name(&zone_name) {
        metrics::record_zone_operation("retransfer", false);
        return Err(e);
    }

    let output = state.rndc.retransfer(&zone_name).await.map_err(|e| {
        error!("RNDC retransfer failed for {}: {}", zone_name, e);
        metrics::record_zone_operation("retransfer", false);
        ApiError::RndcError(e.to_string())
    })?;

    info!("Zone {} retransfer initiated successfully", zone_name);
    metrics::record_zone_operation("retransfer", true);

    Ok(Json(ZoneResponse {
        success: true,
        message: format!("Retransfer initiated for zone {}", zone_name),
        details: Some(output),
    }))
}

/// Get server status
#[utoipa::path(
    get,
    path = "/api/v1/server/status",
    responses(
        (status = 200, description = "Server status retrieved", body = ServerStatusResponse),
        (status = 500, description = "RNDC command failed")
    ),
    tag = "server"
)]
pub async fn server_status(
    State(state): State<AppState>,
) -> Result<Json<ServerStatusResponse>, ApiError> {
    info!("Getting server status");

    let output = state.rndc.status().await.map_err(|e| {
        error!("RNDC status failed: {}", e);
        ApiError::RndcError(e.to_string())
    })?;

    Ok(Json(ServerStatusResponse { status: output }))
}

/// List all zones
#[utoipa::path(
    get,
    path = "/api/v1/zones",
    responses(
        (status = 200, description = "List of zones", body = ZoneListResponse),
        (status = 500, description = "Failed to read zone directory")
    ),
    tag = "zones"
)]
pub async fn list_zones(State(state): State<AppState>) -> Result<Json<ZoneListResponse>, ApiError> {
    info!("Listing all zones");

    // Defense-in-depth barrier (CodeQL `rust/path-injection`): `zone_dir` is
    // canonicalized once at startup (`resolve_zone_dir`), but it reaches this
    // handler through the axum `State` extractor, which static analysis models
    // as untrusted. Re-assert the startup invariant immediately before the
    // `read_dir` sink — the inline `contains("..")` is the guard shape CodeQL
    // recognizes as a path-traversal sanitizer.
    if !is_normalized_zone_dir(&state.zone_dir) || state.zone_dir.contains("..") {
        error!(
            "zone directory {:?} is not a normalized absolute path; refusing to list it",
            state.zone_dir
        );
        return Err(ApiError::InternalError(
            "zone directory is not a normalized absolute path".to_string(),
        ));
    }

    // Get zone files from directory
    let mut zones = Vec::new();
    let mut entries = tokio::fs::read_dir(&state.zone_dir).await.map_err(|e| {
        error!("Failed to read zone directory: {}", e);
        ApiError::InternalError(format!("Failed to read zone directory: {}", e))
    })?;

    while let Ok(Some(entry)) = entries.next_entry().await {
        if let Ok(file_name) = entry.file_name().into_string() {
            if file_name.ends_with(".zone") {
                // Extract zone name from filename (remove .zone extension)
                if let Some(zone_name) = file_name.strip_suffix(".zone") {
                    zones.push(zone_name.to_string());
                }
            }
        }
    }

    zones.sort();
    let count = zones.len();

    info!("Found {} zones", count);
    metrics::update_zones_count(count as i64);

    Ok(Json(ZoneListResponse { zones, count }))
}

/// Get a specific zone
#[utoipa::path(
    get,
    path = "/api/v1/zones/{name}",
    params(
        ("name" = String, Path, description = "Zone name")
    ),
    responses(
        (status = 200, description = "Zone information", body = ZoneInfo),
        (status = 404, description = "Zone not found"),
        (status = 500, description = "RNDC command failed")
    ),
    tag = "zones"
)]
pub async fn get_zone(
    State(state): State<AppState>,
    Path(zone_name): Path<String>,
) -> Result<Json<ZoneInfo>, ApiError> {
    info!("Getting zone: {}", zone_name);

    // Validate the caller-supplied zone name before it is joined into a
    // filesystem path (prevents path traversal: a name like "../../etc/passwd"
    // would otherwise turn into an arbitrary-file existence oracle) or forwarded
    // to rndc.
    validate_zone_name(&zone_name)?;

    // Check if zone file exists
    let zone_file_name = format!("{}.zone", zone_name);
    let zone_file_path = PathBuf::from(&state.zone_dir).join(&zone_file_name);

    if !zone_file_path.exists() {
        return Err(ApiError::ZoneNotFound(zone_name.clone()));
    }

    // Get zone status from BIND9
    let status_output = state.rndc.zonestatus(&zone_name).await.map_err(|e| {
        error!("RNDC zonestatus failed for {}: {}", zone_name, e);
        if e.to_string().contains("not found") {
            ApiError::ZoneNotFound(zone_name.clone())
        } else {
            ApiError::RndcError(e.to_string())
        }
    })?;

    // Parse zone type and serial from status output
    let mut zone_type = "unknown".to_string();
    let mut serial = None;

    for line in status_output.lines() {
        if let Some(type_str) = line.strip_prefix("type:").or_else(|| {
            line.contains("type:")
                .then(|| line.split("type:").nth(1))
                .flatten()
        }) {
            zone_type = type_str.trim().to_string();
        }

        if let Some(serial_str) = line.strip_prefix("serial:").or_else(|| {
            line.contains("serial:")
                .then(|| line.split("serial:").nth(1))
                .flatten()
        }) {
            if let Ok(s) = serial_str.trim().parse::<u32>() {
                serial = Some(s);
            }
        }
    }

    Ok(Json(ZoneInfo {
        name: zone_name,
        zone_type,
        serial,
        file_path: Some(zone_file_path.display().to_string()),
    }))
}

/// The built-in BIND9 policy that gracefully unsigns a zone (ADR-0001).
pub const DNSSEC_POLICY_INSECURE: &str = "insecure";

/// Sentinel policy name requesting removal of the `dnssec-policy` directive.
pub const DNSSEC_POLICY_NONE: &str = "none";

/// Applies the DNSSEC fields of a [`ModifyZoneRequest`] to a parsed zone
/// configuration, enforcing the ADR-0001 transition rules.
///
/// Merge semantics: a `None` request field means "leave as is", never
/// "remove" — verified on BIND 9.18.50, re-issuing a zone config without its
/// `dnssec-policy` abruptly unsigns the zone at the next reconfig.
///
/// * a policy name enables signing or switches policies (BIND manages the
///   rollover); `inline-signing yes` is set implicitly when the zone has no
///   dynamic-update configuration, since BIND requires one or the other
/// * [`DNSSEC_POLICY_INSECURE`] is the only path from signed to unsigned
/// * [`DNSSEC_POLICY_NONE`] removes the directive, and is refused while the
///   zone still serves DNSKEY records
///
/// # Arguments
/// * `zone_config` - Parsed `showzone` configuration to mutate
/// * `requested_policy` - `dnssecPolicy` field of the PATCH body
/// * `requested_inline_signing` - `inlineSigning` field of the PATCH body
/// * `zone_serves_dnskey` - Whether `rndc dnssec -status` reports a key whose
///   DNSKEY state is still `rumoured` or `omnipresent`
///
/// # Returns
/// `true` when the request changed DNSSEC configuration (the caller must run
/// `rndc reconfig` after `rndc modzone` to activate it, per ADR-0001).
///
/// # Errors
/// Returns [`ApiError::UnsafeDnssecTransition`] when the request would
/// remove the policy from a zone that still serves DNSKEY records.
pub fn apply_dnssec_request(
    zone_config: &mut crate::rndc_types::ZoneConfig,
    requested_policy: Option<&str>,
    requested_inline_signing: Option<bool>,
    zone_serves_dnskey: bool,
) -> Result<bool, ApiError> {
    let mut changed = false;

    if let Some(policy) = requested_policy {
        if policy.eq_ignore_ascii_case(DNSSEC_POLICY_NONE) {
            if zone_serves_dnskey {
                return Err(ApiError::UnsafeDnssecTransition(format!(
                    "zone still serves DNSKEY records; removing dnssec-policy now would \
                     take the zone dark for validating resolvers. Transition to the \
                     built-in \"{}\" policy first, confirm DS withdrawal at the parent \
                     (POST .../dnssec/checkds), and remove the policy once the zone no \
                     longer serves DNSKEY/RRSIG records",
                    DNSSEC_POLICY_INSECURE
                )));
            }
            zone_config.dnssec_policy = None;
        } else {
            let zone_is_dynamic = zone_config.allow_update.is_some()
                || zone_config.allow_update_raw.is_some()
                || zone_config.update_policy.is_some();
            zone_config.dnssec_policy = Some(policy.to_string());
            // BIND requires dynamic DNS or inline-signing with dnssec-policy.
            if requested_inline_signing.is_none()
                && zone_config.inline_signing.is_none()
                && !zone_is_dynamic
            {
                zone_config.inline_signing = Some(true);
            }
        }
        changed = true;
    }

    if let Some(inline_signing) = requested_inline_signing {
        zone_config.inline_signing = Some(inline_signing);
        changed = true;
    }

    Ok(changed)
}

/// Modify a zone configuration
///
/// This endpoint allows updating zone configuration parameters such as
/// also-notify and allow-transfer IP addresses without recreating the zone.
/// It uses the `rndc modzone` command to dynamically update the zone
/// configuration. DNSSEC lifecycle transitions (ADR-0001) additionally run
/// `rndc reconfig`, because modzone alone stores a `dnssec-policy` without
/// applying it to the running zone.
#[utoipa::path(
    patch,
    path = "/api/v1/zones/{name}",
    request_body = ModifyZoneRequest,
    params(
        ("name" = String, Path, description = "Zone name to modify")
    ),
    responses(
        (status = 200, description = "Zone modified successfully", body = ZoneResponse),
        (status = 400, description = "Invalid request"),
        (status = 404, description = "Zone not found"),
        (status = 500, description = "RNDC command failed"),
        (status = 500, description = "Internal server error")
    ),
    tag = "zones"
)]
pub async fn modify_zone(
    State(state): State<AppState>,
    Path(zone_name): Path<String>,
    Json(request): Json<ModifyZoneRequest>,
) -> Result<Json<ZoneResponse>, ApiError> {
    info!("Modifying zone: {}", zone_name);

    // Validate the caller-supplied zone name before it is joined into a
    // filesystem path or forwarded to rndc modzone/zonestatus.
    if let Err(e) = validate_zone_name(&zone_name) {
        metrics::record_zone_operation("modify", false);
        return Err(e);
    }

    // Debug log the full request payload
    if let Ok(json_payload) = serde_json::to_string_pretty(&request) {
        debug!(
            "PATCH /api/v1/zones/{} payload: {}",
            zone_name, json_payload
        );
    }

    // Validate that at least one field is being updated
    if request.also_notify.is_none()
        && request.allow_transfer.is_none()
        && request.allow_update.is_none()
        && request.dnssec_policy.is_none()
        && request.inline_signing.is_none()
    {
        metrics::record_zone_operation("modify", false);
        return Err(ApiError::InvalidRequest(
            "At least one field (alsoNotify, allowTransfer, allowUpdate, dnssecPolicy, \
             or inlineSigning) must be provided"
                .to_string(),
        ));
    }

    // Validate the policy name before it is rendered into an rndc config
    // literal (same guard as the create path).
    if let Some(dnssec_policy) = &request.dnssec_policy {
        if let Err(e) = validate_rndc_identifier("dnssecPolicy", dnssec_policy) {
            metrics::record_zone_operation("modify", false);
            return Err(e);
        }
    }

    // Check if zone exists by checking for zone file or querying status
    let zone_file_name = format!("{}.zone", zone_name);
    let zone_file_path = PathBuf::from(&state.zone_dir).join(&zone_file_name);

    // For secondary zones, there may not be a zone file, so we should check status instead
    let zone_exists = if zone_file_path.exists() {
        true
    } else {
        // Try to get zone status to see if it exists
        match state.rndc.zonestatus(&zone_name).await {
            Ok(_) => true,
            Err(e) => {
                if e.to_string().contains("not found") {
                    false
                } else {
                    // Some other error, but zone might exist
                    true
                }
            }
        }
    };

    if !zone_exists {
        metrics::record_zone_operation("modify", false);
        return Err(ApiError::ZoneNotFound(zone_name.clone()));
    }

    // Get current zone configuration from BIND9
    let showzone_output = state.rndc.showzone(&zone_name).await.map_err(|e| {
        error!("Failed to get zone configuration for {}: {}", zone_name, e);
        if e.to_string().contains("not found") {
            ApiError::ZoneNotFound(zone_name.clone())
        } else {
            ApiError::RndcError(e.to_string())
        }
    })?;

    // Parse the zone configuration
    let mut zone_config = crate::rndc_parser::parse_showzone(&showzone_output).map_err(|e| {
        error!(
            "Failed to parse zone configuration for {}: {}",
            zone_name, e
        );
        ApiError::RndcError(format!("Failed to parse zone configuration: {}", e))
    })?;

    info!(
        "Zone {} has type: {}",
        zone_name,
        zone_config.zone_type.as_str()
    );

    // Update the configuration with new values from the request
    if let Some(also_notify) = &request.also_notify {
        // Convert string IPs to IpAddr
        let ip_addrs: Result<Vec<std::net::IpAddr>, _> =
            also_notify.iter().map(|s| s.parse()).collect();

        match ip_addrs {
            Ok(addrs) => {
                zone_config.also_notify = if addrs.is_empty() { None } else { Some(addrs) };
            }
            Err(e) => {
                metrics::record_zone_operation("modify", false);
                return Err(ApiError::InvalidRequest(format!(
                    "Invalid IP address in also-notify: {}",
                    e
                )));
            }
        }
    }

    if let Some(allow_transfer) = &request.allow_transfer {
        // Convert string IPs to IpAddr
        let ip_addrs: Result<Vec<std::net::IpAddr>, _> =
            allow_transfer.iter().map(|s| s.parse()).collect();

        match ip_addrs {
            Ok(addrs) => {
                zone_config.allow_transfer = if addrs.is_empty() { None } else { Some(addrs) };
            }
            Err(e) => {
                metrics::record_zone_operation("modify", false);
                return Err(ApiError::InvalidRequest(format!(
                    "Invalid IP address in allow-transfer: {}",
                    e
                )));
            }
        }
    }

    if let Some(allow_update) = &request.allow_update {
        // Convert string IPs to IpAddr
        let ip_addrs: Result<Vec<std::net::IpAddr>, _> =
            allow_update.iter().map(|s| s.parse()).collect();

        match ip_addrs {
            Ok(addrs) => {
                zone_config.allow_update = if addrs.is_empty() { None } else { Some(addrs) };
                // Clear the raw directive since we're setting explicit IPs
                zone_config.allow_update_raw = None;
            }
            Err(e) => {
                metrics::record_zone_operation("modify", false);
                return Err(ApiError::InvalidRequest(format!(
                    "Invalid IP address in allow-update: {}",
                    e
                )));
            }
        }
    }

    // DNSSEC transitions (ADR-0001). Removing the policy is only permitted
    // once the zone no longer serves DNSKEY records, so fetch the running
    // signing state for exactly that request.
    let removal_requested = request
        .dnssec_policy
        .as_deref()
        .is_some_and(|p| p.eq_ignore_ascii_case(DNSSEC_POLICY_NONE));
    let zone_serves_dnskey = if removal_requested {
        let status_output = state.rndc.dnssec_status(&zone_name).await.map_err(|e| {
            error!("RNDC dnssec -status failed for {}: {}", zone_name, e);
            metrics::record_zone_operation("modify", false);
            ApiError::RndcError(e.to_string())
        })?;
        crate::dnssec::parse_dnssec_status(&status_output).signed
    } else {
        false
    };

    let dnssec_changed = apply_dnssec_request(
        &mut zone_config,
        request.dnssec_policy.as_deref(),
        request.inline_signing,
        zone_serves_dnskey,
    )
    .inspect_err(|_| {
        metrics::record_zone_operation("modify", false);
    })?;

    // Serialize the updated configuration back to RNDC format
    let rndc_config_block = zone_config.to_rndc_block();

    info!(
        "Modifying zone {} with config: {}",
        zone_name, rndc_config_block
    );

    // Execute rndc modzone
    let output = state
        .rndc
        .modzone(&zone_name, &rndc_config_block)
        .await
        .map_err(|e| {
            error!("RNDC modzone failed for {}: {}", zone_name, e);
            metrics::record_zone_operation("modify", false);
            ApiError::RndcError(e.to_string())
        })?;

    // A stored dnssec-policy is not applied by modzone alone (verified on
    // BIND 9.18.50, ADR-0001): reconfig activates it. If reconfig fails the
    // config IS persisted — named applies it on its next reconfig/restart —
    // so report that state honestly instead of a clean failure.
    if dnssec_changed {
        if let Err(e) = state.rndc.reconfig().await {
            error!(
                "RNDC reconfig failed after modzone for {}: {}",
                zone_name, e
            );
            metrics::record_zone_operation("modify", true);
            return Ok(Json(ZoneResponse {
                success: true,
                message: format!(
                    "Zone {} configuration stored, but reconfig failed: the DNSSEC \
                     change is persisted and will activate on the next successful \
                     reconfig or server restart",
                    zone_name
                ),
                details: Some(output),
            }));
        }
        info!(
            "Zone {} DNSSEC configuration activated via reconfig",
            zone_name
        );
    }

    info!("Zone {} modified successfully", zone_name);
    metrics::record_zone_operation("modify", true);

    Ok(Json(ZoneResponse {
        success: true,
        message: format!("Zone {} modified successfully", zone_name),
        details: Some(output),
    }))
}