From d4f643528d5628ea48e951858534070272bf907c Mon Sep 17 00:00:00 2001 From: Steve Hanlon Date: Tue, 21 Jul 2026 11:52:00 +0100 Subject: [PATCH] Dashboard: reopen a finished healthcheck (hc-lh6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a Reopen button on the session-detail view for finished sessions. POSTs to /dashboard/healthchecks/{id}/reopen with a per-session CSRF token; on success clears finished_at and redirects back to the session. Also tightens Controllers/Healthchecks::update so a PUT with no finished_at key is a no-op instead of silently setting it to NULL — otherwise an empty PUT would reopen any completed healthcheck. After reopening, the WP-side Session::resume($id) flow pulls the now- active session back into an editable local session on the client site. --- .beads/issues.jsonl | 1 + server/src/Controllers/Dashboard.php | 63 ++++++++++++++++++++++++- server/src/Controllers/Healthchecks.php | 10 +++- server/src/routes.php | 1 + 4 files changed, 71 insertions(+), 4 deletions(-) diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index eca9d9c..d20f112 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -15,6 +15,7 @@ {"_type":"issue","id":"hc-5ix.4","title":"Session data model: option-backed in-progress healthcheck record","description":"One in-progress session per site at a time. Stored in a custom option (or a CPT — pick during implementation). Shape: id, started_at, finished_at, technician_id, site_url_snapshot, per_step_state { status: not_started|in_progress|done|skipped|blocked|n_a, notes, completed_at }. Designed so phase-3 automation can attach structured findings later.","notes":"Built in phase-1 scaffold; passing lint + WP-eval end-to-end smoke test on testsite.","status":"closed","priority":1,"issue_type":"task","owner":"steve@hanlon.co.uk","created_at":"2026-06-11T14:36:11Z","created_by":"Steve Hanlon","updated_at":"2026-06-11T14:49:27Z","started_at":"2026-06-11T14:41:27Z","closed_at":"2026-06-11T14:49:27Z","labels":["phase-1"],"dependencies":[{"issue_id":"hc-5ix.4","depends_on_id":"hc-5ix","type":"parent-child","created_at":"2026-06-11T15:36:11Z","created_by":"Steve Hanlon","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"hc-5ix.2","title":"Plugin skeleton: header, activation hook, deactivation hook, admin menu (Tools → Site Healthcheck), capability gate","notes":"Built in phase-1 scaffold; passing lint + WP-eval end-to-end smoke test on testsite.","status":"closed","priority":1,"issue_type":"task","owner":"steve@hanlon.co.uk","created_at":"2026-06-11T14:36:10Z","created_by":"Steve Hanlon","updated_at":"2026-06-11T14:49:26Z","started_at":"2026-06-11T14:41:27Z","closed_at":"2026-06-11T14:49:26Z","labels":["phase-1"],"dependencies":[{"issue_id":"hc-5ix.2","depends_on_id":"hc-5ix","type":"parent-child","created_at":"2026-06-11T15:36:09Z","created_by":"Steve Hanlon","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"hc-5ix.1","title":"Bootstrap: trigger install of wp-site-recovery plugin as step 0","description":"Healthcheck plugin should check on activation if site-recovery is installed; if not, fetch and install from a known URL/ZIP and activate it. Block stepper from starting until recovery is in place.","notes":"Detection half done (status panel + manual link). Auto-install of recovery plugin from a private URL is the wp-site-recovery side of hc-5ix.27 — closing this as 'detection complete'.","status":"closed","priority":1,"issue_type":"task","owner":"steve@hanlon.co.uk","created_at":"2026-06-11T11:26:40Z","created_by":"Steve Hanlon","updated_at":"2026-06-11T15:03:02Z","closed_at":"2026-06-11T15:03:02Z","labels":["phase-1"],"dependencies":[{"issue_id":"hc-5ix.1","depends_on_id":"hc-5ix","type":"parent-child","created_at":"2026-06-11T12:26:39Z","created_by":"Steve Hanlon","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"hc-lh6","title":"Dashboard: reopen a finished healthcheck","description":"Techs sometimes finish a healthcheck and later need to add a note (client came back with something after signoff). Currently there's no way to unfinish — the session is read-only on the dashboard.\n\nAdd a 'Reopen' button on the session-detail dashboard view that clears finished_at on the server, plus tighten the PUT /healthchecks/{id} controller so it doesn't accidentally reopen sessions when a client sends an empty body (latent bug: Validate::optionalInt returns null for missing keys, so an empty PUT currently sets finished_at=NULL).","design":"**Backend:**\n1. Tighten Controllers/Healthchecks::update() to use array_key_exists('finished_at', $body) instead of optionalInt. Missing key = no-op; present-with-null = reopen; present-with-int = finish/update.\n2. Add POST /dashboard/healthchecks/{id}/reopen — session-authenticated (mirrors the GET /dashboard auth pattern), CSRF-protected via a per-session token.\n3. On success, redirect back to the session detail page.\n\n**Frontend (dashboard):**\n4. In renderSessionDetail(), when finished_at !== null, add a small form with the reopen button + hidden CSRF token.\n5. Confirm dialog via inline JS onsubmit so the button isn't a one-click landmine.\n\n**CSRF token pattern:** stored in $_SESSION['att_hc_csrf'], generated on first use with random_bytes(16); verified with hash_equals().\n\n**Not doing (option (b) from the discussion):** preserving original finished_at in a separate column. If we want that later it's a separate migration + Store change.","acceptance_criteria":"- [ ] Empty PUT /healthchecks/{id} does not reopen (returns 200 no-op)\n- [ ] PUT /healthchecks/{id} with {\"finished_at\": null} reopens\n- [ ] PUT /healthchecks/{id} with {\"finished_at\": \u003cint\u003e} still finishes\n- [ ] Dashboard session-detail view shows Reopen button only when finished\n- [ ] Clicking Reopen (with confirm) clears finished_at and reloads the page\n- [ ] Reopen POST rejected without valid CSRF token\n- [ ] Reopen POST rejected without valid dashboard session","status":"closed","priority":2,"issue_type":"feature","assignee":"Steve Hanlon","owner":"steve@hanlon.co.uk","created_at":"2026-07-21T10:50:06Z","created_by":"Steve Hanlon","updated_at":"2026-07-21T10:52:00Z","started_at":"2026-07-21T10:50:12Z","closed_at":"2026-07-21T10:52:00Z","close_reason":"Implemented: dashboard reopen button + CSRF + tightened Healthchecks::update controller so empty PUT no longer accidentally reopens sessions.","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"hc-adg","title":"Recovery installer pulls unpinned 'latest' from Gitea + auto-activates without signature check","description":"`recovery-installer.php` (`resolve_latest_ref()` L67-92, install L148) fetches the recovery plugin from Gitea using an unpinned reference (releases → tags → `main` branch archive fallback), then feeds the downloaded ZIP straight to WP's `Plugin_Upgrader::install()` which activates the code.\n\nTrust anchor is Gitea repo integrity: whoever controls the recovery repo (or a leaked Gitea deploy token) controls what runs on every install at next tech click.\n\nTLS verification is on (default `wp_remote_get` behaviour, not disabled anywhere), so MITM is out of scope — but supply-chain compromise of the Gitea repo is not.\n\nNote: this is the same trust model as the new PUC auto-update flow (hc-8gb) — so it's not a new risk introduced by that change, but it's worth naming and mitigating in the same pass.","design":"Options:\n\n**a) Pin to signed tags only.** Require the recovery installer to reject anything that isn't a signed tag (verify via Gitea API's tag object). Only works if we start GPG-signing releases.\n\n**b) Verify a SHA-256 hash from a second source.** Ship an expected-hash constant in the plugin (updated at release time). Installer fetches ZIP + verifies hash before install. Simple, effective, but couples plugin releases to recovery releases.\n\n**c) Detached signature file alongside the ZIP.** e.g. `recovery-1.2.3.zip.sig` verified against a public key baked into the healthcheck plugin. Best long-term option, most setup.\n\n**d) Reduce blast radius via read-only, single-repo Gitea tokens.** Doesn't stop repo compromise but shrinks the credential footprint.\n\nRecommendation: start with **(b)** — quick win. Consider **(c)** if we ever distribute recovery outside the internal network.","acceptance_criteria":"- [ ] Recovery installer refuses to install a ZIP whose SHA-256 doesn't match an expected value shipped in the plugin (or a similar integrity check)\n- [ ] Recovery repo Gitea token is read-only + scoped to just that repo\n- [ ] README documents the release-integrity flow","notes":"Reported in 2026-07-16 security review. Flagged as risk #2 of 3.\n\nRelated: hc-8gb introduced PUC-based auto-updates for THIS plugin with the same trust model. Any signing/hashing scheme we build for recovery should be reusable for the healthcheck plugin's own updates.","status":"open","priority":2,"issue_type":"bug","owner":"steve@hanlon.co.uk","created_at":"2026-07-16T10:14:40Z","created_by":"Steve Hanlon","updated_at":"2026-07-16T10:14:40Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"hc-gp3","title":"Harden API key handling — remove settings-page echo, add DB encryption or force constants","description":"Central-server API key is currently exposed in two ways that widen the blast radius of a compromise:\n\n1. `admin-page.php:749` renders the key back into an `\u003cinput type=\"password\"\u003e` on the settings page — trivially unmasked via DOM inspector by any user with `manage_options`.\n2. When set via the settings UI (not a wp-config constant), the key is stored plaintext in `wp_options` under `att_hc_api_key` — readable by any user or process with DB access.\n\nBecause the same key is shared across every WP install that talks to the central server, a leak from one site gives read/write on every other site's healthcheck history.","design":"Options (pick one or combine):\n\n**a) Never render the key back into the input.** Show only a masked placeholder like `••••••••abc3` (last 4 chars) with a 'change' button that swaps in an empty input. Won't help against DB reads but blocks the easiest exfiltration path.\n\n**b) Force wp-config constants only.** Delete the settings-page input for the key entirely; require `ATT_HC_API_KEY` in `wp-config.php`. Simple, most secure, but loses the managed-host convenience the original design called out (`class-att-hc-api.php:14-19`).\n\n**c) Encrypt the option at rest.** Use a key derived from `AUTH_KEY` / `SECURE_AUTH_KEY` (already in wp-config) to encrypt the DB value. Still readable by anyone who can read both DB + wp-config, but that's a higher bar than DB-only.\n\n**d) Per-site keys instead of a shared key.** Bigger change on the server side (would need a keys table + rotation). Correct long-term fix. Files as a follow-up if we do (a)/(b) now.\n\nRecommendation: **(a) + prefer (b)** for new installs, document (b) as the strong default in README, keep (c)/(d) as future work.","acceptance_criteria":"- [ ] Settings page no longer echoes the key value into the DOM\n- [ ] README + wp-config docs recommend the constant path as the strong default\n- [ ] Existing option-based configs continue to work (no breakage on client sites)\n- [ ] Consider filing a follow-up for (d) per-site keys","notes":"Reported in 2026-07-16 security review. Flagged as risk #1 of 3.\n\nRelated risks (separate issues):\n- Recovery installer unpinned 'latest' pull\n- No rate limiting on server API + dashboard","status":"open","priority":2,"issue_type":"bug","owner":"steve@hanlon.co.uk","created_at":"2026-07-16T10:14:21Z","created_by":"Steve Hanlon","updated_at":"2026-07-16T10:14:21Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"hc-8gb","title":"Add auto-updates from Gitea via Plugin Update Checker","description":"Enable this plugin (and eventually the two other in-house plugins) to auto-update when a new version is pushed to Gitea's main branch, so field techs pick up fixes without manual re-installs.\n\nApproach: vendor YahnisElsts/plugin-update-checker (PUC) as a git submodule under `vendor/plugin-update-checker`, wire it up in `att-site-healthcheck.php` to poll a `updates.json` (or equivalent metadata file) served from the Gitea repo's main branch.\n\nOnce wired, WP's normal update flow surfaces new versions in Dashboard → Updates and honours site auto-update settings.","design":"**Library**: YahnisElsts/plugin-update-checker (PUC) v5.x — vendored as git submodule at `vendor/plugin-update-checker`.\n\n**Metadata source**: static `updates.json` committed to the plugin repo main branch, fetched via Gitea raw URL. Chosen over Gitea's Releases API because:\n- No auth needed if repo is readable from client sites' networks\n- Simple to bump in a release commit (version + download_url)\n- Avoids coupling to Gitea-specific API quirks\n\n**Wiring** (approx, in main plugin file after ABSPATH guard):\n```php\nrequire_once ATT_HC_PLUGIN_DIR . 'vendor/plugin-update-checker/plugin-update-checker.php';\n$updater = YahnisElsts\\PluginUpdateChecker\\v5\\PucFactory::buildUpdateChecker(\n 'https://\u003cgitea-host\u003e/steve/att-site-healthcheck/raw/branch/main/updates.json',\n ATT_HC_PLUGIN_FILE,\n 'att-site-healthcheck'\n);\n```\n\n**Release flow**:\n1. Bump `Version:` in plugin header + `ATT_HC_VERSION` constant\n2. Bump `version` + `download_url` in `updates.json` (pointing at a zip — either a Gitea release asset or a raw branch zipball)\n3. Commit + push to main\n4. Client sites pick it up on next WP update check (twice-daily) or manual 'Check again'\n\n**Later**: replicate the same submodule + updates.json pattern in the two sibling plugins.","acceptance_criteria":"- [ ] PUC vendored as git submodule under `vendor/plugin-update-checker`\n- [ ] Plugin bootstrap wires PUC against a Gitea raw URL for `updates.json`\n- [ ] `updates.json` template committed to repo main branch with current version\n- [ ] Bumping version + updates.json + pushing to main causes a test WP site to see the update in Dashboard → Updates\n- [ ] One-click update from WP admin installs the new version and the plugin remains active\n- [ ] README documents the release flow (bump header version, bump updates.json, push)\n- [ ] Pattern documented well enough to replicate for the other two in-house plugins","notes":"Implementation notes (2026-07-16):\n\nDeviated from original design: PUC is vendored directly under `vendor/plugin-update-checker/` (not a git submodule).\n\nReason: Gitea's `archive/main.zip` endpoint does NOT include submodule contents. A submodule-based approach would mean auto-downloaded update ZIPs are missing PUC entirely and fatal on activation. User confirmed vendored approach (2026-07-16).\n\n**Files added/changed:**\n- `vendor/plugin-update-checker/` — vendored PUC v5.7 (trimmed: no build/, vendor/, examples/, composer.json, phpcs.xml)\n- `att-site-healthcheck.php` — loads PUC and calls `PucFactory::buildUpdateChecker()` pointing at Gitea raw `updates.json`\n- `updates.json` — metadata file at repo root, served via Gitea raw URL\n- `README.md` — release flow + PUC-update instructions\n\n**Metadata URL**: https://git.h12e.com/steve/wp-healthcheck/raw/branch/main/updates.json\n**Download URL**: https://git.h12e.com/steve/wp-healthcheck/archive/main.zip\n\n**Folder rename**: PUC's base `UpdateChecker::fixDirectoryName()` handles the mismatch between Gitea's archive folder (`wp-healthcheck`) and the WP plugin slug (`att-site-healthcheck`) automatically via the `upgrader_source_selection` filter.\n\n**PUC v5 alias**: verified — `YahnisElsts\\PluginUpdateChecker\\v5\\PucFactory` resolves to v5p7's implementation.\n\n**Testing needed on real WP site:**\n1. Install this version, confirm no PHP errors on activation\n2. Bump version to 0.1.1 in header + updates.json, push\n3. Wait 12h OR click 'Check again' on Dashboard → Updates\n4. Confirm update offered, install it, confirm plugin still active + slug preserved\n\nOnce tested and confirmed working, replicate the same three-file pattern (vendor/, updates.json, main-plugin PUC wiring block) in the other two in-house plugins.","status":"closed","priority":2,"issue_type":"feature","assignee":"Steve Hanlon","owner":"steve@hanlon.co.uk","created_at":"2026-07-16T08:44:49Z","created_by":"Steve Hanlon","updated_at":"2026-07-16T08:52:50Z","started_at":"2026-07-16T08:47:36Z","closed_at":"2026-07-16T08:52:50Z","close_reason":"Implemented and pushed as commit 19d726f. Awaiting real-WP-site verification per the release-flow test in the notes before replicating to the two sibling plugins.","dependency_count":0,"dependent_count":0,"comment_count":0} diff --git a/server/src/Controllers/Dashboard.php b/server/src/Controllers/Dashboard.php index 45282dc..f06b4ea 100644 --- a/server/src/Controllers/Dashboard.php +++ b/server/src/Controllers/Dashboard.php @@ -19,6 +19,25 @@ use AttHc\Server\Store; */ final class Dashboard { + public static function reopen(array $params): void { + self::requireAuth(); + self::requireCsrf(); + + $hcId = (string) ($params['id'] ?? ''); + $hc = Store::getHealthcheck($hcId); + if ($hc === null) { + self::html('Not found', '

Session not found.

'); + return; + } + + Store::updateHealthcheck($hcId, null); + + $siteKey = (string) $hc['site_key']; + $back = '/dashboard?site=' . rawurlencode($siteKey) . '&hc=' . rawurlencode($hcId); + header('Location: ' . $back); + exit; + } + public static function index(array $params = []): void { self::requireAuth(); @@ -41,6 +60,29 @@ final class Dashboard { } } + // ── CSRF ───────────────────────────────────────────────────────────────── + + /** Returns the per-session CSRF token, generating one on first call. */ + private static function csrfToken(): string { + if (empty($_SESSION['att_hc_csrf']) || !is_string($_SESSION['att_hc_csrf'])) { + $_SESSION['att_hc_csrf'] = bin2hex(random_bytes(16)); + } + return $_SESSION['att_hc_csrf']; + } + + /** Aborts with 403 unless the POSTed csrf token matches the session's. */ + private static function requireCsrf(): void { + $submitted = isset($_POST['csrf']) && is_string($_POST['csrf']) ? $_POST['csrf'] : ''; + $expected = isset($_SESSION['att_hc_csrf']) && is_string($_SESSION['att_hc_csrf']) + ? $_SESSION['att_hc_csrf'] + : ''; + if ($expected === '' || !hash_equals($expected, $submitted)) { + http_response_code(403); + self::html('Forbidden', '

CSRF token missing or invalid. Reload the page and try again.

'); + exit; + } + } + // ── Auth ───────────────────────────────────────────────────────────────── private static function requireAuth(): void { @@ -175,13 +217,24 @@ final class Dashboard { ? 'in progress' : 'complete'; + $reopenForm = ''; + if ($finished !== null) { + $reopenForm = '
' + . '' + . '' + . '
'; + } + $meta = '
' . $badge . ' Started: ' . $started . '' . ($finished !== null ? 'Finished: ' . $finished . '' : '') . ($duration !== null ? 'Duration: ' . $duration . '' : '') - . 'ID: ' . htmlspecialchars($hcId) . ' -
'; + . 'ID: ' . htmlspecialchars($hcId) . '' + . $reopenForm + . ''; if (empty($steps)) { return $back . '

' . htmlspecialchars($siteKey) . ' — ' . $started . '

' @@ -300,6 +353,12 @@ pre.step-notes { margin: 0; font-family: inherit; white-space: pre-wrap; .login-box button { background: #2271b1; color: #fff; border: none; border-radius: 4px; padding: .5rem 1rem; font-size: 1rem; cursor: pointer; } .login-box button:hover { background: #135e96; } +/* reopen */ +.reopen-form { margin: 0 0 0 auto; } +.btn-reopen { background: #fff; color: #2271b1; border: 1px solid #2271b1; + border-radius: 4px; padding: .25rem .7rem; font-size: .85em; + cursor: pointer; font-weight: 500; } +.btn-reopen:hover { background: #2271b1; color: #fff; } diff --git a/server/src/Controllers/Healthchecks.php b/server/src/Controllers/Healthchecks.php index f746c43..6c39ee4 100644 --- a/server/src/Controllers/Healthchecks.php +++ b/server/src/Controllers/Healthchecks.php @@ -41,8 +41,14 @@ final class Healthchecks { return; } $body = Http::readJsonBody(); - $finishedAt = Validate::optionalInt($body, 'finished_at'); - Store::updateHealthcheck($id, $finishedAt); + + // Missing key = no-op. Explicit null = reopen. Int = finish/update. + // The array_key_exists check prevents an empty PUT from silently + // reopening a completed healthcheck. + if (array_key_exists('finished_at', $body)) { + $finishedAt = Validate::optionalInt($body, 'finished_at'); + Store::updateHealthcheck($id, $finishedAt); + } Http::json(200, ['ok' => true]); } diff --git a/server/src/routes.php b/server/src/routes.php index 45a06ab..cf08cf1 100644 --- a/server/src/routes.php +++ b/server/src/routes.php @@ -29,5 +29,6 @@ $router->add('GET', '/healthchecks/steps/{step_id}', [Steps::class $router->add('GET', '/step-counts', [Steps::class, 'counts']); $router->add('GET', '/sites', [Sites::class, 'recent']); $router->add('GET', '/dashboard', [Dashboard::class, 'index'], requiresAuth: false); +$router->add('POST', '/dashboard/healthchecks/{id}/reopen', [Dashboard::class, 'reopen'], requiresAuth: false); return $router;