Repository navigation
Refactor Phase 0: security pinning, UUID derivation fix, vendored JS, scripts/check - #1
Merged
Merged
Conversation
- nav-item divs: add role=button, tabindex=0, and onkeydown for keyboard accessibility (lines 31-33) - th width attributes: replace deprecated HTML width= with style=width: (lines 56-59, 203) - label elements: add for= attributes matching control ids throughout all modals (create, deploy, template create) - deploy password input: add missing id= so label for= can reference it - info modal host-keys table: add thead with Type/Fingerprint columns - inline Jinja2 script block (lines 383/385): move template data into a <script type=application/json id=app-data> data island to eliminate IDE false-positive JS parse errors from double-brace Jinja2 syntax - app.js: add initFromDataIsland() to read the JSON island at startup and populate window.identitiesList and window.xtermEnabled
- var → let/const throughout (const for values never reassigned)
- window → globalThis (window.location, window.open, window.close,
window.addEventListener, window.xtermEnabled, window.identitiesList)
- String#replace(regex, ...) → String#replaceAll(str, ...) in escapeHtml
- deprecated document.domain → location.hostname in socket.io connect URL
- for (var i ...) over radios → for...of in triggerConnectSubmit
- handleTemplateCreate: remove redundant 'return false' statements;
event.preventDefault() is sufficient, function is now void
- handleCreateSubmit: remove 'return false/true' pattern; allow native
form POST by simply not calling preventDefault when xterm is disabled
- triggerConnect: remove empty if (response.ok) {} block; invert to
if (!response.ok) for the error path only
- JSDoc added to all functions
- nav items: replace div role=button with real <button> elements; add button-reset CSS to .nav-item so rendering is unchanged - flash message: darken colours from #cc0000/#ffcccc to #7a0000/#f5c6c6 to meet WCAG AA minimum contrast ratio - remove explanatory HTML comment block (flagged as commented-out code); the same explanation is present in the app.js JSDoc for initFromDataIsland()
…ed refactor plan Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…st-key scan ssh-new and ssh-user-rotate previously connected with StrictHostKeyChecking=accept-new, which accepted whatever host key was presented — not necessarily the scanned one — and wrote it into the default ~/.ssh/known_hosts, silently trusting a provision-time MITM on the immediate follow-on connection. Every connection these scripts make (ssh-copy-id, verification ssh, post-provision exec ssh) now uses UserKnownHostsFile pointing at the stored known_host_keys plus StrictHostKeyChecking=yes. ssh-new refreshes the informational hostname column on successful re-verification so the pin matches re-provisioning under a new alias; ssh-user-rotate rewrites the column to the connect alias in a temp pin file. The remaining accept-new in lib/ssh-ui.py is retired by the Phase 1 rewrite + Phase 4 bin/ssh-deploy (REFACTOR.md). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The code hashed only the base64 column; ARCHITECTURE.md claimed the full "<host> <type> <base64>" line. Both were wrong: the agreed derivation is sha256 over "<key-type> <base64-key>" — hostname column stripped so the identity is pure key material, stable across hostname/IP changes. Pre-1.0: existing host-uuid/<sha256> dirs no longer match; re-provision. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nstall-time fetch (W5) install.sh no longer downloads anything: xterm assets are committed to lib/ui/vendor/ with SHA-384 hashes recorded in SHA384SUMS. The vendored bytes are the @latest snapshots the previous installer fetched on 2026-01-31 (legacy packages frozen at xterm@5.3.0 / addon-fit@0.8.0 — see vendor/PROVENANCE); run scripts/vendor-update to fetch and pin @xterm/xterm@5.5.0 with a dual-CDN byte-identical cross-check. socket.io-client is dropped entirely, not vendored: Phase 1 replaces flask_socketio with a hand-rolled RFC 6455 WebSocket and a browser-native WebSocket client. Until then the web terminal is disabled (xterm_enabled=False); all UI flows fall back to plain form POSTs. The --install-xterm/--skip-xterm installer flags and the interactive prompt are gone along with the download path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, thin CI wrapper scripts/check runs staged modules from scripts/check.d/: syntax (bash -n, py_compile), lint (shellcheck/ruff when installed, else [SKIP]), vendor (SHA384SUMS verification incl. unrecorded-file detection), tests (plain-bash test_*.sh always; bats/python suites when present), secrets (token/private-key grep over tracked files). Phase 0 lint severity is shellcheck -S error and ruff's runtime-error subset; Phase 5 tightens to full severity per the REFACTOR.md exit gate. scripts/install-hooks writes an opt-in pre-push hook that execs scripts/check. CI (.github/workflows/check.yml) installs the optional tools and runs the same script — no lint logic duplicated in YAML. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t stubs (W9) Both scripts referenced helpers that were never defined anywhere (validate_hostname, CONFIG_DIR, validate_template, get_hosts_using_template, log) and failed immediately when run. Per REFACTOR.md Phase 0 task 5 the preferred option is removal with a stub that says so and exits 1 — keeping the codebase honest about what works. Both are rebuilt from scratch, with bats coverage, in Phase 5. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, case arm) The parser carried a duplicated --comment usage line, duplicated NO_CONNECT/COMMENT_TEXT initialisation, and a dead duplicate --comment case arm. One copy of each remains. tests/bash/test_ssh_new_usage.sh (run by scripts/check tests) asserts --comment appears exactly once in both the usage text and the parser. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…atus, Development section Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…fit@0.10.0 Fetched via scripts/vendor-update (dual-CDN byte-identical cross-check between unpkg and jsdelivr); interim 5.3.0-era snapshots removed, SHA384SUMS regenerated, index.html references and PROVENANCE updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Executes Phase 0 of the clean-slate refactor plan, plus two security fixes agreed during design review. Adds the plan itself (
ARCHITECTURE.md,REFACTOR.md) to the repo. Also carries the three HTML-lint commits fromfix/html-lint-warnings(branched from there).Security fixes
ssh-newandssh-user-rotatepreviously usedStrictHostKeyChecking=accept-new, which accepted whatever host key was presented at deploy time — not necessarily the scanned one — and wrote it into the default~/.ssh/known_hosts, silently trusting a provision-time MITM on the follow-on connection. Every connection these scripts make is now pinned viaUserKnownHostsFile+StrictHostKeyChecking=yesagainst exactly the keys the host identity was derived from.sha256("<key-type> <base64-key>")of the best scanned key. Previously the code hashed only the base64 column while ARCHITECTURE.md claimed the full scan line — both wrong.host-uuid/dirs won't match; re-provision (no migration, per plan).Phase 0 (REFACTOR.md tasks 1–10)
install.shperforms no network fetches. xterm assets vendored at pinned@xterm/xterm@5.5.0/@xterm/addon-fit@0.10.0underlib/ui/vendor/with SHA-384 sums; fetched via newscripts/vendor-update(accepts bytes only when unpkg and jsdelivr agree). socket.io-client dropped entirely — web terminal disabled until the Phase 1 native-WebSocket rewrite; other UI flows fall back to form POSTs.scripts/check— single validation entry point (syntax / lint / vendor / tests / secrets stage modules), opt-in pre-push hook viascripts/install-hooks, thin CI wrapper.github/workflows/check.ymlrunning the same script.ssh-rotate/ssh-template-rotatereplaced with honest stubs (exit 1, point at Phase 5 rebuild).ssh-newarg parsing de-duplicated with a regression test (tests/bash/test_ssh_new_usage.sh).Test plan
scripts/checkexits 0 on a fresh clone (syntax, vendor, tests, secrets; lint skips where shellcheck/ruff absent — CI runs them)install.shcompleted with curl/wget shadowed to fail and sandboxed$HOME— zero network attempts, vendored assets installedbin/, noaccept-newinbin/orinstall.sh,bash -nclean on every script🤖 Generated with Claude Code