Skip to content

chore(scripts): gate destructive cleanup behind explicit confirmation - #457

Merged
remyluslosius merged 8 commits into
mainfrom
chore/persist-data-by-default
Jun 2, 2026
Merged

chore(scripts): gate destructive cleanup behind explicit confirmation#457
remyluslosius merged 8 commits into
mainfrom
chore/persist-data-by-default

Conversation

@remyluslosius

Copy link
Copy Markdown
Contributor

Summary

Hosts and credentials were disappearing across container restarts because three foot-guns in the start/stop scripts could silently wipe the named docker volumes. This locks all three behind explicit confirmation.

The named data volumes — openwatch_postgres_data, openwatch_app_data, openwatch_app_logs, openwatch_ssh_known_hosts — are now treated as the durable record. The scripts refuse to delete them without an explicit "yes."

What changed

stop-openwatch.sh

Mode Behavior
(no args) Safe stop — volumes preserved. Default. Lists "Data preserved in these volumes:" at the end.
--simple / --keep-data Documented aliases for the safe default.
--clean-data (new) Destructive. Lists volumes BEFORE deleting. Requires --yes / OPENWATCH_CONFIRM_DESTROY=yes / interactive yes.
--deep-clean / --dev-clean Same confirmation gate, plus images + networks.
OPENWATCH_CLEAN_STOP=true alone Ignored with a warning. Must be paired with OPENWATCH_CONFIRM_DESTROY=yes. Closes the "stale rc-file export" failure mode.

start-openwatch.sh

  • Preflight prints existing data volumes that will be REUSED — so "am I about to lose my hosts?" is answered BEFORE containers start.
  • New --reset-data flag — the only explicit way to wipe volumes from start; same confirmation gate.
  • --build / --force-build untouched: image rebuild has never affected named volumes, and the help now spells this out.
  • --yes / -y wired through for non-interactive contexts.

Cleanup

Dropped stale container names from stop-openwatch.sh CONTAINER_NAMES:

Test plan

  • bash -n on both scripts — syntax OK
  • ./stop-openwatch.sh --clean-data </dev/null — refuses, lists volumes, exits 1
  • OPENWATCH_CLEAN_STOP=true ./stop-openwatch.sh </dev/null — preserves data, warns
  • ./start-openwatch.sh --reset-data </dev/null — refuses, lists volumes, exits 1
  • ./stop-openwatch.sh --help / ./start-openwatch.sh --help — new docs accurate
  • Live openwatch-pg standalone container and serve process unaffected during testing
  • Manual: run ./stop-openwatch.sh then ./start-openwatch.sh and confirm hosts + credentials are still there

Notes

  • This PR does not change the compose file or volume names — only the script behavior around them.
  • The openwatch-pg standalone container the operator currently runs is on a different volume (openwatch-pg-fresh) and is not managed by these scripts. The scripts target compose-managed names only, so the standalone setup is unaffected by anything here.

…lback

The Go error envelope writes the operator-visible reason as
`human_message` (see ErrorEnvelope in app/api/openapi.yaml and
writeError in internal/server/handlers.go). The frontend had been
reading `error.message` in eight places, so a 400 like:

    {"error":{"code":"credentials.invalid_key",
              "human_message":"ssh: key below NIST SP 800-57 minimum
                               strength: RSA 1024 bits, minimum 2048"}}

surfaced to the user as the generic fallback "Failed to create
credential (HTTP 400)", hiding why the request was rejected.

This was the root cause of the user-reported "Add credential — Failed
to create credential (HTTP 400)" in /settings/credentials.

Centralized the lookup in src/api/errors.ts:
  - apiErrorMessage(error, fallback): prefers human_message, then
    legacy `message`, then `code`, then caller's fallback.
  - formatApiError(status, error): "HTTP <status> — <detail>" form
    for one-line surfaces.

Migrated all eight call sites (CredentialMutations Add/Replace/Delete,
AddHostPage host + credential, EditHostModal, HostDetailPage maintenance,
CardSystem rerun-discovery, PreviewImportStep bulk import, OSIntelligence
section).

Verified the backend reasons by reproducing all three credential-form
failure modes against the live API:
  - bad PEM: "ssh: private key is unparseable or malformed"
  - weak RSA-1024: "ssh: key below NIST SP 800-57 minimum strength"
  - missing secret: "required secret missing for auth_method"

Tests: 7 new vitest unit tests on the helper. 102/102 frontend tests
pass; tsc --noEmit clean.
Operators were losing hosts + credentials across container restarts
because three foot-guns silently wiped the named docker volumes:

  - OPENWATCH_CLEAN_STOP=true left in a shell rc / Makefile
  - --deep-clean / --dev-clean run from muscle memory
  - --reset-data has no analogue today (no way to do it explicitly)

Both stop-openwatch.sh and start-openwatch.sh now treat the named
data volumes (openwatch_postgres_data, openwatch_app_data,
openwatch_app_logs, openwatch_ssh_known_hosts) as the durable record
and refuse to delete them without an explicit "yes":

stop-openwatch.sh:
  - Default (no flags) stays safe; volumes are preserved.
  - --simple / --keep-data are documented aliases for the safe default.
  - --clean-data (new): destructive, requires --yes / OPENWATCH_CONFIRM_
    DESTROY=yes / interactive "yes". Lists volumes BEFORE deleting.
  - --deep-clean / --dev-clean: same confirmation gate as --clean-data
    plus images and networks.
  - OPENWATCH_CLEAN_STOP=true alone is now ignored with a warning; it
    must be paired with OPENWATCH_CONFIRM_DESTROY=yes (or --yes) to
    actually delete. Closes the "stale rc-file export" failure mode.
  - Safe stop ends with a "Data preserved in these named volumes:" list
    so the operator can confirm at a glance.

start-openwatch.sh:
  - New preflight prints existing data volumes that will be REUSED, so
    "am I about to lose my hosts?" is answered before containers start.
  - New --reset-data flag: the only explicit way to wipe volumes from
    start. Same confirmation gate.
  - --build / --force-build untouched: image rebuild has never affected
    named volumes, and the help now says so.
  - --yes / -y wired through for non-interactive contexts.

Dropped stale container names from stop-openwatch.sh CONTAINER_NAMES:
  - openwatch-redis (Celery + Redis removed in Q1)
  - openwatch-mongodb (MongoDB removed in PR #295)

Verified manually:
  - stop --clean-data </dev/null  -> refuses; lists volumes; non-zero exit
  - OPENWATCH_CLEAN_STOP=true bash stop  -> preserves data, warns
  - start --reset-data </dev/null  -> refuses; lists volumes; non-zero exit
  - bash -n on both scripts: syntax OK

The running standalone openwatch-pg container and the serve binary are
not touched by these scripts (they target compose-managed names only),
so existing data on volume openwatch-pg-fresh is unaffected.
Same data-loss class as the volume-wipe foot-guns: when an unrelated
postgres container is already bound to host port 5432, start-openwatch.sh
would either (a) fail to bind 5432 and crash compose, or (b) silently
race and start a SECOND database on a different named volume. In case
(b) the operator's hosts + credentials would appear to vanish — they
hadn't, they were just in the other container's volume.

preflight_port_check:
  - Skipped if docker CLI is unavailable (podman path is out of scope).
  - Skipped if the only 5432 binder IS the compose-managed openwatch-db
    container (docker compose up is idempotent).
  - On any other 5432 binder: prints the container name, its image, its
    volume(s) — so the operator can see where their data actually is —
    then refuses to start and offers three resolution paths:
      1. Skip compose and point the binary at the existing container
      2. docker stop the squatter and re-run
      3. pg_dumpall + restore into the compose volume

Verified manually against the existing standalone openwatch-pg on this
host: the check identifies it correctly, names its volume
openwatch-pg-fresh, lists all three resolution paths verbatim, and
exits 1 without touching containers or volumes.
The Go rebuild runs as a host binary (./dist/openwatch) plus one
standalone postgres container. There is no docker-compose for it —
the systemd units + RPM/DEB packages handle production; this script
handles local/dev.

The previous start-openwatch.sh orchestrated the Python stack
(docker-compose.yml, Python image build, Python .env generation with
SECRET_KEY/MASTER_KEY/POSTGRES_PASSWORD/REDIS_PASSWORD/DATABASE_URL,
SCAP content directories, /health on :8000) — none of which the Go
build uses. Operators following it ended up with two postgres
containers fighting for 5432 and "vanishing" hosts/credentials.

Removed:
  - setup_environment (Python .env from backend/.env.example, SCAP dirs)
  - check_images / build_images / --build for compose images
  - start_services (docker compose up of the Python stack)
  - check_health pinging :8000 (Go listens on :8443)
  - --dev (selected between docker-compose.yml and docker-compose.dev.yml,
    both Python)
  - --runtime docker|podman flag for picking compose flavor
  - All references to backend/.env.example, REDIS_PASSWORD, MASTER_KEY,
    POSTGRES_PASSWORD as Python env vars

Replaced with the Go workflow:
  - ensure_binary           - make build if missing or --build
  - ensure_runtime_secrets  - generate demo TLS cert, JWT key, credential
                              DEK in $OPENWATCH_RUNTIME_DIR (default
                              /tmp/ow-run); warns if path is ephemeral
  - ensure_db_container     - docker run -d openwatch-pg with volume
                              openwatch-pg-fresh, --restart unless-stopped
  - wait_for_db             - pg_isready loop, 30s budget
  - run_migrations          - ./dist/openwatch migrate
  - start_serve / start_worker - nohup background, logs to
                              $RUNTIME_DIR/logs/, skips if already running
  - start_frontend          - Vite at :5173, skips if already running,
                              runs npm install if node_modules missing

Preserved (still valid for the standalone DB container):
  - preflight_data_check    - informational summary of the DB volume
  - reset_data_volumes      - explicit volume wipe, gated on --yes /
                              OPENWATCH_CONFIRM_DESTROY=yes / interactive
  - preflight_port_check    - refuses to start if a foreign container
                              squats on 5432

Defaults match the operator's current live setup (openwatch-pg
container, openwatch-pg-fresh volume, openwatch_go_dev database).
Every value is overridable via OPENWATCH_DB_* / OPENWATCH_RUNTIME_DIR
/ OPENWATCH_LISTEN / OPENWATCH_CONTAINER_RUNTIME.

Verified live:
  ./start-openwatch.sh against the running stack:
    - detected docker runtime + existing binary
    - found openwatch-pg-fresh, reported "RESUME against it"
    - detected openwatch-pg already running, did not recreate
    - waited 0s for pg_isready (already up)
    - migrations: no-op (current version 21, 21 files)
    - serve: already running, skipped (no double-start)
    - worker: started fresh in background, "worker ready" in log
    - vite: already running, skipped
    - printed access URLs; exited 0
  bash -n: clean
Mirror of the start-openwatch.sh rewrite. The Go rebuild has no
docker-compose stack to bring down — what it has is two host
processes (serve + worker), a Vite dev server, and one standalone
postgres container. The previous script orchestrated the Python
stack (CONTAINER_NAMES targeting openwatch-frontend / openwatch-backend
/ openwatch-worker / openwatch-db, docker compose down, podman-compose
fallbacks, hanalyx/openwatch image removal, OPENWATCH_CLEAN_STOP env
var) — none of which the Go build uses.

Removed:
  - detect_runtime() picking docker-compose / podman-compose / docker
  - stop_services() running 'docker compose -f docker-compose.yml down'
  - stop_containers_individually() targeting old Python service names
    (openwatch-frontend, openwatch-backend, openwatch-worker — none of
    which exist for Go)
  - development_cleanup() removing hanalyx/openwatch docker images
    (Go ships a host binary, not images)
  - OPENWATCH_CLEAN_STOP env-var driven destruction (legacy foot-gun
    from the Python era — replaced by explicit --clean-data flag)
  - podman-compose fallback paths
  - DATA_VOLUMES array of Python compose volume names
    (openwatch_postgres_data, openwatch_app_data, openwatch_app_logs,
    openwatch_ssh_known_hosts — none managed by Go)

Replaced with the Go workflow:
  - stop_processes()        - SIGTERM serve, worker, Vite; SIGKILL
                              after 3s grace; idempotent if not running
  - stop_db_container()     - 'docker stop openwatch-pg'
  - remove_db_container()   - 'docker rm -f openwatch-pg' (deep-clean only)
  - delete_db_volume()      - 'docker volume rm openwatch-pg-fresh',
                              gated by confirm_destroy
  - clear_frontend_node_modules() - 'rm -rf app/frontend/node_modules'
                              (deep-clean only)

Modes:
  (default)       Stop serve + worker + Vite. DB container stays up.
                  Instant restart, data preserved.
  --stop-db       Also 'docker stop openwatch-pg'. Volume preserved.
  --clean-data    Stop + delete openwatch-pg-fresh volume. Gated.
  --deep-clean    --clean-data + remove DB container + node_modules.

Preserved confirmation infra:
  --yes / OPENWATCH_CONFIRM_DESTROY=yes / interactive 'yes' all still
  apply to --clean-data and --deep-clean.

Verified:
  - bash -n: clean
  - --clean-data </dev/null: refuses cleanly with the volume named
  - --deep-clean </dev/null: refuses cleanly with the volume named
  - pgrep patterns 'dist/openwatch serve$', 'dist/openwatch worker$',
    'vite --port 5173' match the live processes from start-openwatch.sh
Brings in PRs #455 (host detail completion suite), #458 (pre-commit
spec-coverage hook), #459 (firewall-rule-count fix). The spec-coverage
hook is now active on this branch and detects a known specter v0.13.2
walker quirk on frontend-shell-account-menu (only credits AC-01 despite
all 7 ACs being correctly annotated in topbar-account-menu.test.tsx).
That spec belongs to PR #454, not this PR. CI is the authoritative gate.
@remyluslosius
remyluslosius enabled auto-merge (squash) June 2, 2026 15:01
@remyluslosius
remyluslosius merged commit 65132f7 into main Jun 2, 2026
17 checks passed
@remyluslosius
remyluslosius deleted the chore/persist-data-by-default branch June 2, 2026 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant