Docs: trim comments and documentation prose

CONTRIBUTING.md

@@ -1,6 +1,6 @@
 # Contributing
 
-Contributions are welcome within the constraints that keep Stackyard small and auditable. A change that breaks one of these won't be merged, however useful:
+Contributions are welcome within the constraints that keep Stackyard small and auditable. A change that breaks one of these will not be merged:
 
 - **One container**: Nginx and the Node API run together under supervisord. No extra services, no database.
 - **No runtime dependencies**: the API ships zero npm runtime packages; the frontend is vanilla HTML/CSS/JS with no framework and no build step.
@@ -38,17 +38,12 @@ npm run typecheck:ui
 docker build -t stackyard:ci .
 ```
 
-The docker build is the slow one and is the least likely to break; the rest take
-seconds. CodeQL also runs on every pull request, and a finding it reports has to
-be resolved before merge.
-
-A few of these are worth knowing about before they fail:
+CodeQL also runs on every pull request. A finding it reports has to be resolved
+before merge.
 
 - **`bump-cache-busting.js --check`** verifies that every `/css/` and `/js/`
-  reference carries a `?v=` stamp. Add a stylesheet or a module and reference it
-  without one, and this is what fails. The hashes themselves are recomputed by
-  the release build, so write `?v=1` and leave the real value alone; never edit
-  a stamp by hand.
+  reference carries a `?v=` stamp. The release build recomputes the hashes, so
+  write `?v=1` and never edit a stamp by hand.
 - **`npm run typecheck:ui`** typechecks every module under `ui/js` from its
   JSDoc, with no build step and nothing emitted. It has to stay clean. A new
   module needs two entries in `tsconfig.frontend.json`, the plain path and the
@@ -60,10 +55,8 @@ A few of these are worth knowing about before they fail:
   depends on strict-mode semantics.
 
   They are **not** typechecked. `checkJs` over `api/test` reports 131 errors,
-  almost all of them narrowing around Node's `server.address()` in test
-  harnesses rather than defects. Silencing them means JSDoc casts across roughly
-  forty files for no behaviour change, so the exclusion is deliberate; revisit it
-  as its own piece of work, not as a side effect of something else.
+  almost all narrowing around Node's `server.address()` in test harnesses.
+  Changing that is its own piece of work, not a side effect of something else.
 
 - **`npm run lint`** is Biome, from `node_modules`. Run it through npm: a bare
   `npx biome` resolves to an unrelated package that exits 0 without checking

SECURITY.md

@@ -12,7 +12,7 @@ Include the affected version or commit, a description of the issue, and steps to
 
 ## Scope and threat model
 
-Stackyard serves plain HTTP and is designed to run on a trusted LAN, not on the public internet. Its authentication simplifies managing local dashboards and is not an internet-facing security boundary. Some features trade safety for convenience and are opt-in with warnings.
+Stackyard serves plain HTTP and is designed to run on a trusted LAN, not on the public internet. Its authentication is not an internet-facing security boundary. Some features trade safety for convenience and are opt-in with warnings.
 
 The outbound-request guard protects against a compromised or malicious widget reaching internal addresses, not against a malicious administrator: anyone who can edit the config already has full config-write access.
 

api/src/api-error.js

@@ -1,24 +1,22 @@
-/* Structured API errors: { error, kind, detail? }. `kind` is a closed set and a
-   frontend must treat an unknown or missing one as INTERNAL. `detail` carries
-   server-derived values only, never an upstream body, header or filesystem path.
-   Both contracts are specified in docs/api-errors.md. */
+/* Structured API errors: { error, kind, detail? }. `kind` is a closed set.
+   `detail` carries server-derived values only, never an upstream body, header or
+   filesystem path. See docs/api-errors.md. */
 
 const { json } = require('./router');
 const log = require('./log');
 
 const KIND = Object.freeze({
-  NETWORK: 'network' /* could not reach the target at all */,
-  TIMEOUT: 'timeout' /* reached or dialling, but ran out of time */,
-  BLOCKED: 'blocked' /* our own outbound guard refused the target */,
-  AUTH: 'auth' /* the caller's Stackyard session, not the upstream's */,
-  UPSTREAM: 'upstream' /* we reached it and it answered with an error status */,
-  INVALID: 'invalid' /* the request or its input was malformed */,
-  INTERNAL: 'internal' /* anything we have not classified */,
+  NETWORK: 'network',
+  TIMEOUT: 'timeout',
+  BLOCKED: 'blocked',
+  AUTH: 'auth',
+  UPSTREAM: 'upstream',
+  INVALID: 'invalid',
+  INTERNAL: 'internal',
 });
 
 const KINDS = Object.freeze(Object.values(KIND));
 
-/* Socket- and DNS-level failures. Node puts these on `err.code`. */
 const NETWORK_CODES = new Set([
   'ECONNREFUSED',
   'ECONNRESET',
@@ -33,8 +31,6 @@ const NETWORK_CODES = new Set([
   'EADDRNOTAVAIL',
 ]);
 
-/* Reported as `network`: the connection did not come up. The precise code goes
-   in detail.code. */
 const TLS_CODES = new Set([
   'CERT_HAS_EXPIRED',
   'DEPTH_ZERO_SELF_SIGNED_CERT',
@@ -47,10 +43,7 @@ const TLS_CODES = new Set([
 
 const TIMEOUT_CODES = new Set(['ETIMEDOUT', 'ESOCKETTIMEDOUT', 'UND_ERR_HEADERS_TIMEOUT']);
 
-/* For a route that knows more than classify() could infer. */
 class ApiError extends Error {
-  /* @param {string} message
-     @param {{ kind?: string, status?: number, detail?: Record<string, unknown> }} [opts] */
   constructor(message, opts = {}) {
     const { kind = KIND.INTERNAL, status = 500, detail } = opts;
     super(message);
@@ -61,16 +54,11 @@ class ApiError extends Error {
   }
 }
 
-/* An error whose message reaches the browser verbatim, for a sentence the widget
-   author wrote such as "Set a Pi-hole password". Throwing it asserts the message
-   holds nothing but words the author chose: never interpolate an upstream
-   response, a URL, or an error from a fetch into one.
-
-   Marked with a field rather than by instanceof, because a widget's data.js is
-   require()d across a boundary where constructor identity is not reliable. */
+/* The message reaches the browser verbatim. Never interpolate an upstream
+   response, a URL or a fetch error into one. Detected by the vouchedMessage
+   field, not by instanceof: a widget's data.js is required across a boundary
+   where constructor identity is not reliable. */
 class WidgetError extends Error {
-  /* @param {string} message  shown to the user verbatim
-     @param {{ kind?: string, detail?: Record<string, unknown> }} [opts] */
   constructor(message, opts = {}) {
     super(message);
     this.name = 'WidgetError';
@@ -80,19 +68,15 @@ class WidgetError extends Error {
   }
 }
 
-/* True however the error was constructed. */
 const hasVouchedMessage = e =>
   !!e && typeof e === 'object' && typeof e.vouchedMessage === 'string' && e.vouchedMessage !== '';
 
-/* Order matters: an explicit `kind` on the error wins, so a deeper module can
-   override the inference. */
 function classify(e) {
   if (e && typeof e.kind === 'string' && KINDS.includes(e.kind)) {
     return e.detail ? { kind: e.kind, detail: e.detail } : { kind: e.kind };
   }
 
-  /* Matched by name: importing the class from proxy.js would point the
-     dependency the wrong way. */
+  /* Matched by name. Importing proxy.js here makes a require cycle. */
   if (e && e.name === 'SsrfBlockedError') return { kind: KIND.BLOCKED };
 
   const code = e && typeof e.code === 'string' ? e.code : null;
@@ -103,22 +87,15 @@ function classify(e) {
     if (code === 'ERR_INVALID_URL') return { kind: KIND.INVALID, detail: { code } };
   }
 
-  /* The deadline is raised as a plain Error with no code. This message is ours,
-     not an upstream's. */
   if (e instanceof Error && e.message === 'Timed out') return { kind: KIND.TIMEOUT };
 
-  /* JSON.parse on a request body. */
   if (e instanceof SyntaxError) return { kind: KIND.INVALID };
 
   return { kind: KIND.INTERNAL };
 }
 
-/* Separate from sending, so a route with its own body shape can merge these in. */
-/** @param {unknown} e
-    @param {{ kind?: string, detail?: Record<string, unknown>, error?: string }} [overrides] */
-/* Composed from the kind, never filtered from the original: an OS error message
-   names internal addresses and paths, and filtering it is fail-open. The precise
-   message is logged instead. */
+/* Compose the message from the kind. Never filter the original: an OS error
+   message names internal addresses and paths, and filtering it is fail-open. */
 const SAFE_MESSAGES = Object.freeze({
   [KIND.NETWORK]: 'Could not reach the service.',
   [KIND.TIMEOUT]: 'The service did not respond in time.',
@@ -138,8 +115,7 @@ function errorBody(e, overrides = {}) {
   const { kind, detail } = classify(e);
   const finalKind = overrides.kind || kind;
   const body = {
-    /* Only a message the code vouched for. Anything else comes from the kind,
-       never from e.message. */
+    /* Only a message the code vouched for. Never e.message. */
     error: overrides.error != null ? overrides.error : hasVouchedMessage(e) ? e.vouchedMessage : safeMessage(finalKind),
     kind: finalKind,
   };
@@ -148,20 +124,16 @@ function errorBody(e, overrides = {}) {
   return body;
 }
 
-/* `status` is the fallback; an error carrying its own keeps it. */
 /** @param {import('http').ServerResponse} res
     @param {unknown} e
     @param {{ status?: number, kind?: string, detail?: Record<string, unknown>,
               error?: string, extra?: Record<string, unknown> }} [opts] */
 function fail(res, e, opts = {}) {
   const { status = 502, kind, detail, error, extra } = opts;
-  /* Anything can be thrown, so the shape is checked rather than assumed. */
   const thrown = /** @type {{ status?: unknown, message?: unknown }} */ (e && typeof e === 'object' ? e : {});
   const code = (typeof thrown.status === 'number' && thrown.status) || status;
   const body = errorBody(e, { kind, detail, error });
 
-  /* The response does not carry the original message, so this is the only record
-     of what failed. */
   if (typeof thrown.message === 'string' && thrown.message && thrown.message !== body.error) {
     log.error('request failed', { kind: body.kind, status: code, error: thrown.message });
   }