obsidian-plugin/lllin000_PaperForge
1
0
Fork 0
mirror of https://github.com/lllin000/PaperForge.git synced 2026-07-22 06:50:53 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

docs(49-module-hardening): create 3 wave-1 plans covering all 7 HARDEN requirements

Browse source
This commit is contained in:
Research Assistant 2026-05-07 19:56:48 +08:00
parent 3331676c5d
commit 39bdcd8650
4 changed files with 1008 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
.planning/ROADMAP.md
View file

@ -202,7 +202,12 @@ Plans:
4. Obsidian plugin spawns OCR subprocess with `PADDLEOCR_API_TOKEN` in environment variable (not command-line argument) — API key not visible in process list via Task Manager
5. Plugin directory tree renders via `createEl()` DOM API (not `innerHTML` assignment) — no XSS vector from user-configured directory names containing HTML/script tags
6. `paperforge status --json` returns `lifecycle_level_counts`, `health_aggregate`, and `maturity_distribution` as empty dicts `{}` (not `null`) when no canonical index exists — downstream JSON parsers do not crash on field access
**Plans**: TBD
**Plans**: 3 plans
Plans:
- [ ] `49-001-PLAN.md` — discussion.py hardening: UTC timestamps (HARDEN-03), markdown escaping (HARDEN-02), file locking (HARDEN-01)
- [ ] `49-002-PLAN.md` — main.js hardening: API key via env var (HARDEN-04), createEl() not innerHTML (HARDEN-05)
- [ ] `49-003-PLAN.md` — asset_state.py + status.py hardening: reorder next_step checks (HARDEN-06), empty dicts not null (HARDEN-07)
**UI hint**: yes
### Phase 50: Repair Blind Spots

402
.planning/phases/49-module-hardening/49-001-PLAN.md Normal file
View file

@ -0,0 +1,402 @@
---
phase: 49-module-hardening
plan: 001
type: execute
wave: 1
depends_on: []
files_modified:
- paperforge/worker/discussion.py
- tests/test_discussion.py
autonomous: true
requirements:
- HARDEN-01
- HARDEN-02
- HARDEN-03
must_haves:
truths:
- "Three timestamps in discussion.json use UTC (no CST/UTC+8 hardcoded offset)"
- "Markdown special characters (*, #, [, _, `) in QA question/answer fields are escaped before writing to discussion.md"
- "Two concurrent /pf-paper calls for the same paper do not corrupt discussion.json or discussion.md"
artifacts:
- path: paperforge/worker/discussion.py
provides: "UTC timestamps, markdown escaping, file locking"
min_lines: 1
- path: tests/test_discussion.py
provides: "Tests for UTC timestamp, markdown escaping, locking"
min_lines: 1
key_links:
- from: discussion.py:_now_iso()
to: "timezone.utc"
via: "datetime.now(timezone.utc)"
pattern: "datetime\\.now\\(timezone\\.utc\\)"
- from: discussion.py:_build_md_session()
to: "_escape_md()"
via: "escape call on question/answer"
pattern: "_escape_md\\(qa\\["
- from: discussion.py:record_session()
to: "filelock.FileLock"
via: "lock around JSON+MD read-modify-write"
pattern: "filelock\\.FileLock"
---
<objective>
Add production-grade safety guards to `discussion.py`: UTC timestamps (HARDEN-03), markdown escaping (HARDEN-02), and file-level locking (HARDEN-01).
Purpose: Eliminate timezone inconsistency in discussion.json, prevent broken Obsidian formatting from unescaped markdown characters, and prevent concurrent process write corruption.
Output:
- Modified `paperforge/worker/discussion.py` with all three hardening measures
- Modified `tests/test_discussion.py` with tests for the new behavior
</objective>
<execution_context>
@C:/Users/Lin/.opencode/get-shit-done/workflows/execute-plan.md
@C:/Users/Lin/.opencode/get-shit-done/templates/summary.md
</execution_context>
<context>
@.planning/PROJECT.md
@.planning/ROADMAP.md
@.planning/STATE.md
<interfaces>
From paperforge/worker/asset_index.py (established filelock pattern):
```python
import filelock
LOCK_TIMEOUT = 10
lock_path = path.with_suffix(".json.lock")
lock = filelock.FileLock(lock_path, timeout=LOCK_TIMEOUT)
with lock:
# read-modify-write
```
From tests/test_discussion.py (existing test helpers):
```python
def _sample_qa_pairs() -> list[dict]:
return [
{"question": "What is the primary outcome?", "answer": "The primary outcome is biomechanical strength.",
"source": "user_question", "timestamp": "2026-05-06T12:00:00+08:00"},
]
```
</interfaces>
</context>
<tasks>
<task type="auto" tdd="true">
<name>Task 1: Fix UTC timestamps (HARDEN-03) + add markdown escaping (HARDEN-02)</name>
<files>paperforge/worker/discussion.py, tests/test_discussion.py</files>
<behavior>
- Test 1 (UTC): `record_session()` returns `started` timestamp ending with `+00:00` in discussion.json
- Test 2 (escape): `record_session()` with QA containing `*` `#` `[` `_` `` ` `` produces escaped sequences in discussion.md (no bare `*text*` bold rendering)
- Test 3 (escape CJK): CJK characters in QA fields are preserved alongside escaped markdown chars
- Test 4 (idempotent escape): Already-escaped characters like `\*` are not double-escaped
</behavior>
<action>
=== discussion.py changes (HARDEN-03) ===
Line 40: Replace `_CST = timezone(timedelta(hours=8))` with `_CST = timezone.utc`
Line 48-50: Update `_now_iso()` docstring:
Old: `"""Return current time in ISO 8601 with timezone (CST, UTC+8)."""`
New: `"""Return current time in ISO 8601 with UTC timezone."""`
=== discussion.py changes (HARDEN-02) ===
After `_MD_SEPARATOR` constant (around line 38-39), add:
```python
_MD_SPECIAL_CHARS = re.compile(r'([*#\[\]_`])')
"""Regex for markdown special characters that must be escaped in QA text fields."""
```
Between `_MD_SEPARATOR` and `_now_iso()`, add helper function:
```python
def _escape_md(text: str) -> str:
"""Escape markdown special characters in QA text fields.
Escapes: *, #, [, ], _, ` — prevents broken Obsidian formatting
when user questions or AI answers contain these characters.
Does not double-escape already-escaped characters.
"""
return _MD_SPECIAL_CHARS.sub(r'\\\1', text)
```
In `_build_md_session()` (lines 169-171), wrap question and answer with `_escape_md()`:
```python
lines.append(f"**问题:** {_escape_md(qa['question'])}")
lines.append(f"**解答:** {_escape_md(qa['answer'])}\n")
```
=== test_discussion.py changes ===
Add import for `re` at top if not present.
In `TestRecordSession`, add:
1. `test_utc_timestamp()` — Verify `started` field ends with `+00:00`
```python
def test_utc_timestamp(self, tmp_path: Path) -> None:
"""HARDEN-03: started timestamp uses UTC (not CST/UTC+8)."""
vault = _create_minimal_vault(tmp_path)
result = record_session(
vault_path=vault, zotero_key="TSTONE001",
agent="pf-paper", model="gpt-4", qa_pairs=_sample_qa_pairs(),
)
assert result["status"] == "ok"
data = json.loads(Path(result["json_path"]).read_text(encoding="utf-8"))
started = data["sessions"][0]["started"]
# UTC offset is either +00:00 or Z
assert started.endswith("+00:00") or started.endswith("Z"), f"Expected UTC, got: {started}"
```
2. `test_markdown_escaping()` — Verify special chars escaped in MD output
```python
def test_markdown_escaping(self, tmp_path: Path) -> None:
"""HARDEN-02: Markdown special chars in QA are escaped in discussion.md."""
vault = _create_minimal_vault(tmp_path)
qa = [
{
"question": "What does *italic* and **bold** mean?",
"answer": "Use # for headings and [link](url) syntax _underscore_ `code`",
"source": "user_question",
"timestamp": "2026-05-06T12:00:00+00:00",
},
]
result = record_session(
vault_path=vault, zotero_key="TSTONE001",
agent="pf-paper", model="gpt-4", qa_pairs=qa,
)
assert result["status"] == "ok"
md = Path(result["md_path"]).read_text(encoding="utf-8")
# Verify escaped forms exist (not bare markdown)
assert "\\*italic\\*" in md, f"Expected escaped asterisks in: {md}"
assert "\\*\\*bold\\*\\*" in md
assert "\\# " in md or "\\#" in md
assert "\\_" in md
assert "\\`" in md
```
3. `test_markdown_escaping_cjk()` — CJK preserved alongside escaped chars
```python
def test_markdown_escaping_cjk(self, tmp_path: Path) -> None:
"""HARDEN-02: CJK characters preserved alongside escaped markdown chars."""
vault = _create_minimal_vault(tmp_path, title="中文测试")
qa = [
{
"question": "什么是*p值*和#显著性？",
"answer": "_效应量_为0.5`[95% CI]",
"source": "user_question",
"timestamp": "2026-05-06T12:00:00+00:00",
},
]
result = record_session(
vault_path=vault, zotero_key="TSTONE001",
agent="pf-paper", model="gpt-4", qa_pairs=qa,
)
assert result["status"] == "ok"
md = Path(result["md_path"]).read_text(encoding="utf-8")
# CJK preserved
assert "什么是" in md
assert "效应量" in md
# Special chars escaped
assert "\\*p值\\*" in md or "\\*p\\u5024\\*" in md
assert "\\_" in md
assert "\\`" in md
```
</action>
<verify>
<automated>cd /D "D:\L\Med\Research\99_System\LiteraturePipeline\github-release" && python -m pytest tests/test_discussion.py -v -x --tb=short 2>&1</automated>
</verify>
<done>
- All 4 new tests pass (UTC timestamp, markdown escape, CJK escape, idempotent)
- All existing tests in test_discussion.py still pass (9 existing tests)
- Total: 13+ tests passing
</done>
</task>
<task type="auto" tdd="true">
<name>Task 2: Add file-level locking to discussion.py record_session() (HARDEN-01)</name>
<files>paperforge/worker/discussion.py, tests/test_discussion.py</files>
<behavior>
- Test 1: `filelock.FileLock` is called with the correct lock path (`.json.lock` suffix) during `record_session()`
- Test 2: `record_session()` correctly handles `filelock.Timeout` by returning error status
- Test 3: Existing atomic-write tests continue to pass (no regression)
</behavior>
<action>
=== discussion.py changes (HARDEN-01) ===
Add `import filelock` to imports (after `import tempfile`, before `from datetime`):
Add `LOCK_TIMEOUT` constant after _MD_SEPARATOR / _MD_SPECIAL_CHARS:
```python
LOCK_TIMEOUT = 10
"""Seconds to wait for cross-process file lock before raising filelock.Timeout."""
```
Replace the two separate try blocks for JSON write (lines 281-299) and MD write (lines 302-322) with a single locked block:
Old code (lines 280-322):
```python
# --- Write discussion.json (atomic, append) ---
try:
existing_sessions = []
if json_path.exists():
...
envelope = { ... }
_atomic_write_json(json_path, envelope)
except Exception as exc:
logger.warning("Failed to write discussion.json: %s", exc)
return {"status": "error", "message": f"Failed to write discussion.json: {exc}"}
# --- Write discussion.md (atomic, append) ---
try:
existing_md = ""
if md_path.exists():
...
header = _build_md_header(paper_title)
session_md = _build_md_session(session)
content = _md_content(existing_md, header, session_md)
_atomic_write_md(md_path, content)
except Exception as exc:
logger.warning("Failed to write discussion.md: %s", exc)
return {"status": "error", "message": f"Failed to write discussion.md: {exc}",
"json_path": str(json_path)}
```
New code:
```python
# --- Write discussion.json + discussion.md (atomic, locked) ---
# HARDEN-01: Cross-process file lock prevents concurrent write corruption.
# Uses same filelock pattern as asset_index.py: lock on .json.lock suffix,
# timeout after LOCK_TIMEOUT seconds.
lock_path = json_path.with_suffix(".json.lock")
lock = filelock.FileLock(lock_path, timeout=LOCK_TIMEOUT)
try:
with lock:
# Write discussion.json
existing_sessions = []
if json_path.exists():
try:
existing_data = json.loads(json_path.read_text(encoding="utf-8"))
existing_sessions = existing_data.get("sessions", [])
except (json.JSONDecodeError, OSError) as exc:
logger.warning("Corrupted discussion.json, starting fresh: %s", exc)
existing_sessions = []
envelope = {
"schema_version": "1",
"paper_key": zotero_key,
"sessions": [*existing_sessions, session],
}
_atomic_write_json(json_path, envelope)
# Write discussion.md
existing_md = ""
if md_path.exists():
try:
existing_md = md_path.read_text(encoding="utf-8")
except (OSError, UnicodeDecodeError) as exc:
logger.warning("Could not read existing discussion.md: %s", exc)
existing_md = ""
header = _build_md_header(paper_title)
session_md = _build_md_session(session)
content = _md_content(existing_md, header, session_md)
_atomic_write_md(md_path, content)
except filelock.Timeout:
logger.warning("Could not acquire lock for discussion files: %s", lock_path)
return {"status": "error",
"message": "Concurrent access conflict. Please try again."}
except Exception as exc:
logger.warning("Failed to write discussion files: %s", exc)
return {"status": "error",
"message": f"Failed to write discussion files: {exc}"}
```
=== test_discussion.py changes ===
Add import at top:
```python
import filelock
```
Add test for lock file creation:
```python
def test_file_lock_prevents_concurrent_write(self, tmp_path: Path) -> None:
"""HARDEN-01: File lock (.json.lock) is created during record_session()."""
vault = _create_minimal_vault(tmp_path)
result = record_session(
vault_path=vault, zotero_key="TSTONE001",
agent="pf-paper", model="gpt-4", qa_pairs=_sample_qa_pairs(),
)
assert result["status"] == "ok"
json_path = Path(result["json_path"])
# Lock file should NOT remain after successful write (released + cleaned up)
lock_path = json_path.with_suffix(".json.lock")
assert not lock_path.exists(), "Lock file should be released after write"
```
Add test for concurrent safety (simulate by holding a lock):
```python
def test_lock_timeout_returns_error(self, tmp_path: Path) -> None:
"""HARDEN-01: When lock cannot be acquired, returns error status."""
vault = _create_minimal_vault(tmp_path)
# First call to create the json file
r1 = record_session(
vault_path=vault, zotero_key="TSTONE001",
agent="pf-paper", model="gpt-4", qa_pairs=_sample_qa_pairs(),
)
assert r1["status"] == "ok"
json_path = Path(r1["json_path"])
# Hold an exclusive lock to simulate concurrent access
lock_path = json_path.with_suffix(".json.lock")
external_lock = filelock.FileLock(lock_path, timeout=1)
with external_lock:
# This should fail because lock is held
r2 = record_session(
vault_path=vault, zotero_key="TSTONE001",
agent="pf-paper", model="gpt-4", qa_pairs=_sample_qa_pairs(),
)
assert r2["status"] == "error"
assert "Concurrent access" in r2.get("message", "")
```
</action>
<verify>
<automated>cd /D "D:\L\Med\Research\99_System\LiteraturePipeline\github-release" && python -m pytest tests/test_discussion.py -v -x --tb=short 2>&1</automated>
</verify>
<done>
- All new locking tests pass (lock creation, timeout handling)
- All previous discussion tests still pass (UTC, escape, atomic, append, CJK, CLI)
- Total: 15+ tests passing
</done>
</task>
</tasks>
<verification>
1. `python -m pytest tests/test_discussion.py -v --tb=short` — ALL tests pass (15+)
2. `python -m pytest tests/ -q --tb=short` — full suite no regressions
3. Verify no `timedelta(hours=8)` or `+08:00` remains in discussion.py
4. Verify `import filelock` present in discussion.py
5. Verify `_escape_md` is called in `_build_md_session()` for both question and answer
</verification>
<success_criteria>
- [ ] `paperforge/worker/discussion.py` has `import filelock` and uses `filelock.FileLock` around JSON+MD read-modify-write
- [ ] `paperforge/worker/discussion.py` has `_escape_md()` helper called in `_build_md_session()`
- [ ] `paperforge/worker/discussion.py` uses `timezone.utc` (no `timedelta(hours=8)`)
- [ ] All tests in `tests/test_discussion.py` pass (existing + new)
- [ ] Full test suite passes with zero regressions
</success_criteria>
<output>
After completion, create `.planning/phases/49-module-hardening/49-001-SUMMARY.md`
</output>

236
.planning/phases/49-module-hardening/49-002-PLAN.md Normal file
View file

@ -0,0 +1,236 @@
---
phase: 49-module-hardening
plan: 002
type: execute
wave: 1
depends_on: []
files_modified:
- paperforge/plugin/main.js
autonomous: true
requirements:
- HARDEN-04
- HARDEN-05
must_haves:
truths:
- "Obsidian plugin spawns OCR subprocess with PADDLEOCR_API_TOKEN in environment variable (not CLI argument) — API key not visible in Task Manager process list"
- "Plugin directory tree renders via createEl() DOM API (not innerHTML assignment) — no XSS vector from user-configured directory names containing HTML/script tags"
artifacts:
- path: paperforge/plugin/main.js
provides: "Env-based API key passing and createEl() DOM rendering"
min_lines: 1
key_links:
- from: main.js:_runInstall()
to: "spawn(..., { env: { ...process.env, PADDLEOCR_API_TOKEN } })"
via: "env property in spawn options"
pattern: "PADDLEOCR_API_TOKEN"
- from: main.js:_stepOverview()
to: "createEl() API chain"
via: "DOM API instead of innerHTML"
pattern: "createEl\\(.*paperforge-dir"
---
<objective>
Harden the Obsidian plugin (`main.js`) with two safety measures: pass the PaddleOCR API key through environment variables instead of CLI arguments (HARDEN-04), and replace `innerHTML` DOM assignment with `createEl()` DOM API calls to eliminate XSS vectors (HARDEN-05).
Purpose: Prevent API key leakage via process listing (Task Manager / ps), and prevent XSS injection through user-configured directory names.
Output:
- Modified `paperforge/plugin/main.js` with both hardening measures
</objective>
<execution_context>
@C:/Users/Lin/.opencode/get-shit-done/workflows/execute-plan.md
@C:/Users/Lin/.opencode/get-shit-done/templates/summary.md
</execution_context>
<context>
@.planning/PROJECT.md
@.planning/ROADMAP.md
@.planning/STATE.md
<interfaces>
From paperforge/plugin/main.js (current spawn pattern, line 2088-2093):
```js
const runPython = (args, options = {}) => new Promise((resolve, reject) => {
const child = spawn('python', args, {
cwd: s.vault_path.trim(),
env: process.env, // base env, overridden by ...options
timeout: 120000,
...options, // options.env replaces env above if provided
});
...
});
```
Current setupArgs (lines 2112-2123):
```js
const setupArgs = [
'-m', 'paperforge',
'--vault', s.vault_path.trim(),
'setup', '--headless',
'--paddleocr-key', s.paddleocr_api_key.trim(), // <-- HARDEN-04: REMOVE
'--system-dir', s.system_dir.trim(),
'--resources-dir', s.resources_dir.trim(),
'--literature-dir', s.literature_dir.trim(),
'--control-dir', s.control_dir.trim(),
'--base-dir', s.base_dir.trim(),
'--agent', s.agent_platform || 'opencode',
];
```
Current innerHTML block (lines 1815-1825):
```js
tree.innerHTML = `
<div class="paperforge-dir-node root">📁 Vault (${vault})</div>
<div class="paperforge-dir-children">
<div class="paperforge-dir-node folder">📁 ${s.resources_dir || 'Resources'}/ — description
<div class="paperforge-dir-children">
<div class="paperforge-dir-node file">📁 ${s.literature_dir || 'Literature'}/ — description</div>
</div>
</div>
<div class="paperforge-dir-node folder">📁 ${s.base_dir || 'Bases'}/ — description</div>
<div class="paperforge-dir-node folder">📁 ${s.system_dir || 'System'}/ — description</div>
</div>`;
```
</interfaces>
</context>
<tasks>
<task type="auto">
<name>Task 1: Pass PADDLEOCR_API_TOKEN via environment variable, not CLI argument (HARDEN-04)</name>
<files>paperforge/plugin/main.js</files>
<action>
=== Change 1: Remove --paddleocr-key from setupArgs ===
Lines 2112-2123: Remove the line `'--paddleocr-key', s.paddleocr_api_key.trim(),` from the `setupArgs` array.
Old:
```js
const setupArgs = [
'-m', 'paperforge',
'--vault', s.vault_path.trim(),
'setup', '--headless',
'--paddleocr-key', s.paddleocr_api_key.trim(),
'--system-dir', s.system_dir.trim(),
...
];
```
New:
```js
const setupArgs = [
'-m', 'paperforge',
'--vault', s.vault_path.trim(),
'setup', '--headless',
'--system-dir', s.system_dir.trim(),
...
];
```
=== Change 2: Pass API key via environment in spawn() ===
Line 2144: Change the `runPython` call to pass the API token in the environment.
Old:
```js
await runPython(setupArgs, { logStdout: true });
```
New:
```js
await runPython(setupArgs, {
logStdout: true,
env: { ...process.env, PADDLEOCR_API_TOKEN: s.paddleocr_api_key.trim() },
});
```
Note: The `env` property in options overrides the default `env: process.env` from `runPython`'s base spawn options. Spreading `...process.env` preserves all existing environment variables, and `PADDLEOCR_API_TOKEN` adds the API key without exposing it in the process command line.
</action>
<verify>
<automated>cd /D "D:\L\Med\Research\99_System\LiteraturePipeline\github-release" && node -e "const fs=require('fs');const c=fs.readFileSync('paperforge/plugin/main.js','utf-8');console.log('--paddleocr-key present:',c.includes('--paddleocr-key'));console.log('PADDLEOCR_API_TOKEN present:',c.includes('PADDLEOCR_API_TOKEN'));process.exit(c.includes('--paddleocr-key')?1:0)"</automated>
</verify>
<done>
- `--paddleocr-key` no longer appears in main.js
- `PADDLEOCR_API_TOKEN` appears in main.js in the env block
- No CLI argument exposes the API key for process listing
</done>
</task>
<task type="auto">
<name>Task 2: Replace innerHTML with createEl() DOM API for directory tree (HARDEN-05)</name>
<files>paperforge/plugin/main.js</files>
<action>
Lines 1815-1825: Replace the innerHTML template literal with createEl() DOM API calls.
Replace this block (lines 1815-1825):
```js
tree.innerHTML = `
<div class="paperforge-dir-node root">📁 Vault (${vault})</div>
<div class="paperforge-dir-children">
<div class="paperforge-dir-node folder">📁 ${s.resources_dir || 'Resources'}/ — 文献卡片目录（Base 数据来源）
<div class="paperforge-dir-children">
<div class="paperforge-dir-node file">📁 ${s.literature_dir || 'Literature'}/ — 文献卡片</div>
</div>
</div>
<div class="paperforge-dir-node folder">📁 ${s.base_dir || 'Bases'}/ — 数据管理面板</div>
<div class="paperforge-dir-node folder">📁 ${s.system_dir || 'System'}/ — Zotero 软链接 + PaperForge 系统文件夹</div>
</div>`;
```
With this createEl() chain:
```js
// HARDEN-05: Use createEl() DOM API instead of innerHTML to prevent XSS
// from user-configured directory names containing HTML/script tags.
const rootNode = tree.createEl('div', { cls: 'paperforge-dir-node root' });
rootNode.textContent = `📁 Vault (${vault})`;
const children = tree.createEl('div', { cls: 'paperforge-dir-children' });
const resourcesFolder = children.createEl('div', { cls: 'paperforge-dir-node folder' });
resourcesFolder.textContent = `📁 ${s.resources_dir || 'Resources'}/ — 文献卡片目录（Base 数据来源）`;
const resourcesChildren = resourcesFolder.createEl('div', { cls: 'paperforge-dir-children' });
resourcesChildren.createEl('div', { cls: 'paperforge-dir-node file',
text: `📁 ${s.literature_dir || 'Literature'}/ — 文献卡片` });
children.createEl('div', { cls: 'paperforge-dir-node folder',
text: `📁 ${s.base_dir || 'Bases'}/ — 数据管理面板` });
children.createEl('div', { cls: 'paperforge-dir-node folder',
text: `📁 ${s.system_dir || 'System'}/ — Zotero 软链接 + PaperForge 系统文件夹` });
```
Key points:
- `createEl()` appends the new element to the parent (same as `el.appendChild(document.createElement(...))`)
- `textContent` (or `createEl`'s `text` property) is used instead of `innerHTML`, which means user-supplied directory names like `<script>alert('xss')</script>` are rendered as literal text, not executed
- The `text` property on `createEl()` sets `textContent` internally, same protection
- The visual tree structure is preserved exactly: same CSS classes, same content
</action>
<verify>
<automated>cd /D "D:\L\Med\Research\99_System\LiteraturePipeline\github-release" && node -e "const fs=require('fs');const c=fs.readFileSync('paperforge/plugin/main.js','utf-8');console.log('innerHTML present:',c.includes('tree.innerHTML='));console.log('createEl dir-tree present:',c.includes('paperforge-dir-node'));process.exit(c.includes('tree.innerHTML=')?1:0)"</automated>
</verify>
<done>
- `tree.innerHTML` no longer appears in main.js (directory tree section)
- Directory tree renders via `createEl()` chain
- XSS vector from user-configured directory names eliminated
</done>
</task>
</tasks>
<verification>
1. Verify no `tree.innerHTML` exists in paperforge/plugin/main.js — grep returns zero hits
2. Verify `PADDLEOCR_API_TOKEN` env var pattern present in spawn() call
3. Verify `--paddleocr-key` no longer appears in main.js
</verification>
<success_criteria>
- [ ] `paperforge/plugin/main.js` passes PADDLEOCR_API_TOKEN via `env:` in spawn(), not `--paddleocr-key` CLI arg
- [ ] `paperforge/plugin/main.js` renders directory tree via `createEl()` (no `innerHTML` in that section)
- [ ] No XSS vector from user-configured directory names containing HTML/script tags
- [ ] No API key exposure in process list
</success_criteria>
<output>
After completion, create `.planning/phases/49-module-hardening/49-002-SUMMARY.md`
</output>

364
.planning/phases/49-module-hardening/49-003-PLAN.md Normal file
View file

@ -0,0 +1,364 @@
---
phase: 49-module-hardening
plan: 003
type: execute
wave: 1
depends_on: []
files_modified:
- paperforge/worker/asset_state.py
- paperforge/worker/status.py
- tests/test_asset_state.py
- tests/test_status.py
autonomous: true
requirements:
- HARDEN-06
- HARDEN-07
must_haves:
truths:
- "Workspace integrity checks (note_path, workspace paths) are performed before returning '/pf-deep' — no broken next_step recommendation"
- "paperforge status --json returns lifecycle_level_counts, health_aggregate, maturity_distribution as empty dicts {} (not null) when no canonical index exists"
artifacts:
- path: paperforge/worker/asset_state.py
provides: "Reordered next_step checks — workspace integrity before /pf-deep"
min_lines: 1
- path: paperforge/worker/status.py
provides: "Empty dict {} instead of None for index-derived aggregates"
min_lines: 1
- path: tests/test_asset_state.py
provides: "Updated tests for new next_step ordering"
min_lines: 1
- path: tests/test_status.py
provides: "Updated test for {} instead of None"
min_lines: 1
key_links:
- from: asset_state.py:compute_next_step()
to: "integrity checks before /pf-deep return"
via: "moved note_path/workspace_paths checks"
pattern: "note_path.*sync.*workspace_paths.*sync.*/pf-deep"
- from: status.py:687-690
to: "{} (empty dict) instead of None"
via: "initialization with {}"
pattern: "lifecycle_level_counts = \\{\\}"
---
<objective>
Fix two production safety gaps: reorder next-step checks in `asset_state.py` to validate workspace integrity before recommending `/pf-deep` (HARDEN-06), and initialize index-derived aggregates in `status.py` as empty dicts `{}` instead of `None` (HARDEN-07).
Purpose: Prevent recommending deep-read when workspace paths are missing, and prevent null-pointer crashes in downstream JSON consumers.
Output:
- Modified `paperforge/worker/asset_state.py` — reordered checks
- Modified `paperforge/worker/status.py` — safe empty dict initialization
- Modified `tests/test_asset_state.py` — updated test expectations
- Modified `tests/test_status.py` — updated test expectations
</objective>
<execution_context>
@C:/Users/Lin/.opencode/get-shit-done/workflows/execute-plan.md
@C:/Users/Lin/.opencode/get-shit-done/templates/summary.md
</execution_context>
<context>
@.planning/PROJECT.md
@.planning/ROADMAP.md
@.planning/STATE.md
<interfaces>
Current asset_state.py:compute_next_step() structure (lines 187-243):
```python
def compute_next_step(entry: dict) -> str:
# 1. OCR error state → ocr
if ocr_status in ("failed", "blocked", "done_incomplete"):
return "ocr"
# 2. No PDF → sync
if not has_pdf:
return "sync"
# 3. PDF, OCR not started → ocr
if ocr_status in ("pending", "nopdf"):
return "ocr"
# 4. OCR processing → ready
if ocr_status == "processing":
return "ready"
# 5. OCR done, deep_read pending → /pf-deep <-- should be AFTER integrity
if ocr_status == "done" and deep_reading_status == "pending":
return "/pf-deep"
# 6. Formal note missing → sync <-- should be BEFORE /pf-deep
if not note_path:
return "sync"
# 7. Workspace path missing → sync <-- should be BEFORE /pf-deep
workspace_paths = [...]
if not all(workspace_paths):
return "sync"
# 8. Ready
return "ready"
```
Current status.py initialization (lines 683-690):
```python
lifecycle_level_counts = None # should be {}
health_aggregate = None # should be {}
maturity_distribution = None # should be {}
if summary is not None:
lifecycle_level_counts = summary["lifecycle_level_counts"]
health_aggregate = summary["health_aggregate"]
maturity_distribution = summary["maturity_distribution"]
```
</interfaces>
</context>
<tasks>
<task type="auto" tdd="true">
<name>Task 1: Reorder workspace integrity checks before /pf-deep in asset_state.py (HARDEN-06)</name>
<files>paperforge/worker/asset_state.py, tests/test_asset_state.py</files>
<behavior>
- Test 1 (existing, must still pass): OCR error → "ocr", no PDF → "sync", OCR pending → "ocr", OCR processing → "ready", all done → "ready"
- Test 2 (existing, must update): OCR done, deep_read pending, WITH note_path AND workspace paths → "/pf-deep"
- Test 3 (new): OCR done, deep_read pending, BUT empty note_path → "sync" (not "/pf-deep")
- Test 4 (new): OCR done, deep_read pending, note_path present BUT missing workspace path → "sync" (not "/pf-deep")
</behavior>
<action>
=== asset_state.py changes (HARDEN-06) ===
Move the "note_path" check (currently step 6, lines 228-230) and "workspace_paths" check (currently step 7, lines 232-240) BEFORE the "/pf-deep" check (currently step 5, lines 224-226).
Old order (lines 208-243):
```python
# ... steps 1-4 (ocr error → ocr, no pdf → sync, ocr not started → ocr, processing → ready) ...
# 5. OCR done, deep reading pending → /pf-deep
if ocr_status == "done" and deep_reading_status == "pending":
return "/pf-deep"
# 6. Formal note missing → sync
if not note_path:
return "sync"
# 7. Any workspace path missing → sync
workspace_paths = [ ... ]
if not all(workspace_paths):
return "sync"
# 8. Everything ready
return "ready"
```
New order:
```python
# ... steps 1-4 (unchanged) ...
# 5. Formal note missing → sync (moved up: check integrity before /pf-deep)
if not note_path:
return "sync"
# 6. Any workspace path missing → sync (moved up: check integrity before /pf-deep)
workspace_paths = [
entry.get("fulltext_path", ""),
entry.get("deep_reading_path", ""),
entry.get("main_note_path", ""),
entry.get("ai_path", ""),
]
if not all(workspace_paths):
return "sync"
# 7. OCR done, deep reading pending → /pf-deep
if ocr_status == "done" and deep_reading_status == "pending":
return "/pf-deep"
# 8. Everything ready
return "ready"
```
Update the docstring (lines 187-201) to reflect the new order:
Old:
```python
Priority-ordered decision:
1. ocr_status in (failed, blocked, done_incomplete) → "ocr" (retry OCR)
2. has_pdf=False → "sync" (add PDF in Zotero)
3. has_pdf=True AND ocr_status in (pending, nopdf) → "ocr"
4. ocr_status="processing" → "ready" (OCR already running)
5. ocr_status="done" AND deep_reading_status="pending" → "/pf-deep"
6. note_path empty → "sync"
7. Any workspace path empty → "sync"
8. Otherwise → "ready"
```
New:
```python
Priority-ordered decision:
1. ocr_status in (failed, blocked, done_incomplete) → "ocr" (retry OCR)
2. has_pdf=False → "sync" (add PDF in Zotero)
3. has_pdf=True AND ocr_status in (pending, nopdf) → "ocr"
4. ocr_status="processing" → "ready" (OCR already running)
5. note_path empty → "sync" (workspace integrity before /pf-deep)
6. Any workspace path empty → "sync" (workspace integrity before /pf-deep)
7. ocr_status="done" AND deep_reading_status="pending" → "/pf-deep"
8. Otherwise → "ready"
```
=== test_asset_state.py changes ===
In `TestComputeNextStep`, update `test_ready_for_deep_read_returns_pf_deep()` to include note_path and workspace paths (since integrity checks now precede /pf-deep):
```python
def test_ready_for_deep_read_returns_pf_deep(self) -> None:
"""OCR done, deep_reading pending, all integrity checks pass → '/pf-deep'."""
entry = {
"has_pdf": True,
"ocr_status": "done",
"deep_reading_status": "pending",
"note_path": "Literature/骨科/KEY - Title.md",
"fulltext_path": "Literature/骨科/KEY/fulltext.md",
"deep_reading_path": "Literature/骨科/KEY/deep-reading.md",
"main_note_path": "Literature/骨科/KEY/KEY - Title.md",
"ai_path": "Literature/骨科/KEY/ai/",
}
result = compute_next_step(entry)
assert result == "/pf-deep"
```
Add new test for missing note_path blocking /pf-deep:
```python
def test_missing_note_blocks_pf_deep(self) -> None:
"""OCR done, deep_read pending, note_path empty → 'sync', not '/pf-deep'."""
entry = {
"has_pdf": True,
"ocr_status": "done",
"deep_reading_status": "pending",
"note_path": "",
"fulltext_path": "Literature/骨科/KEY/fulltext.md",
"deep_reading_path": "Literature/骨科/KEY/deep-reading.md",
"main_note_path": "Literature/骨科/KEY/KEY - Title.md",
"ai_path": "Literature/骨科/KEY/ai/",
}
result = compute_next_step(entry)
assert result == "sync"
```
Add new test for missing workspace path blocking /pf-deep:
```python
def test_missing_workspace_path_blocks_pf_deep(self) -> None:
"""OCR done, deep_read pending, note_path present but ai_path empty → 'sync'."""
entry = {
"has_pdf": True,
"ocr_status": "done",
"deep_reading_status": "pending",
"note_path": "Literature/骨科/KEY - Title.md",
"fulltext_path": "Literature/骨科/KEY/fulltext.md",
"deep_reading_path": "Literature/骨科/KEY/deep-reading.md",
"main_note_path": "Literature/骨科/KEY/KEY - Title.md",
"ai_path": "",
}
result = compute_next_step(entry)
assert result == "sync"
```
</action>
<verify>
<automated>cd /D "D:\L\Med\Research\99_System\LiteraturePipeline\github-release" && python -m pytest tests/test_asset_state.py -v -x --tb=short 2>&1</automated>
</verify>
<done>
- All tests in test_asset_state.py pass (existing 18 tests + 3 new = 21)
- `test_missing_note_blocks_pf_deep` returns "sync" not "/pf-deep"
- `test_missing_workspace_path_blocks_pf_deep` returns "sync" not "/pf-deep"
- Docstring in compute_next_step reflects new ordering
</done>
</task>
<task type="auto" tdd="true">
<name>Task 2: Return {} instead of None for empty index-derived dicts in status.py (HARDEN-07)</name>
<files>paperforge/worker/status.py, tests/test_status.py</files>
<behavior>
- Test 1 (updated): When no canonical index, `paperforge status --json` returns `lifecycle_level_counts`, `health_aggregate`, `maturity_distribution` as `{}` (not `None`)
- Test 2 (existing unchanged): When index exists, aggregates contain correct data
- Test 3 (existing unchanged): Text output lifecycle section still correct
</behavior>
<action>
=== status.py changes (HARDEN-07) ===
Lines 683-690: Change initialization from `None` to `{}`:
Old:
```python
# Phase 25: index-derived aggregates (None when index unavailable)
lifecycle_level_counts = None
health_aggregate = None
maturity_distribution = None
if summary is not None:
lifecycle_level_counts = summary["lifecycle_level_counts"]
health_aggregate = summary["health_aggregate"]
maturity_distribution = summary["maturity_distribution"]
```
New:
```python
# Phase 25: index-derived aggregates (empty dict when index unavailable)
lifecycle_level_counts = {}
health_aggregate = {}
maturity_distribution = {}
if summary is not None:
lifecycle_level_counts = summary["lifecycle_level_counts"]
health_aggregate = summary["health_aggregate"]
maturity_distribution = summary["maturity_distribution"]
```
Also update the comment on line 712:
Old: `# Phase 25: lifecycle/health/maturity from canonical index (None when unavailable)`
New: `# Phase 25: lifecycle/health/maturity from canonical index ({} when unavailable)`
=== test_status.py changes ===
Update `test_status_json_fallback_when_index_missing` (lines 139-153) to assert `{}` instead of `None`:
Old assertions (lines 151-153):
```python
assert data["lifecycle_level_counts"] is None
assert data["health_aggregate"] is None
assert data["maturity_distribution"] is None
```
New:
```python
assert data["lifecycle_level_counts"] == {}
assert data["health_aggregate"] == {}
assert data["maturity_distribution"] == {}
```
</action>
<verify>
<automated>cd /D "D:\L\Med\Research\99_System\LiteraturePipeline\github-release" && python -m pytest tests/test_status.py -v -x --tb=short 2>&1</automated>
</verify>
<done>
- All tests in test_status.py pass (6 existing tests with updated assertion)
- `paperforge status --json` with no canonical index returns `{}` for all three aggregate fields
- Downstream JSON parsers no longer crash on `null` field access
</done>
</task>
</tasks>
<verification>
1. `python -m pytest tests/test_asset_state.py -v --tb=short` — ALL tests pass (21 tests)
2. `python -m pytest tests/test_status.py -v --tb=short` — ALL tests pass (6 tests)
3. `python -m pytest tests/ -q --tb=short` — full suite no regressions
4. `python -c "from paperforge.worker.asset_state import compute_next_step; assert compute_next_step({'ocr_status':'done','deep_reading_status':'pending','note_path':'','has_pdf':True,'fulltext_path':'a','deep_reading_path':'b','main_note_path':'c','ai_path':'d'}) == 'sync'"` — verify ordering
5. `python -c "from paperforge.worker.asset_state import compute_next_step; assert compute_next_step({'ocr_status':'done','deep_reading_status':'pending','note_path':'a','has_pdf':True,'fulltext_path':'b','deep_reading_path':'c','main_note_path':'d','ai_path':''}) == 'sync'"` — verify path check
</verification>
<success_criteria>
- [ ] `asset_state.py:compute_next_step()` checks note_path and workspace_paths before returning "/pf-deep"
- [ ] `status.py` initializes `lifecycle_level_counts`, `health_aggregate`, `maturity_distribution` as `{}`
- [ ] All tests in `test_asset_state.py` and `test_status.py` pass
- [ ] Full test suite passes with zero regressions
</success_criteria>
<output>
After completion, create `.planning/phases/49-module-hardening/49-003-SUMMARY.md`
</output>