Merge pull request #15 from jcreek/14-read-only-mode

fix(#14): surface explicit writable diagnostics in API and read-only …

.github/workflows/dockerhub-description.yml

@@ -0,0 +1,59 @@
+name: Docker Hub Description
+
+on:
+  push:
+    branches:
+      - main
+    paths:
+      - README.md
+      - .github/workflows/dockerhub-description.yml
+
+jobs:
+  dockerhub-description:
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Preflight Docker Hub repository access
+        env:
+          DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }}
+          DOCKERHUB_PASSWORD: ${{ secrets.DOCKERHUB_TOKEN }}
+          DOCKERHUB_REPOSITORY: jcreek23/open-network-diagram
+        run: |
+          set -euo pipefail
+
+          if [ -z "${DOCKERHUB_USERNAME}" ] || [ -z "${DOCKERHUB_PASSWORD}" ]; then
+            echo "Missing DOCKERHUB_USERNAME or DOCKERHUB_PASSWORD secret."
+            exit 1
+          fi
+
+          LOGIN_RESPONSE="$(curl -fsS -X POST \
+            -H 'Content-Type: application/json' \
+            -d "{\"username\":\"${DOCKERHUB_USERNAME}\",\"password\":\"${DOCKERHUB_PASSWORD}\"}" \
+            https://hub.docker.com/v2/users/login/)"
+
+          TOKEN="$(printf '%s' "${LOGIN_RESPONSE}" | ruby -rjson -e 'input = STDIN.read; data = JSON.parse(input); puts data.fetch("token", "")')"
+
+          if [ -z "${TOKEN}" ]; then
+            echo "Docker Hub login succeeded but token was missing."
+            exit 1
+          fi
+
+          curl -fsS \
+            -H "Authorization: JWT ${TOKEN}" \
+            "https://hub.docker.com/v2/repositories/${DOCKERHUB_REPOSITORY}/" >/dev/null
+
+          echo "Docker Hub preflight access check passed for ${DOCKERHUB_REPOSITORY}."
+
+      - name: Update Docker Hub repository overview
+        uses: peter-evans/dockerhub-description@v5
+        with:
+          username: ${{ secrets.DOCKERHUB_USERNAME }}
+          password: ${{ secrets.DOCKERHUB_TOKEN }}
+          repository: jcreek23/open-network-diagram
+          readme-filepath: ./README.md
+          enable-url-completion: true

.github/workflows/release.yml

@@ -13,27 +13,6 @@ concurrency:
   cancel-in-progress: false
 
 jobs:
-  changed-files:
-    runs-on: ubuntu-latest
-    permissions:
-      contents: read
-    outputs:
-      readme: ${{ steps.filter.outputs.readme }}
-
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v4
-        with:
-          fetch-depth: 0
-
-      - name: Detect changed files
-        id: filter
-        uses: dorny/paths-filter@v3
-        with:
-          filters: |
-            readme:
-              - README.md
-
   release:
     runs-on: ubuntu-latest
     permissions:
@@ -110,23 +89,3 @@ jobs:
           labels: ${{ steps.meta.outputs.labels }}
           cache-from: type=gha
           cache-to: type=gha,mode=max
-
-  dockerhub-overview:
-    needs: changed-files
-    if: needs.changed-files.outputs.readme == 'true'
-    runs-on: ubuntu-latest
-    permissions:
-      contents: read
-
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v4
-
-      - name: Update Docker Hub repository overview
-        uses: peter-evans/dockerhub-description@v5
-        with:
-          username: ${{ secrets.DOCKERHUB_USERNAME }}
-          password: ${{ secrets.DOCKERHUB_TOKEN }}
-          repository: ${{ env.IMAGE_NAME }}
-          readme-filepath: ./README.md
-          enable-url-completion: true

README.md

@@ -12,7 +12,7 @@ Open Network Diagram helps you document your infrastructure in a visual UI while
 
 - Homelab-friendly: run it in minutes with Docker.
 - Practical: edit in the UI and autosave to `network.json`.
-- Declarative: keep your topology in Git if you want.
+- Declarative: keep your topology in Git if you wish.
 
 [Docker Hub](https://hub.docker.com/r/jcreek23/open-network-diagram) | [Live Demo (Read-Only)](https://opennetworkdiagram.jcreek.co.uk) | [GitHub Releases](https://github.com/jcreek/OpenNetworkDiagram/releases)
 
@@ -75,7 +75,7 @@ docker rm open-network-diagram
 ## What It Looks Like
 
 | Network view with ethernet labels                                          | Non-network view with VMs expanded                                          | Modal editing a machine                                          |
-| -------------------------------------------------------------------- | ---------------------------------------------------------------------- | ---------------------------------------------------- |
+| -------------------------------------------------------------------------- | --------------------------------------------------------------------------- | ---------------------------------------------------------------- |
 | ![Open Network Diagram network view with ethernet labels](screenshot1.png) | ![Open Network Diagram non-network view with VMs expanded](screenshot2.png) | ![Open Network Diagram modal editing a machine](screenshot3.png) |
 
 ## Docker Compose Option
@@ -128,6 +128,7 @@ pnpm run icons:manifest # regenerate local vendor icon manifest
 
 - API endpoint: `GET/PUT /api/network-data`
 - Writes are enabled unless `NETWORK_READ_ONLY=true`
+- When writes are unavailable, API responses include `writableReason` for diagnostics.
 - Writes are persisted atomically to the configured data file
 - Rolling backups are kept in the backup directory (last 5)
 

server.mjs

@@ -4,8 +4,7 @@ import path from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { validateNetworkData } from './src/lib/shared/networkSchemaCore.mjs';
 import {
-	checkWritableState,
+	getWritableState,
-	isWriteEnabled,
 	readNetworkFile,
 	writeNetworkFile
 } from './src/lib/shared/networkPersistenceCore.mjs';
@@ -50,10 +49,17 @@ function badRequest(response, message, details) {
 	});
 }
 
+function resolveReadOnlyErrorMessage(reason) {
+	if (!reason) {
+		return 'Write API unavailable in current deployment.';
+	}
+	return `Write API unavailable: ${reason}`;
+}
+
 async function serveApi(request, response) {
 	if (request.method === 'GET') {
 		try {
-			const payload = await readNetworkFile();
+			const [payload, writableState] = await Promise.all([readNetworkFile(), getWritableState()]);
 			const validation = validateNetworkData(payload.data);
 			if (!validation.valid) {
 				sendJson(response, 500, {
@@ -63,11 +69,10 @@ async function serveApi(request, response) {
 				return;
 			}
 
-			const canWrite = await checkWritableState();
-
 			sendJson(response, 200, {
 				data: validation.data,
-				writable: canWrite,
+				writable: writableState.writable,
+				writableReason: writableState.reason,
 				source: payload.source,
 				updatedAt: payload.updatedAt
 			});
@@ -80,9 +85,12 @@ async function serveApi(request, response) {
 	}
 
 	if (request.method === 'PUT') {
-		if (!isWriteEnabled()) {
+		const writableStateBeforeWrite = await getWritableState();
+		if (!writableStateBeforeWrite.writable) {
 			sendJson(response, 403, {
-				error: 'Write API disabled. Unset NETWORK_READ_ONLY to enable persistence.'
+				error: resolveReadOnlyErrorMessage(writableStateBeforeWrite.reason),
+				writable: false,
+				writableReason: writableStateBeforeWrite.reason
 			});
 			return;
 		}
@@ -123,9 +131,11 @@ async function serveApi(request, response) {
 
 		try {
 			const metadata = await writeNetworkFile(validation.data);
+			const writableStateAfterWrite = await getWritableState();
 			sendJson(response, 200, {
 				data: validation.data,
-				writable: true,
+				writable: writableStateAfterWrite.writable,
+				writableReason: writableStateAfterWrite.reason,
 				source: metadata.source,
 				updatedAt: metadata.updatedAt
 			});

src/lib/components/NetworkDiagram.svelte

@@ -73,7 +73,8 @@
 	let saveState: SaveState = 'saved';
 	let dataSourceLabel = jsonPath;
 	let writable = false;
-	let readOnlyNotice = 'Read-only deployment; changes are not persisted.';
+	const defaultReadOnlyNotice = 'Read-only deployment; changes are not persisted.';
+	let readOnlyNotice = defaultReadOnlyNotice;
 
 	let showEthernetLabels = false;
 	let diagramViewMode: DiagramViewMode = 'network';
@@ -401,6 +402,13 @@
 		return 'Unexpected error';
 	}
 
+	function resolveReadOnlyNotice(reason: string | null | undefined): string {
+		if (!reason) {
+			return defaultReadOnlyNotice;
+		}
+		return `Read-only: ${reason}`;
+	}
+
 	function createGraphStyles(theme: ThemeMode): cytoscape.Stylesheet[] {
 		const nodeText = theme === 'dark' ? '#e2e8f0' : '#1e293b';
 		const textOutline = theme === 'dark' ? '#0f172a' : '#f8fafc';
@@ -976,8 +984,12 @@
 			});
 
 			if (response.status === 403) {
+				const body = (await response.json().catch(() => ({}))) as {
+					error?: string;
+					writableReason?: string | null;
+				};
 				writable = false;
-				readOnlyNotice = 'Read-only deployment; changes are not persisted.';
+				readOnlyNotice = resolveReadOnlyNotice(body.writableReason ?? body.error ?? null);
 				saveState = 'unsaved';
 				return;
 			}
@@ -998,10 +1010,14 @@
 			const body = (await response.json()) as {
 				data: NetworkData;
 				writable: boolean;
+				writableReason?: string | null;
 				source: string;
 				updatedAt: string;
 			};
 			writable = body.writable;
+			readOnlyNotice = body.writable
+				? defaultReadOnlyNotice
+				: resolveReadOnlyNotice(body.writableReason);
 			dataSourceLabel = body.source;
 			lastSavedSnapshot = JSON.stringify(body.data);
 			networkData = cloneNetworkData(body.data);
@@ -1385,6 +1401,7 @@
 			const body = (await response.json()) as {
 				data: NetworkData;
 				writable: boolean;
+				writableReason?: string | null;
 				source: string;
 				updatedAt: string;
 			};
@@ -1396,6 +1413,9 @@
 				networkData = cloneNetworkData(validation.data);
 				ensureSelectedTargetValid();
 				writable = body.writable;
+				readOnlyNotice = body.writable
+					? defaultReadOnlyNotice
+					: resolveReadOnlyNotice(body.writableReason);
 				dataSourceLabel = body.source;
 				lastSavedSnapshot = JSON.stringify(networkData);
 				saveState = 'saved';
@@ -1414,6 +1434,7 @@
 				networkData = cloneNetworkData(fallbackData);
 				ensureSelectedTargetValid();
 				writable = false;
+				readOnlyNotice = 'Read-only: API unavailable; using bundled static data.';
 				dataSourceLabel = jsonPath;
 				lastSavedSnapshot = JSON.stringify(networkData);
 				saveState = 'saved';

src/lib/server/networkPersistence.ts

@@ -1,5 +1,6 @@
 import {
 	checkWritableState as checkWritableStateCore,
+	getWritableState as getWritableStateCore,
 	isWriteEnabled as isWriteEnabledCore,
 	readNetworkFile as readNetworkFileCore,
 	writeNetworkFile as writeNetworkFileCore
@@ -12,6 +13,11 @@ export interface NetworkFileMetadata {
 	updatedAt: string;
 }
 
+export interface WritableState {
+	writable: boolean;
+	reason: string | null;
+}
+
 export function isWriteEnabled(): boolean {
 	return isWriteEnabledCore();
 }
@@ -24,6 +30,10 @@ export async function writeNetworkFile(data: NetworkData): Promise<NetworkFileMe
 	return (await writeNetworkFileCore(data)) as NetworkFileMetadata;
 }
 
+export async function getWritableState(): Promise<WritableState> {
+	return (await getWritableStateCore()) as WritableState;
+}
+
 export async function checkWritableState(): Promise<boolean> {
 	return checkWritableStateCore();
 }

src/lib/shared/networkPersistenceCore.mjs

@@ -16,6 +16,7 @@ import path from 'node:path';
  * @typedef {{ machines: unknown[]; devices: unknown[] }} NetworkData
  * @typedef {{ source: string; updatedAt: string }} NetworkFileMetadata
  * @typedef {{ data: NetworkData } & NetworkFileMetadata} NetworkFileReadResult
+ * @typedef {{ writable: boolean; reason: string | null }} WritableState
  */
 
 const NETWORK_READ_ONLY = process.env.NETWORK_READ_ONLY === 'true';
@@ -43,6 +44,38 @@ export function isWriteEnabled() {
 	return !NETWORK_READ_ONLY;
 }
 
+/**
+ * @param {unknown} error
+ * @returns {string}
+ */
+function formatFileSystemErrorCode(error) {
+	if (error && typeof error === 'object' && 'code' in error && typeof error.code === 'string') {
+		return error.code;
+	}
+	return 'UNKNOWN';
+}
+
+/**
+ * @param {string} directory
+ * @param {string} label
+ * @returns {Promise<string | null>}
+ */
+async function assertDirectoryWritable(directory, label) {
+	try {
+		await mkdir(directory, { recursive: true });
+	} catch (error) {
+		return `${label} directory "${directory}" could not be created (${formatFileSystemErrorCode(error)}).`;
+	}
+
+	try {
+		await access(directory, constants.W_OK);
+	} catch (error) {
+		return `${label} directory "${directory}" is not writable (${formatFileSystemErrorCode(error)}).`;
+	}
+
+	return null;
+}
+
 /**
  * @param {string} dataFilePath
  * @returns {Promise<string>}
@@ -151,20 +184,62 @@ export async function writeNetworkFile(data) {
 }
 
 /**
- * @returns {Promise<boolean>}
+ * @returns {Promise<WritableState>}
  */
-export async function checkWritableState() {
+export async function getWritableState() {
 	if (!isWriteEnabled()) {
-		return false;
+		return {
+			writable: false,
+			reason: 'Writes disabled by NETWORK_READ_ONLY=true.'
+		};
 	}
 
 	const source = resolveDataFilePath();
 	const directory = path.dirname(source);
-	try {
+	const dataDirectoryError = await assertDirectoryWritable(directory, 'Data');
-		await mkdir(directory, { recursive: true });
+	if (dataDirectoryError) {
-		await access(directory, constants.W_OK);
+		return {
-	} catch {
+			writable: false,
-		return false;
+			reason: dataDirectoryError
+		};
 	}
-	return true;
+
+	const backupDirectory = resolveBackupDirectory();
+	const backupDirectoryError = await assertDirectoryWritable(backupDirectory, 'Backup');
+	if (backupDirectoryError) {
+		return {
+			writable: false,
+			reason: backupDirectoryError
+		};
+	}
+
+	try {
+		await access(source, constants.F_OK);
+	} catch {
+		return {
+			writable: true,
+			reason: null
+		};
+	}
+
+	try {
+		await access(source, constants.R_OK | constants.W_OK);
+	} catch (error) {
+		return {
+			writable: false,
+			reason: `Data file "${source}" exists but is not readable and writable (${formatFileSystemErrorCode(error)}).`
+		};
+	}
+
+	return {
+		writable: true,
+		reason: null
+	};
+}
+
+/**
+ * @returns {Promise<boolean>}
+ */
+export async function checkWritableState() {
+	return (await getWritableState()).writable;
 }

src/routes/api/network-data/+server.ts

@@ -2,8 +2,7 @@ import { json } from '@sveltejs/kit';
 
 import { validateNetworkData } from '$lib/data/networkSchema';
 import {
-	checkWritableState,
+	getWritableState,
-	isWriteEnabled,
 	readNetworkFile,
 	writeNetworkFile
 } from '$lib/server/networkPersistence';
@@ -16,11 +15,18 @@ function serializeValidationErrors(errors: Array<{ path: string; message: string
 	}));
 }
 
+function resolveReadOnlyErrorMessage(reason: string | null): string {
+	if (!reason) {
+		return 'Write API unavailable in current deployment.';
+	}
+	return `Write API unavailable: ${reason}`;
+}
+
 export const GET: RequestHandler = async () => {
 	try {
-		const [{ data, source, updatedAt }, writable] = await Promise.all([
+		const [{ data, source, updatedAt }, writableState] = await Promise.all([
 			readNetworkFile(),
-			checkWritableState()
+			getWritableState()
 		]);
 		const validation = validateNetworkData(data);
 		if (!validation.valid || !validation.data) {
@@ -35,7 +41,8 @@ export const GET: RequestHandler = async () => {
 
 		return json({
 			data: validation.data,
-			writable,
+			writable: writableState.writable,
+			writableReason: writableState.reason,
 			source,
 			updatedAt
 		});
@@ -47,10 +54,13 @@ export const GET: RequestHandler = async () => {
 };
 
 export const PUT: RequestHandler = async ({ request }) => {
-	if (!isWriteEnabled()) {
+	const writableStateBeforeWrite = await getWritableState();
+	if (!writableStateBeforeWrite.writable) {
 		return json(
 			{
-				error: 'Write API disabled. Unset NETWORK_READ_ONLY to enable persistence.'
+				error: resolveReadOnlyErrorMessage(writableStateBeforeWrite.reason),
+				writable: false,
+				writableReason: writableStateBeforeWrite.reason
 			},
 			{ status: 403 }
 		);
@@ -76,10 +86,11 @@ export const PUT: RequestHandler = async ({ request }) => {
 
 	try {
 		const metadata = await writeNetworkFile(validation.data);
-		const writable = await checkWritableState();
+		const writableStateAfterWrite = await getWritableState();
 		return json({
 			data: validation.data,
-			writable,
+			writable: writableStateAfterWrite.writable,
+			writableReason: writableStateAfterWrite.reason,
 			source: metadata.source,
 			updatedAt: metadata.updatedAt
 		});