fix(web): preserve lifecycle update invariants

Author: wiresock

amneziawg-install.sh

@@ -1688,25 +1688,81 @@ function resolveWebPanelEnvFile() {
 	printf '%s\n' "${env_file}"
 }
 
-# Resolve the web panel's active config directory without sourcing its env file.
-function resolveWebPanelConfigDir() {
-	local env_file configured_dir
+# Read one web-panel setting without evaluating shell syntax. EnvironmentFile
+# values override inline Environment= values, matching systemd semantics.
+function readWebPanelSetting() {
+	local setting_name="$1"
+	local env_file inline_assignment value="" use_env_file=1
 	env_file="$(resolveWebPanelEnvFile)" || return 1
+	if [[ -f "${WEB_PANEL_SYSTEMD_UNIT}" ]] && \
+		! grep -q '^[[:space:]]*EnvironmentFile=' "${WEB_PANEL_SYSTEMD_UNIT}" 2>/dev/null; then
+		use_env_file=0
+	fi
 
-	if [[ -f "${env_file}" ]]; then
-		configured_dir="$(sed -n 's/^AWG_CONFIG_DIR=//p' "${env_file}" 2>/dev/null | tail -n 1)"
-		configured_dir="${configured_dir#\"}"
-		configured_dir="${configured_dir%\"}"
-		configured_dir="${configured_dir#\'}"
-		configured_dir="${configured_dir%\'}"
-		if [[ -n "${configured_dir}" ]]; then
-			if [[ "${configured_dir}" != /* || "${configured_dir}" =~ [[:space:][:cntrl:]] ]]; then
-				echo "ERROR: refusing unsafe AWG_CONFIG_DIR '${configured_dir}'" >&2
+	if [[ "${use_env_file}" -eq 1 && -f "${env_file}" ]] && \
+		grep -q "^[[:space:]]*${setting_name}=" "${env_file}" 2>/dev/null; then
+		value="$(sed -n "s/^[[:space:]]*${setting_name}=//p" "${env_file}" 2>/dev/null | tail -n 1)"
+	else
+		while IFS= read -r inline_assignment; do
+			if [[ "${inline_assignment}" != *"${setting_name}="* ]]; then
+				continue
+			fi
+			inline_assignment="${inline_assignment#\"}"
+			inline_assignment="${inline_assignment%\"}"
+			inline_assignment="${inline_assignment#\'}"
+			inline_assignment="${inline_assignment%\'}"
+			if [[ "${inline_assignment}" != "${setting_name}="* ]]; then
+				echo "ERROR: unsupported inline ${setting_name} assignment in '${WEB_PANEL_SYSTEMD_UNIT}'" >&2
 				return 1
 			fi
-			printf '%s\n' "${configured_dir%/}"
-			return 0
+			value="${inline_assignment#*=}"
+		done < <(sed -n 's/^[[:space:]]*Environment=//p' "${WEB_PANEL_SYSTEMD_UNIT}" 2>/dev/null)
+	fi
+
+	value="${value#\"}"
+	value="${value%\"}"
+	value="${value#\'}"
+	value="${value%\'}"
+	printf '%s\n' "${value}"
+}
+
+# Resolve the service working directory used for relative database paths.
+function resolveWebPanelWorkingDirectory() {
+	local working_dir=""
+	if [[ -f "${WEB_PANEL_SYSTEMD_UNIT}" ]]; then
+		working_dir="$(sed -n 's/^[[:space:]]*WorkingDirectory=//p' "${WEB_PANEL_SYSTEMD_UNIT}" 2>/dev/null | tail -n 1)"
+		working_dir="${working_dir#-}"
+		working_dir="${working_dir#\"}"
+		working_dir="${working_dir%\"}"
+		working_dir="${working_dir#\'}"
+		working_dir="${working_dir%\'}"
+		working_dir="${working_dir:-/}"
+	else
+		working_dir="${WEB_PANEL_DATA_DIR}"
+	fi
+
+	if [[ "${working_dir}" != /* || "${working_dir}" =~ [[:space:][:cntrl:]] ]]; then
+		echo "ERROR: refusing unsafe web panel WorkingDirectory '${working_dir}'" >&2
+		return 1
+	fi
+	if [[ "${working_dir}" != "/" ]]; then
+		working_dir="${working_dir%/}"
+	fi
+	printf '%s\n' "${working_dir}"
+}
+
+# Resolve the web panel's active config directory without sourcing its env file.
+function resolveWebPanelConfigDir() {
+	local configured_dir
+	configured_dir="$(readWebPanelSetting AWG_CONFIG_DIR)" || return 1
+
+	if [[ -n "${configured_dir}" ]]; then
+		if [[ "${configured_dir}" != /* || "${configured_dir}" =~ [[:space:][:cntrl:]] ]]; then
+			echo "ERROR: refusing unsafe AWG_CONFIG_DIR '${configured_dir}'" >&2
+			return 1
 		fi
+		printf '%s\n' "${configured_dir%/}"
+		return 0
 	fi
 
 	printf '%s\n' "${WEB_PANEL_CONFIG_DIR%/}"
@@ -1717,28 +1773,42 @@ function resolveWebPanelConfigDir() {
 # client has been created yet. Standalone installer use (no panel env file)
 # retains the config-directory fallback.
 function resolveClientLifecycleLockDir() {
-	local env_file database_path
+	local env_file database_path working_dir database_parent
 	env_file="$(resolveWebPanelEnvFile)" || return 1
 
-	if [[ -f "${env_file}" ]]; then
-		database_path="$(sed -n 's/^AWG_WEB_DB=//p' "${env_file}" 2>/dev/null | tail -n 1)"
-		database_path="${database_path#\"}"
-		database_path="${database_path%\"}"
-		database_path="${database_path#\'}"
-		database_path="${database_path%\'}"
-		if [[ -n "${database_path}" ]]; then
-			if [[ "${database_path}" != /* || "${database_path}" =~ [[:space:][:cntrl:]] ]]; then
-				echo "ERROR: refusing unsafe AWG_WEB_DB '${database_path}'" >&2
-				return 1
-			fi
-			dirname -- "${database_path}"
-			return 0
-		fi
-		printf '%s\n' "${WEB_PANEL_DATA_DIR%/}"
-		return 0
+	if [[ ! -f "${env_file}" && ! -e "${WEB_PANEL_SYSTEMD_UNIT}" ]]; then
+		resolveWebPanelConfigDir
+		return
 	fi
 
-	resolveWebPanelConfigDir
+	database_path="$(readWebPanelSetting AWG_WEB_DB)" || return 1
+	database_path="${database_path:-awg-web.db}"
+	if [[ "${database_path}" =~ [[:space:][:cntrl:]] ]]; then
+		echo "ERROR: refusing unsafe AWG_WEB_DB '${database_path}'" >&2
+		return 1
+	fi
+	case "${database_path}" in
+		sqlite://*) database_path="${database_path#sqlite://}" ;;
+		sqlite:*) database_path="${database_path#sqlite:}" ;;
+	esac
+	database_path="${database_path%%\?*}"
+	working_dir="$(resolveWebPanelWorkingDirectory)" || return 1
+
+	if [[ -z "${database_path}" || "${database_path}" == ":memory:" ]]; then
+		printf '%s\n' "${working_dir}"
+		return 0
+	fi
+	database_parent="$(dirname -- "${database_path}")"
+	if [[ "${database_path}" == /* ]]; then
+		if [[ "${database_parent}" != "/" ]]; then
+			database_parent="${database_parent%/}"
+		fi
+		printf '%s\n' "${database_parent}"
+	elif [[ "${database_parent}" == "." ]]; then
+		printf '%s\n' "${working_dir}"
+	else
+		printf '%s/%s\n' "${working_dir%/}" "${database_parent}"
+	fi
 }
 
 # Copy a client config file to the web panel config directory so the panel

amneziawg-web/docs/ARCHITECTURE.md

@@ -77,12 +77,14 @@ the same lock, and atomically replaces the config.  Arbitrary config
 content, raw file reads, arbitrary `syncconf` stdin, and unknown operations are
 rejected rather than forwarded.
 
-The web panel holds an advisory lock on the open client-config directory across
-the complete managed-client lifecycle, including database persistence and
-client-config cleanup. Supported installer `--add-client` and `--remove-client`
-operations lock the same directory descriptor, preventing an out-of-band
-same-name replacement from appearing inside a web lifecycle operation without
-introducing a mutable lock pathname in the service-writable directory.
+The web panel holds an advisory lock on its open persistent state directory
+(the parent of the SQLite database) across the complete managed-client
+lifecycle, including database persistence and client-config cleanup. Supported
+installer `--add-client` and `--remove-client` operations resolve and lock the
+same directory descriptor from the installed service configuration. This inode
+remains available when the client-config directory is absent, prevents an
+out-of-band same-name replacement during web lifecycle work, and avoids a
+mutable lock pathname in a service-writable directory.
 
 ---
 

amneziawg-web/docs/MVP.md

@@ -27,7 +27,7 @@
 |21 | 232 unit + integration tests              | Auth, domain, DB, config, history, web handler, lifecycle/admin layers    |
 |22 | User create (native Rust)                 | `POST /api/admin/users`, HTML form at `/admin/users/add`; validates name; allocates IPs; writes configs; syncs AWG directly |
 |23 | User remove (native Rust)                 | `POST /api/admin/users/:id/remove`, HTML form at `/admin/users/:id/remove`; confirmation required; rewrites server config and syncs AWG directly |
-|24 | Lifecycle locking + validation             | Cross-process add/remove lock on the client-config directory shared with installer CLI operations, plus installer-name validation for managed user actions |
+|24 | Lifecycle locking + validation             | Cross-process add/remove lock on the persistent database state directory shared with installer CLI operations, plus installer-name validation for managed user actions |
 |25 | User lifecycle audit events               | `user_create_requested`, `user_created`, `user_create_failed`, `user_remove_requested`, `user_removed`, `user_remove_failed` |
 |26 | Post-action config rescan                 | `poller::rescan_configs()` called after create/remove; no manual restart needed |
 

amneziawg-web/src/db/peers.rs

@@ -357,6 +357,7 @@ pub async fn find_snapshots(
 ///
 /// Returns the updated `PeerRow`, or `None` if no peer with the given `id`
 /// exists.
+#[cfg(test)]
 pub async fn update_peer_metadata(
     pool: &SqlitePool,
     id: i64,
@@ -393,18 +394,19 @@ pub async fn update_peer_details(
     expiration_update: Option<Option<&str>>,
     managed_client_name: Option<&str>,
 ) -> Result<Option<PeerRow>, sqlx::Error> {
-    let Some(expires_at) = expiration_update else {
-        return update_peer_metadata(pool, id, display_name, comment).await;
-    };
+    let update_expiration = expiration_update.is_some();
+    let expires_at = expiration_update.flatten();
     let result = sqlx::query(
         "UPDATE peers
-         SET display_name = ?, comment = ?, expires_at = ?,
+         SET display_name = ?, comment = ?,
+             expires_at = CASE WHEN ? THEN ? ELSE expires_at END,
              managed_client_name = COALESCE(?, managed_client_name),
              updated_at = CURRENT_TIMESTAMP
          WHERE id = ? AND archived = 0 AND removal_pending = 0",
     )
     .bind(display_name)
     .bind(comment)
+    .bind(update_expiration)
     .bind(expires_at)
     .bind(managed_client_name)
     .bind(id)
@@ -1384,6 +1386,47 @@ mod tests {
         assert!(result.is_none());
     }
 
+    #[tokio::test]
+    async fn update_peer_details_keeps_omitted_expiration_and_rejects_pending_removal() {
+        let db = test_db().await;
+        let id = insert_peer(&db.pool, "KEY_DETAILS_REMOVING=", Some("Original")).await;
+        update_peer_expiration(&db.pool, id, Some("2026-08-18T12:00:00Z"), Some("alice"))
+            .await
+            .unwrap()
+            .expect("set expiration");
+
+        let updated = update_peer_details(
+            &db.pool,
+            id,
+            Some("Before removal"),
+            Some("Before removal comment"),
+            None,
+            None,
+        )
+        .await
+        .unwrap()
+        .expect("metadata-only update");
+        assert_eq!(updated.expires_at.as_deref(), Some("2026-08-18T12:00:00Z"));
+
+        assert!(mark_removal_pending(&db.pool, id).await.unwrap());
+        assert!(update_peer_details(
+            &db.pool,
+            id,
+            Some("Changed"),
+            Some("Changed comment"),
+            None,
+            None,
+        )
+        .await
+        .unwrap()
+        .is_none());
+
+        let row = find_by_id(&db.pool, id).await.unwrap().unwrap();
+        assert_eq!(row.display_name.as_deref(), Some("Before removal"));
+        assert_eq!(row.comment.as_deref(), Some("Before removal comment"));
+        assert_eq!(row.expires_at.as_deref(), Some("2026-08-18T12:00:00Z"));
+    }
+
     // ── expiration ──────────────────────────────────────────────────────────
 
     #[tokio::test]

amneziawg-web/src/web/mod.rs

@@ -6144,6 +6144,7 @@ mod tests {
 
         let app = test_router(db.clone());
         let response = app
+            .clone()
             .oneshot(
                 Request::builder()
                     .method("PATCH")
@@ -6157,6 +6158,20 @@ mod tests {
             .await
             .unwrap();
         assert_eq!(response.status(), StatusCode::CONFLICT);
+        let response = app
+            .oneshot(
+                Request::builder()
+                    .method("PATCH")
+                    .uri(format!("/api/peers/{id}"))
+                    .header("content-type", "application/json")
+                    .body(Body::from(
+                        r#"{"display_name":"Changed","comment":"Changed comment"}"#,
+                    ))
+                    .unwrap(),
+            )
+            .await
+            .unwrap();
+        assert_eq!(response.status(), StatusCode::CONFLICT);
         let row = crate::db::peers::find_by_id(&db.pool, id)
             .await
             .unwrap()
@@ -6202,6 +6217,7 @@ mod tests {
 
         let app = test_router(db.clone());
         let response = app
+            .clone()
             .oneshot(
                 Request::builder()
                     .method("POST")
@@ -6215,6 +6231,18 @@ mod tests {
             .await
             .unwrap();
         assert_eq!(response.status(), StatusCode::CONFLICT);
+        let response = app
+            .oneshot(
+                Request::builder()
+                    .method("POST")
+                    .uri(format!("/peers/{id}"))
+                    .header("content-type", "application/x-www-form-urlencoded")
+                    .body(Body::from("display_name=Changed&comment=Changed+comment"))
+                    .unwrap(),
+            )
+            .await
+            .unwrap();
+        assert_eq!(response.status(), StatusCode::CONFLICT);
         let row = crate::db::peers::find_by_id(&db.pool, id)
             .await
             .unwrap()

tests/test-functions.sh

@@ -1164,11 +1164,13 @@ NIAC_CUSTOM_CONFIG_DIR="${NIAC_WEB_ROOT}/custom-clients"
 NIAC_CUSTOM_STATE_DIR="${NIAC_WEB_ROOT}/custom-state"
 mkdir -p "${NIAC_CUSTOM_STATE_DIR}"
 cat > "${NIAC_WEB_ROOT}/custom.env" <<EOF
-AWG_WEB_DB=${NIAC_CUSTOM_STATE_DIR}/awg-web.db
+AWG_WEB_DB=sqlite://${NIAC_CUSTOM_STATE_DIR}/awg-web.db
 AWG_CONFIG_DIR=${NIAC_CUSTOM_CONFIG_DIR}
 EOF
 cat > "${NIAC_WEB_ROOT}/amneziawg-web.service" <<EOF
 [Service]
+Environment=AWG_WEB_DB=${NIAC_LOCK_DIR}/ignored.db
+Environment=AWG_CONFIG_DIR=${NIAC_LOCK_DIR}/ignored-clients
 EnvironmentFile=${NIAC_WEB_ROOT}/custom.env
 EOF
 WEB_PANEL_CONFIG_DIR="${NIAC_LOCK_DIR}"
@@ -1189,12 +1191,35 @@ assert_rc 1 acquireClientLifecycleLock
 assert_eq "false" "$([[ -e "${NIAC_CUSTOM_STATE_DIR}" ]] && echo true || echo false)" \
 	"root CLI does not recreate a missing panel state directory"
 
+# Inline unit settings are supported too. Relative database paths resolve from
+# WorkingDirectory, and an unreferenced environment file is ignored.
+NIAC_INLINE_WORK_DIR="${NIAC_WEB_ROOT}/inline-work"
+NIAC_INLINE_STATE_DIR="${NIAC_INLINE_WORK_DIR}/relative-state"
+NIAC_INLINE_CONFIG_DIR="${NIAC_WEB_ROOT}/inline-clients"
+mkdir -p "${NIAC_INLINE_STATE_DIR}"
+WEB_PANEL_ENV_FILE="${NIAC_WEB_ROOT}/custom.env"
+cat > "${NIAC_WEB_ROOT}/amneziawg-web.service" <<EOF
+[Service]
+WorkingDirectory=${NIAC_INLINE_WORK_DIR}
+Environment="AWG_WEB_DB=sqlite:relative-state/awg-web.db"
+Environment=AWG_CONFIG_DIR=${NIAC_INLINE_CONFIG_DIR}
+EOF
+assert_eq "${NIAC_INLINE_STATE_DIR}" "$(resolveClientLifecycleLockDir)" \
+	"inline relative database path resolves from WorkingDirectory"
+assert_eq "${NIAC_INLINE_CONFIG_DIR}" "$(resolveWebPanelConfigDir)" \
+	"inline client config directory is discovered"
+exec {NIAC_HELD_LOCK_FD}< "${NIAC_INLINE_STATE_DIR}"
+flock -xn "${NIAC_HELD_LOCK_FD}"
+assert_rc 1 _client_lock_must_be_busy
+exec {NIAC_HELD_LOCK_FD}>&-
+
 WEB_PANEL_CONFIG_DIR="${NIAC_ORIGINAL_WEB_PANEL_CONFIG_DIR}"
 WEB_PANEL_ENV_FILE="${NIAC_ORIGINAL_WEB_PANEL_ENV_FILE}"
 WEB_PANEL_SYSTEMD_UNIT="${NIAC_ORIGINAL_WEB_PANEL_SYSTEMD_UNIT}"
 rm -rf "${NIAC_LOCK_DIR}"
 rm -rf "${NIAC_WEB_ROOT}"
 unset NIAC_LOCK_DIR NIAC_LOCK_LINK NIAC_CUSTOM_CONFIG_DIR NIAC_CUSTOM_STATE_DIR NIAC_WEB_ROOT
+unset NIAC_INLINE_WORK_DIR NIAC_INLINE_STATE_DIR NIAC_INLINE_CONFIG_DIR
 unset NIAC_ORIGINAL_WEB_PANEL_CONFIG_DIR NIAC_ORIGINAL_WEB_PANEL_ENV_FILE
 unset NIAC_ORIGINAL_WEB_PANEL_SYSTEMD_UNIT NIAC_HELD_LOCK_FD
 unset -f _client_lock_must_be_busy