diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index c05a8f79..4d59c9a4 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -32,21 +32,21 @@ **Root cause:** The Textual TUI setup wizard (`paperforge setup` without `--headless`) is broken (NameError crash), unreachable from either real install workflow (plugin settings tab uses `--headless`, AI agents use `--headless`), and adds ~1200 lines of Textual-dependent code to maintain. -- [ ] **DEPR-01**: `paperforge setup` (bare, no `--headless`) prints a help message redirecting to `--headless` or the plugin settings tab. Textual TUI classes removed from `setup_wizard.py`: `WelcomeStep`, `DirOverviewStep`, `VaultStep`, `PlatformStep`, `DeployStep`, `DoneStep`, `SetupWizardApp`, `ContentSwitcher`, `StepScreen`, and all TUI-only import paths. `headless_setup()` and shared utilities (`EnvChecker`, `AGENT_CONFIGS`, `_copy_file_incremental`, `_merge_env_incremental`) preserved. -- [ ] **DEPR-02**: Three documentation files updated: `docs/setup-guide.md`, `docs/INSTALLATION.md`, `README.md` — all bare `paperforge setup` references changed to `paperforge setup --headless`. -- [ ] **DEPR-03**: Post-install instruction text in `setup_wizard.py:1306-1307` and `headless_setup` completion message updated to reflect headless-only workflow. `--non-interactive` removed from CLI options. `textual` removed from project optional dependencies. +- [x] **DEPR-01**: `paperforge setup` (bare, no `--headless`) prints a help message redirecting to `--headless` or the plugin settings tab. Textual TUI classes removed from `setup_wizard.py`: `WelcomeStep`, `DirOverviewStep`, `VaultStep`, `PlatformStep`, `DeployStep`, `DoneStep`, `SetupWizardApp`, `ContentSwitcher`, `StepScreen`, and all TUI-only import paths. `headless_setup()` and shared utilities (`EnvChecker`, `AGENT_CONFIGS`, `_copy_file_incremental`, `_merge_env_incremental`) preserved. +- [x] **DEPR-02**: Three documentation files updated: `docs/setup-guide.md`, `docs/INSTALLATION.md`, `README.md` — all bare `paperforge setup` references changed to `paperforge setup --headless`. +- [x] **DEPR-03**: Post-install instruction text in `setup_wizard.py:1306-1307` and `headless_setup` completion message updated to reflect headless-only workflow. `--non-interactive` removed from CLI options. `textual` removed from project optional dependencies. ### HARDEN — Module Hardening (7 requirements) **Root cause:** New modules (`discussion.py`, `asset_state.py`) and the Obsidian plugin were built quickly during v1.6-v1.8 and lack safety hardening. -- [ ] **HARDEN-01**: `discussion.py:281-314` file-level locking around JSON and MD read-modify-write cycles. Concurrent `/pf-paper` and `/pf-deep` calls for the same paper do not silently drop sessions. -- [ ] **HARDEN-02**: `discussion.py:170-171` Markdown special characters (`*`, `#`, `[`, `_`, `` ` ``) escaped in QA question/answer fields. Individual QA dict keys validated before building session. -- [ ] **HARDEN-03**: `discussion.py:40` hardcoded CST (UTC+8) replaced with UTC. All timestamps use `datetime.now(timezone.utc)`. -- [ ] **HARDEN-04**: `main.js:2116` PaddleOCR API key passed via environment variable, not command-line argument. `spawn()` with `env: { PADDLEOCR_API_TOKEN: ... }`. -- [ ] **HARDEN-05**: `main.js:1815` `innerHTML` assignment replaced with `createEl()` DOM API for directory tree rendering. No XSS vector from user-configured directory names. -- [ ] **HARDEN-06**: `asset_state.py:225-226` workspace integrity checks performed before returning `"/pf-deep"`. Checks currently at lines 233-240 moved before line 226. -- [ ] **HARDEN-07**: `status.py:687-690` `lifecycle_level_counts`, `health_aggregate`, `maturity_distribution` return empty dicts `{}` when no canonical index exists (not `null`/`None`). Downstream JSON parsers do not crash on field access. +- [x] **HARDEN-01**: `discussion.py:281-314` file-level locking around JSON and MD read-modify-write cycles. Concurrent `/pf-paper` and `/pf-deep` calls for the same paper do not silently drop sessions. +- [x] **HARDEN-02**: `discussion.py:170-171` Markdown special characters (`*`, `#`, `[`, `_`, `` ` ``) escaped in QA question/answer fields. Individual QA dict keys validated before building session. +- [x] **HARDEN-03**: `discussion.py:40` hardcoded CST (UTC+8) replaced with UTC. All timestamps use `datetime.now(timezone.utc)`. +- [x] **HARDEN-04**: `main.js:2116` PaddleOCR API key passed via environment variable, not command-line argument. `spawn()` with `env: { PADDLEOCR_API_TOKEN: ... }`. +- [x] **HARDEN-05**: `main.js:1815` `innerHTML` assignment replaced with `createEl()` DOM API for directory tree rendering. No XSS vector from user-configured directory names. +- [x] **HARDEN-06**: `asset_state.py:225-226` workspace integrity checks performed before returning `"/pf-deep"`. Checks currently at lines 233-240 moved before line 226. +- [x] **HARDEN-07**: `status.py:687-690` `lifecycle_level_counts`, `health_aggregate`, `maturity_distribution` return empty dicts `{}` when no canonical index exists (not `null`/`None`). Downstream JSON parsers do not crash on field access. ### REPAIR — Repair Blind Spots (4 requirements) @@ -83,16 +83,16 @@ | LEGACY-05 | Phase 47 | Complete | | LEGACY-06 | Phase 47 | Complete | | LEGACY-07 | Phase 47 | Complete | -| DEPR-01 | Phase 48 | Pending | -| DEPR-02 | Phase 48 | Pending | -| DEPR-03 | Phase 48 | Pending | -| HARDEN-01 | Phase 49 | Pending | -| HARDEN-02 | Phase 49 | Pending | -| HARDEN-03 | Phase 49 | Pending | -| HARDEN-04 | Phase 49 | Pending | -| HARDEN-05 | Phase 49 | Pending | -| HARDEN-06 | Phase 49 | Pending | -| HARDEN-07 | Phase 49 | Pending | +| DEPR-01 | Phase 48 | Complete | +| DEPR-02 | Phase 48 | Complete | +| DEPR-03 | Phase 48 | Complete | +| HARDEN-01 | Phase 49 | Complete | +| HARDEN-02 | Phase 49 | Complete | +| HARDEN-03 | Phase 49 | Complete | +| HARDEN-04 | Phase 49 | Complete | +| HARDEN-05 | Phase 49 | Complete | +| HARDEN-06 | Phase 49 | Complete | +| HARDEN-07 | Phase 49 | Complete | | REPAIR-01 | Phase 50 | Pending | | REPAIR-02 | Phase 50 | Pending | | REPAIR-03 | Phase 50 | Pending | diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index ae2cd057..0bdf8907 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -245,7 +245,7 @@ Plans: | 46. Index Path Resolution | v1.11 | 2/2 | Complete | 2026-05-07 | | 47. Library-Records Deprecation Cleanup | v1.11 | 1/2 | In Progress| | | 48. Textual TUI Removal | v1.11 | 3/2 | Complete | 2026-05-07 | -| 49. Module Hardening | v1.11 | 0/0 | Not started | - | +| 49. Module Hardening | v1.11 | 1/3 | In Progress| | | 50. Repair Blind Spots | v1.11 | 0/0 | Not started | - | --- diff --git a/.planning/STATE.md b/.planning/STATE.md index c0245bd5..cc881998 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -3,14 +3,14 @@ gsd_state_version: 1.0 milestone: v1.11 milestone_name: Merge Gate — v1.9 Ripple Remediation status: completed -stopped_at: Completed 48 - Textual TUI Removal (48-001, 48-002) -last_updated: "2026-05-07T11:44:47.723Z" -last_activity: 2026-05-07 — Phase 48 executed (DEPR-01 through DEPR-03) +stopped_at: Completed 49 - Module Hardening (HARDEN-01 through HARDEN-07) +last_updated: "2026-05-07T12:07:44.635Z" +last_activity: 2026-05-07 — Phase 48 executed (48-001, 48-002) progress: total_phases: 9 - completed_phases: 0 - total_plans: 6 - completed_plans: 7 + completed_phases: 1 + total_plans: 9 + completed_plans: 10 percent: 20 --- @@ -54,6 +54,7 @@ Progress: [████████░░] 20% *Updated after each plan completion* | Phase 47-library-records-deprecation-cleanup P001-002 | 18min | 4 tasks | 16 files | | Phase 48-textual-tui-removal P001-002 | 15min | 5 tasks | 6 files | +| Phase 49-module-hardening P001-003 | 8 | 6 tasks | 7 files | ## Accumulated Context @@ -81,6 +82,6 @@ Progress: [████████░░] 20% ## Session Continuity -Last session: 2026-05-07T11:44:47.723Z -Stopped at: Completed 48 - Textual TUI Removal (DEPR-01 through DEPR-03) +Last session: 2026-05-07T12:07:44.632Z +Stopped at: Completed 49 - Module Hardening (HARDEN-01 through HARDEN-07) Resume file: None diff --git a/.planning/phases/49-module-hardening/49-SUMMARY.md b/.planning/phases/49-module-hardening/49-SUMMARY.md new file mode 100644 index 00000000..aaea54b9 --- /dev/null +++ b/.planning/phases/49-module-hardening/49-SUMMARY.md @@ -0,0 +1,157 @@ +--- +phase: 49-module-hardening +plan: 001-003 (combined) +wave: 1 +subsystem: worker, plugin +tags: hardening, security, safety +provides: + - HARDEN-01: Cross-process file locking in discussion.py + - HARDEN-02: Markdown escaping in QA text fields + - HARDEN-03: UTC timestamps (not CST/UTC+8) + - HARDEN-04: API key via environment variable (not CLI arg) + - HARDEN-05: createEl() DOM API (no innerHTML XSS vector) + - HARDEN-06: Workspace integrity checks before /pf-deep recommendation + - HARDEN-07: Empty dicts {} instead of None for index aggregates +affects: + - discussion.py: read-modify-write now locked, timestamps UTC, MD escaped + - main.js: spawn uses env var, dir tree uses createEl() + - asset_state.py: next_step ordering corrected + - status.py: JSON output safe for dict consumers +tech-stack: + added: + - filelock library (existing dependency, now used by discussion.py) + patterns: + - filelock.FileLock for cross-process synchronization + - datetime.now(timezone.utc) for canonical timestamps + - PADDLEOCR_API_TOKEN env var for API key transport + - createEl() obsidian DOM API for XSS-safe rendering +key-files: + created: [] + modified: + - paperforge/worker/discussion.py + - paperforge/plugin/main.js + - paperforge/worker/asset_state.py + - paperforge/worker/status.py + - tests/test_discussion.py + - tests/test_asset_state.py + - tests/test_status.py +decisions: + - "filelock.FileLock used over fcntl/msvcrt for cross-platform portability (same pattern as asset_index.py)" + - "CST constant name preserved but now assigned timezone.utc — minimal diff, no downstream impact" + - "Both JSON and MD writes wrapped in single lock scope for read-modify-write consistency" +metrics: + duration: 8 min + completed_date: "2026-05-07" +--- + +# Phase 49 Module Hardening: Summary + +**One-liner:** Production-grade safety guards across discussion.py (locking, escaping, UTC), main.js (env var API key, createEl() DOM), and asset_state/status (reordered checks, empty-safe dicts). + +## Plans Executed + +### Plan 49-001: discussion.py hardening (HARDEN-01, HARDEN-02, HARDEN-03) + +| Task | Name | Commit | Files | +|------|------|--------|-------| +| 1 | UTC timestamps + Markdown escaping | `5a69874` | discussion.py, test_discussion.py | +| 2 | File locking | `5a69874` | discussion.py, test_discussion.py | + +### Plan 49-002: main.js hardening (HARDEN-04, HARDEN-05) + +| Task | Name | Commit | Files | +|------|------|--------|-------| +| 1 | API key via env var | `8b9b366` | main.js | +| 2 | createEl() not innerHTML | `8b9b366` | main.js | + +### Plan 49-003: asset_state.py + status.py hardening (HARDEN-06, HARDEN-07) + +| Task | Name | Commit | Files | +|------|------|--------|-------| +| 1 | Workspace integrity check order | `4e816f9` | asset_state.py, test_asset_state.py | +| 2 | Empty dicts not null | `4e816f9` | status.py, test_status.py | + +## Verification Results + +``` +test_discussion.py ........ 12/12 passed (5 new tests) +test_asset_state.py ....... 28/28 passed (2 new + 1 updated) +test_status.py ............ 6/6 passed (1 assertion updated) +Full suite ................ 485/489 passed, 2 skipped, 2 pre-existing failures +``` + +- **2 pre-existing failures** in `test_ocr_state_machine.py` (unrelated — test OCR state transitions, not modified by any plan) +- **0 regressions** from our changes +- **JS verification:** `--paddleocr-key` absent, `PADDLEOCR_API_TOKEN` present, `innerHTML` eliminated from dir tree, `createEl()` chain present + +## Deviations from Plan + +None — all three plans executed exactly as written. + +## Verification Details + +### HARDEN-01: File locking +- `import filelock` present in discussion.py ✅ +- `filelock.FileLock` used with `.json.lock` suffix ✅ +- Lock timeout = 10s ✅ +- Both JSON and MD writes inside same lock scope ✅ +- `test_file_lock_prevents_concurrent_write` passes ✅ +- `test_lock_timeout_returns_error` passes ✅ + +### HARDEN-02: Markdown escaping +- `_escape_md()` helper escapes `*`, `#`, `[`, `]`, `_`, `` ` `` via regex ✅ +- Applied to both question and answer in `_build_md_session()` ✅ +- `test_markdown_escaping` passes ✅ +- `test_markdown_escaping_cjk` passes ✅ +- Idempotent (does not double-escape) via regex pattern ✅ + +### HARDEN-03: UTC timestamps +- `_CST = timezone.utc` (replaced `timedelta(hours=8)`) ✅ +- `_now_iso()` docstring updated ✅ +- `test_utc_timestamp` passes (ends with `+00:00`) ✅ +- No `timedelta(hours=8)` remains in discussion.py ✅ + +### HARDEN-04: API key via env var +- CLI arg `--paddleocr-key` removed from setupArgs ✅ +- `PADDLEOCR_API_TOKEN` present in spawn options ✅ +- `env: { ...process.env, PADDLEOCR_API_TOKEN: ... }` pattern ✅ +- API key not visible in process listing ✅ + +### HARDEN-05: createEl() DOM API +- `tree.innerHTML = ...` replaced with `tree.createEl()` chain ✅ +- `textContent` used instead of innerHTML for user-supplied values ✅ +- XSS vector from directory names eliminated ✅ + +### HARDEN-06: Workspace integrity before /pf-deep +- note_path check moved before /pf-deep check ✅ +- workspace_paths check moved before /pf-deep check ✅ +- Docstring updated to reflect new ordering ✅ +- `test_missing_note_blocks_pf_deep` returns "sync" ✅ +- `test_missing_workspace_path_blocks_pf_deep` returns "sync" ✅ + +### HARDEN-07: Empty dicts not null +- `lifecycle_level_counts = {}` (was `None`) ✅ +- `health_aggregate = {}` (was `None`) ✅ +- `maturity_distribution = {}` (was `None`) ✅ +- Comment updated ✅ +- `test_status_json_fallback_when_index_missing` asserts `== {}` ✅ + +## Known Stubs + +None identified. + +## Self-Check: PASSED + +- [x] All modified files verified: discussion.py, main.js, asset_state.py, status.py, 3 test files +- [x] All 3 commits exist in git log +- [x] Tests pass for all modified modules +- [x] Full suite: no regressions (2 pre-existing failures unrelated) +- [x] JS verification clean + +## Self-Check + +- [x] All modified files verified: discussion.py, main.js, asset_state.py, status.py, 3 test files +- [x] All 3 commits exist in git log +- [x] Tests pass for all modified modules +- [x] Full suite: no regressions (2 pre-existing failures unrelated) +- [x] JS verification clean diff --git a/.planning/phases/49-module-hardening/49-VERIFICATION.md b/.planning/phases/49-module-hardening/49-VERIFICATION.md new file mode 100644 index 00000000..c23642b3 --- /dev/null +++ b/.planning/phases/49-module-hardening/49-VERIFICATION.md @@ -0,0 +1,90 @@ +--- +phase: 49-module-hardening +plans: 001, 002, 003 +requirements: HARDEN-01 through HARDEN-07 +status: PASSED +--- + +# Phase 49: Module Hardening — Verification Report + +## Summary + +**Result: PASSED** — 7/7 HARDEN requirements verified. All automated checks green. + +| Requirement | Status | Evidence | +|-------------|--------|----------| +| HARDEN-01 | PASS | filelock.FileLock wraps JSON+MD writes; 2 tests pass | +| HARDEN-02 | PASS | _escape_md() escapes 6 special chars; 2 tests pass | +| HARDEN-03 | PASS | timezone.utc replaces CST; 1 test passes | +| HARDEN-04 | PASS | --paddleocr-key removed, PADDLEOCR_API_TOKEN in env | +| HARDEN-05 | PASS | innerHTML removed, createEl() chain present | +| HARDEN-06 | PASS | note_path/workspace_paths checked before /pf-deep | +| HARDEN-07 | PASS | {} instead of None for 3 dict fields | + +## Automated Test Results + +### Plan 49-001: discussion.py +``` +tests/test_discussion.py::TestRecordSession::test_create_both_files PASSED +tests/test_discussion.py::TestRecordSession::test_append_second_session PASSED +tests/test_discussion.py::TestRecordSession::test_utc_timestamp PASSED [NEW] +tests/test_discussion.py::TestRecordSession::test_markdown_escaping PASSED [NEW] +tests/test_discussion.py::TestRecordSession::test_markdown_escaping_cjk PASSED [NEW] +tests/test_discussion.py::TestRecordSession::test_file_lock_prevents_concurrent_write PASSED [NEW] +tests/test_discussion.py::TestRecordSession::test_lock_timeout_returns_error PASSED [NEW] +tests/test_discussion.py::TestRecordSession::test_missing_vault PASSED +tests/test_discussion.py::TestRecordSession::test_unknown_key PASSED +tests/test_discussion.py::TestRecordSession::test_cjk_encoding PASSED +tests/test_discussion.py::TestRecordSession::test_atomic_write_no_partial PASSED +tests/test_discussion.py::TestRecordSession::test_cli_invocation PASSED +``` +**12/12 passed** (7 existing + 5 new) + +### Plan 49-002: main.js +``` +JS static verification: + --paddleocr-key present: false [OK] + PADDLEOCR_API_TOKEN present: true [OK] + innerHTML (dir tree): absent [OK] + createEl dir-tree present: true [OK] +``` +**4/4 checks passed** (syntax-only verification) + +### Plan 49-003: asset_state.py + status.py +``` +tests/test_asset_state.py ... 28/28 passed (2 new, 1 updated) +tests/test_status.py ......... 6/6 passed (1 assertion updated) +``` +**34/34 passed** + +### Full Regression Suite +``` +485 passed, 2 skipped, 2 failed +``` +The 2 failures are pre-existing in `test_ocr_state_machine.py` (unrelated): +- `test_retry_exhaustion_becomes_error` — expects "error" but OCR returns "blocked" +- `test_full_cycle_from_pending_to_done` — expects "done" but OCR returns "queued" + +**0 regressions from Phase 49 changes.** + +## Verification Commands Executed + +```bash +python -m pytest tests/test_discussion.py -v --tb=short +python -m pytest tests/test_asset_state.py -v --tb=short +python -m pytest tests/test_status.py -v --tb=short +python -m pytest tests/ -q --tb=short +node -e "const fs=require('fs');const c=fs.readFileSync('paperforge/plugin/main.js','utf-8');..." +``` + +## Manual Verification Notes + +- **main.js**: Syntax verified via Node.js require. Full Obsidian plugin runtime verification requires user testing in Obsidian. +- **No linting violations** observed (ruff check passed for Python files). + +## Gap Items + +| Gap | Severity | Note | +|-----|----------|------| +| OCR state machine pre-existing failures | Low | Unrelated to Phase 49; existed before changes | +| main.js runtime not tested | Low | Cannot automate Obsidian plugin testing in CI |