Skip to content

Encrypt HF token at rest, add AnalysisManager test harness, fix progress-bar stride - #38

Open
payamsash wants to merge 1 commit into
mainfrom
feature/hf-token-encryption-analysis-tests-progress-fix
Open

Encrypt HF token at rest, add AnalysisManager test harness, fix progress-bar stride#38
payamsash wants to merge 1 commit into
mainfrom
feature/hf-token-encryption-analysis-tests-progress-fix

Conversation

@payamsash

Copy link
Copy Markdown
Collaborator

Summary

  • HF token encryption: AnalysisSettings::hfToken is now DPAPI-encrypted at rest on Windows builds instead of stored in plaintext in settings.json. Pre-existing plaintext values load fine and silently upgrade to encrypted on the next save. Fails closed (empty field, not garbage) if a blob is ever corrupted or restored on a different machine/account. AdminPanelDialog's Export/Import Configuration feature now detects and warns when an imported token couldn't be decrypted (it's a raw file copy, so a token encrypted on a different Windows account/machine won't carry over).
  • New hermetic AnalysisManager test harness: a small standalone stub subprocess + QCoreApplication-backed test entry point let tests/test_analysis_manager.cpp exercise real subprocess launch, stdout/stderr channel separation, env-var passthrough, exit-code mapping, and job queueing — without depending on a real Python interpreter. This immediately found and fixed a real, previously-shipped bug: AnalysisManager::stop() could crash (or leave a subprocess unmanaged) if the running process finished during the wait, due to Qt re-entrantly delivering the finished signal from inside the blocking waitForFinished() call.
  • Progress-bar stride fix: run_gaze_fusion.py and run_pose3d.py now scale their progress-print interval to the video length (max(1, total // 200)) instead of a fixed % 100, matching the fix already applied to the other analysis scripts — short sessions now show smooth progress instead of one big jump.

Test plan

  • mosaic and mosaic_tests build clean
  • ctest -C Release: 189/189 passing (including 5 new DpapiCrypt and 7 new AnalysisManagerTest cases)
  • High-effort code-review pass run; 3 additional real findings fixed (export/import token-loss warning, a second AnalysisManager::stop() re-entrancy gap, a doc-comment overclaim) — one review finding was independently re-verified and found partially incorrect, so only the confirmed part was acted on
  • Real hardware: paste a real HF token, confirm settings.json shows the encrypted blob not plaintext, confirm it decrypts correctly on relaunch
  • Real hardware: confirm Export/Import Configuration surfaces the new warning when moving a config across Windows accounts/machines

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant