Skip to content

Batch 20260917: security hardening, safe legacy migration, diagnostics, installer lifecycle - #14

Merged
mimaxon1 merged 2 commits into
mainfrom
hermes/pc-remote-project-batch-20260917
Sep 17, 2026
Merged

mimaxon1 merged 2 commits into
mainfrom
hermes/pc-remote-project-batch-20260917

Conversation

@mimaxon1

@mimaxon1 mimaxon1 commented Sep 17, 2026 •

Copy link
Copy Markdown
Owner

Summary

Security hardening, non-destructive legacy migration, read-only diagnostics, and an installer lifecycle report/smoke harness for PC Remote. Based on main @ d728079.

1. Security review (confirmed defects only)

Finding Fix
Alternate PIN paths (check, change_password) skipped the login attempt budget Shared serialized budget in _verify_legacy_pin (lock around admission/verify/accounting). Token auth is unaffected.
CORS only restricted response visibility, not request execution Explicit Origin middleware rejects disallowed browser origins with 403. No-Origin native/tray clients still work.
Uvicorn trusted forwarded client headers by default proxy_headers=False on this direct-LAN server.

Documented, not “fixed” (by design): trusted-LAN plain HTTP, 4-digit PIN entropy, broad authenticated desktop control.

  • SECURITY.md — policy + runtime residual risks
  • docs/SECURITY_REVIEW.md — boundary table + evidence
  • tests/test_security_contract.py

2. Unified safe migration layer

migration.safe_migrate is now the single owner of legacy network/settings/autostart moves.

  • Never deletes the source when the destination already exists (old code did)
  • Never deletes the source when the move fails
  • Auth: leftover legacy + missing destination → SettingsError (no silent fresh PIN reset)
  • Network leftover: preserved, ignored this run, explicit warning
  • Autostart: colliding legacy Startup CMD is kept as a user-visible fallback; install writes exact CRLF (newline="")

auth.py / net_utils.py / autostart.py now call into this layer instead of three divergent copies.

3. Legacy branch disposition

BRANCH_MIGRATION_REPORT.md compares 12 topic refs against pinned main (d728079):

  • 5 agent/windows-installer-* tips already contained
  • Experimental AppId / admin+firewall / unconditional Run-key autostart rejected wholesale
  • fix/preserve-legacy-* behavior carried via the migration owner, not by merging the branches
  • Inno gains ownership-checked uninstall cleanup only (exact Startup CMD / HKCU Run match for this install)

4. Read-only diagnostics

python main.py --diagnostics

JSON: version, bind address/port, autostart state, firewall summary, config path, last-error category. No PINs, hashes, tokens, settings contents, or raw log text. Runs before GUI/logging/auth/settings init.

5. Installer smoke matrix (harness ready, lifecycle not run here)

  • Static Inno contracts + plan-only PowerShell harness with dual consent gates
  • Matrix rows for disposable Win10/Win11 standard-user VMs: disabled / owned / foreign
  • Uninstall removes only owned Startup CMD + matching HKCU Run value
  • Machine firewall/service inventory must be byte-identical before/after

Testing

  • Full suite: 132 passed (baseline 107), Python 3.13.14, pytest 9.0.2
  • Includes a real child-process --diagnostics run with temp APPDATA (settings bytes unchanged, no log created)
  • Migration fail-closed paths covered: collision without delete, failed move without delete, SettingsError when leftover cannot move
  • Inno static ownership/precision contracts; harness plan-only + missing-gate denial
.venv\Scripts\python.exe -m pytest -q
powershell.exe -NoProfile -NonInteractive -File scripts\installer_smoke.ps1

Changed files (20)

Code: main.py, auth.py, net_utils.py, autostart.py, migration.py, diagnostics.py, installer/PC-Remote.iss, scripts/installer_smoke.ps1
Docs: SECURITY.md, BRANCH_MIGRATION_REPORT.md, docs/SECURITY_REVIEW.md, docs/diagnostics.md, docs/installer-smoke.md, docs/PROJECT_BATCH_CHECKLIST.md
Tests: test_security_contract.py, test_migration.py, test_diagnostics.py, test_installer_contract.py, test_installer_smoke.py, conftest_helpers.py

Commits

  1. caf5b79 — batch: security hardening, safe migration, diagnostics, installer report/smoke
  2. 9043fbc — migration: fail closed when leftover legacy settings cannot move

Review order

  1. 9043fbc — small fail-closed migration fix (auth/net_utils + 3 tests)
  2. caf5b79 — remaining batch (security + migration core + diagnostics + installer)
  3. Suggested reading path: docs/PROJECT_BATCH_CHECKLIST.md → docs/SECURITY_REVIEW.md → BRANCH_MIGRATION_REPORT.md → docs/installer-smoke.md → code/tests

Blocked / residual risks

  • Not executed: Inno compile, real upgrade, login autostart, Windows uninstall residue. No ISCC on PATH, no dist/PC Remote/PC Remote.exe, no disposable VM authorized here. Static/gate tests are not lifecycle proof.
  • Trusted-LAN plain HTTP and low-entropy PIN are intentional product limits, not regressions.
  • This is a defensive source review, not a penetration test.
  • Legacy admin/firewall experimental packages need a separate ownership-aware cleanup plan; do not port those installer branches.

Do not mark install acceptance until the disposable-VM matrix in docs/installer-smoke.md has been run.

Raise SettingsError instead of silently initializing fresh auth settings when a legacy file remains and the destination is missing. Preserve the same non-destructive rule for network settings with an explicit warning.

Claude-Session: https://claude.ai/code/session_012Wt8ZEQeC9WJFrTQaieQJH
@mimaxon1
mimaxon1 merged commit 5af0db7 into main Sep 17, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant