Skip to content

Commit 0f03f13

Browse files
committed
fix: harden output boundaries and provider verification
Release as deckflow-core 0.1.1.
1 parent 83263ad commit 0f03f13

15 files changed

Lines changed: 470 additions & 29 deletions

README.md

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ virtualenv, or vendored into a Skill with no install step at all.
2323

2424
```bash
2525
# Managed install — no venv, no pipx, no uv, works on a PEP 668 interpreter
26-
python3 -m pip install --target ~/.deckflow/core/0.1.0 deckflow-core==0.1.0
27-
PYTHONPATH=~/.deckflow/core/0.1.0 python3 -m deckflow_core providers
26+
python3 -m pip install --target ~/.deckflow/core/0.1.1 deckflow-core==0.1.1
27+
PYTHONPATH=~/.deckflow/core/0.1.1 python3 -m deckflow_core providers
2828
```
2929

3030
```bash
@@ -224,7 +224,7 @@ deterministically so two isolated runs over the same inputs produce the same
224224
report bytes.
225225

226226
```json
227-
{"schema_version": 1, "command": "providers", "core_version": "0.1.0",
227+
{"schema_version": 1, "command": "providers", "core_version": "0.1.1",
228228
"status": "succeeded", "started_at": "...", "finished_at": "...",
229229
"providers": [], "inputs": [], "outputs": [], "diagnostics": []}
230230
```
@@ -246,8 +246,10 @@ A failure still prints a parseable envelope on stdout; prose goes to stderr.
246246

247247
## Scope of this release
248248

249-
v0.1.0 registers `providers`, `parse`, `editor` and `export pptx` — the whole
250-
planned surface.
249+
v0.1.1 registers `providers`, `parse`, `editor` and `export pptx` — the whole
250+
planned surface. This patch release hardens report/output collision handling,
251+
Parse Bundle replacement, editor page boundaries and provider version
252+
verification, and reports editor crashes after readiness as failures.
251253

252254
`validate html` is deferred beyond 0.1.x and is not registered at all: a
253255
deferred command may not ship as a stub, a placeholder, or a "not implemented"

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
44

55
[project]
66
name = "deckflow-core"
7-
version = "0.1.0"
7+
version = "0.1.1"
88
description = "Deckflow capability broker: one CLI over deckflow-extract, html-editor and deckhtml, with on-demand provider acquisition."
99
requires-python = ">=3.10"
1010
license = "MIT"

src/deckflow_core/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
from pathlib import Path
1717

18-
__version__ = "0.1.0"
18+
__version__ = "0.1.1"
1919
SCHEMA_VERSION = 1
2020

2121

src/deckflow_core/cli.py

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
from .commands import editor as editor_cmd, export_pptx, parse as parse_cmd, providers_cmd
2222
from .diagnostics import Diagnostic
2323
from .envelope import STATUS_FAILED, Envelope
24-
from .exits import EXIT_INTERRUPT, EXIT_OK, EXIT_USAGE, CoreError
24+
from .exits import EXIT_INTERRUPT, EXIT_OK, EXIT_OUTPUT, EXIT_USAGE, CoreError
2525
from .fsutil import atomic_write_text
2626
from .providers import resolve as resolver
2727

@@ -226,6 +226,95 @@ def _normalize(options: argparse.Namespace) -> argparse.Namespace:
226226
return options
227227

228228

229+
def _prepare_report_path(options: argparse.Namespace) -> None:
230+
"""Resolve and validate the report before any provider or project can run.
231+
232+
A report is an output in its own right. It may not alias another output,
233+
overwrite an input, or land inside the canonical ``project/deck`` tree.
234+
Clearing ``options.report`` before validation also ensures that a rejected
235+
report path is never used while emitting the failure envelope.
236+
"""
237+
raw = getattr(options, "report", None)
238+
if not raw:
239+
return
240+
241+
options.report = None
242+
report = Path(raw).expanduser().resolve()
243+
conflicts: list[tuple[str, Path]] = []
244+
245+
def protect(label: str, value: str | Path | None) -> None:
246+
if value is None:
247+
return
248+
protected = Path(value).expanduser().resolve()
249+
if report == protected:
250+
conflicts.append((label, protected))
251+
252+
protect("input", getattr(options, "input", None))
253+
protect("output", getattr(options, "output", None))
254+
protect("browser executable", getattr(options, "browser", None))
255+
for name, path in getattr(options, "provider_bins", {}).items():
256+
protect(f"{name} provider executable", path)
257+
258+
if getattr(options, "command", None) == "parse":
259+
output_root = Path(options.out).expanduser().resolve()
260+
if report == output_root or output_root in report.parents:
261+
conflicts.append(("parse output directory", output_root))
262+
263+
project_value = getattr(options, "project", None)
264+
if project_value:
265+
project_root = Path(project_value).expanduser().resolve()
266+
deck_root = project_root / "deck"
267+
if report == deck_root or deck_root in report.parents:
268+
conflicts.append(("canonical deck tree", deck_root))
269+
for relative in ("deck-plan.json", "intent-detail.json"):
270+
protect(f"project record {relative}", project_root / relative)
271+
272+
if conflicts:
273+
label, protected = conflicts[0]
274+
raise CoreError(
275+
Diagnostic(
276+
rule_id="REPORT_PATH_CONFLICT",
277+
severity="error",
278+
message=f"The report path conflicts with the command's {label}.",
279+
location=str(report),
280+
expected="a distinct report path that cannot overwrite command inputs or outputs",
281+
actual=f"same as or inside {protected}",
282+
recovery="Choose a separate --report path outside the parse bundle and canonical deck tree.",
283+
),
284+
exit_code=EXIT_OUTPUT,
285+
)
286+
287+
if report.is_dir():
288+
raise CoreError(
289+
Diagnostic(
290+
rule_id="REPORT_NOT_A_FILE",
291+
severity="error",
292+
message="The report target is a directory.",
293+
location=str(report),
294+
expected="a JSON file path",
295+
actual="an existing directory",
296+
recovery="Choose a file path for --report.",
297+
),
298+
exit_code=EXIT_OUTPUT,
299+
)
300+
301+
overwrite = bool(getattr(options, "overwrite", False))
302+
if report.exists() and not overwrite:
303+
raise CoreError(
304+
Diagnostic(
305+
rule_id="REPORT_EXISTS",
306+
severity="error",
307+
message="The report target already exists.",
308+
location=str(report),
309+
expected="a new report path, or an explicit --overwrite",
310+
actual="an existing path",
311+
recovery="Choose another --report path, or pass --overwrite on commands that support it.",
312+
),
313+
exit_code=EXIT_OUTPUT,
314+
)
315+
options.report = str(report)
316+
317+
229318
def _emit(envelope: Envelope, human: str | None, report: str | None) -> None:
230319
payload = envelope.dumps()
231320
if report:
@@ -250,6 +339,7 @@ def main(argv: Sequence[str] | None = None) -> int:
250339

251340
try:
252341
options = _normalize(options)
342+
_prepare_report_path(options)
253343
if options.command == "providers":
254344
envelope, human, code = providers_cmd.run(options)
255345
elif options.command == "editor":

src/deckflow_core/commands/editor.py

Lines changed: 63 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939

4040
_ELEMENT_ID_RE = re.compile(r"""data-element-id\s*=\s*["']([^"']+)["']""")
4141
_URL_RE = re.compile(r"https?://[0-9A-Za-z\.\-]+:\d+\S*")
42+
_SLIDE_ID_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*$")
4243
# The provider's own working files. They appear under the project root by
4344
# design, so they are expected artefacts rather than boundary violations.
4445
_EDITOR_ARTEFACTS = (".local-html-editor",)
@@ -106,7 +107,23 @@ def _preflight(root: Path, page: str | None) -> tuple[Path, Path]:
106107
),
107108
exit_code=EXIT_INPUT,
108109
)
109-
pages = sorted(pages_dir.glob("*.html"))
110+
pages_root = pages_dir.resolve()
111+
page_candidates = sorted(candidate for candidate in pages_dir.glob("*.html") if candidate.is_file())
112+
escaped = [candidate for candidate in page_candidates if candidate.resolve().parent != pages_root]
113+
if escaped:
114+
raise CoreError(
115+
Diagnostic(
116+
rule_id="EDITOR_PAGE_ESCAPES_ROOT",
117+
severity="error",
118+
message="A canonical page resolves outside deck/pages/.",
119+
location=str(escaped[0]),
120+
expected=f"a direct HTML file inside {pages_root}",
121+
actual=str(escaped[0].resolve()),
122+
recovery="Replace the escaping symlink with a real canonical page inside deck/pages/.",
123+
),
124+
exit_code=EXIT_INPUT,
125+
)
126+
pages = [candidate.resolve() for candidate in page_candidates]
110127
if not pages:
111128
raise CoreError(
112129
Diagnostic(
@@ -121,15 +138,29 @@ def _preflight(root: Path, page: str | None) -> tuple[Path, Path]:
121138
exit_code=EXIT_INPUT,
122139
)
123140
if page is None:
124-
return deck_dir, pages_dir
125-
target = pages_dir / f"{page}.html"
126-
if not target.is_file():
141+
return deck_dir, pages_root
142+
if not _SLIDE_ID_RE.fullmatch(page):
143+
raise CoreError(
144+
Diagnostic(
145+
rule_id="EDITOR_PAGE_ID_INVALID",
146+
severity="error",
147+
message="The requested page is not a safe slide id.",
148+
location=page,
149+
expected="an alphanumeric slide id containing only letters, digits, dot, underscore, or hyphen",
150+
actual=page,
151+
recovery="Pass the stem of a direct deck/pages/*.html file, without path separators or `..`.",
152+
),
153+
exit_code=EXIT_INPUT,
154+
)
155+
pages_by_id = {candidate.stem: candidate for candidate in pages}
156+
target = pages_by_id.get(page)
157+
if target is None:
127158
raise CoreError(
128159
Diagnostic(
129160
rule_id="EDITOR_PAGE_NOT_FOUND",
130161
severity="error",
131162
message=f"No canonical page named {page}.",
132-
location=str(target),
163+
location=str(pages_root / f"{page}.html"),
133164
expected=f"one of: {', '.join(p.stem for p in pages)}",
134165
actual=page,
135166
recovery="Pass --page with a slide id that exists, or omit it to open all pages.",
@@ -318,8 +349,11 @@ def run(options: Any) -> tuple[Envelope, str | None, int]:
318349
)
319350
stdout_lines: list[str] = []
320351
stderr_lines: list[str] = []
352+
drainers: list[threading.Thread] = []
321353
for stream, sink in ((child.stdout, stdout_lines), (child.stderr, stderr_lines)):
322-
threading.Thread(target=_drain, args=(stream, sink), daemon=True).start()
354+
thread = threading.Thread(target=_drain, args=(stream, sink), daemon=True)
355+
thread.start()
356+
drainers.append(thread)
323357

324358
url = _await_ready(stdout_lines, child)
325359
if url is None:
@@ -345,16 +379,36 @@ def run(options: Any) -> tuple[Envelope, str | None, int]:
345379
sys.stderr.write("[deckflow] press Ctrl-C when you are done editing\n")
346380

347381
interrupted = _wait_for_session_end(child)
382+
for thread in drainers:
383+
thread.join(timeout=1)
384+
provider_exit_code = child.poll()
385+
provider_failed = not interrupted and provider_exit_code != 0
386+
provider_output = summarize_output("\n".join(stderr_lines), "\n".join(stdout_lines))
348387

349388
after = _snapshot(deck_dir)
350389
changed_pages, findings = _classify(before, after)
351390
envelope.extend(findings)
391+
if provider_failed:
392+
envelope.add(
393+
Diagnostic(
394+
rule_id="EDITOR_PROVIDER_EXITED",
395+
severity="error",
396+
message="The editor provider exited unexpectedly after announcing readiness.",
397+
location=str(target),
398+
expected="exit code 0, or a session ended by the supervising interrupt",
399+
actual=f"exit code {provider_exit_code}: {provider_output}",
400+
recovery="Restore any affected pages from the editor backups, then re-run the session.",
401+
)
402+
)
352403
envelope.extra.update({
353404
"event": "finished",
354405
"session_id": session_id,
355406
"project": str(deck_dir.parent),
356407
"changed_pages": changed_pages,
357-
"ended_by": "interrupt" if interrupted else "editor-exit",
408+
"ended_by": (
409+
"interrupt" if interrupted else "provider-error" if provider_failed else "editor-exit"
410+
),
411+
"provider_exit_code": provider_exit_code,
358412
"backups": str(deck_dir / ".local-html-editor" / "backups"),
359413
})
360414

@@ -388,4 +442,6 @@ def run(options: Any) -> tuple[Envelope, str | None, int]:
388442

389443
has_error = any(d.severity == "error" for d in envelope.diagnostics)
390444
envelope.status = STATUS_FAILED if has_error else STATUS_SUCCEEDED
445+
if provider_failed:
446+
return envelope, None, EXIT_EXECUTION
391447
return envelope, None, EXIT_CONTRACT if has_error else EXIT_OK

src/deckflow_core/commands/parse.py

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
file_record,
3030
)
3131
from ..exits import EXIT_EXECUTION, EXIT_INPUT, EXIT_OK, EXIT_OUTPUT, CoreError
32-
from ..fsutil import deckflow_home, require_empty_dir, sha256_file
32+
from ..fsutil import deckflow_home, require_empty_dir, resolve_within, sha256_file
3333
from ..providers import matrix
3434
from ..providers import resolve as resolver
3535

@@ -142,6 +142,81 @@ def _bundle_outputs(out: Path) -> list[dict[str, Any]]:
142142
return outputs
143143

144144

145+
def _refuse_unsafe_overwrite(out: Path, actual: str) -> None:
146+
raise CoreError(
147+
Diagnostic(
148+
rule_id="PARSE_OVERWRITE_UNOWNED",
149+
severity="error",
150+
message="Refusing to replace a directory that is not a valid Parse Bundle.",
151+
location=str(out),
152+
expected=(
153+
"a deckflow-extract bundle containing a valid parse-manifest.json, "
154+
"document, and assets directory"
155+
),
156+
actual=actual,
157+
recovery="Choose a new --out directory. Move unrelated files manually if they are no longer needed.",
158+
),
159+
exit_code=EXIT_OUTPUT,
160+
)
161+
162+
163+
def _require_safe_output(out: Path, *, overwrite: bool) -> None:
164+
"""Allow replacement only for a complete deckflow-extract-owned bundle."""
165+
require_empty_dir(out, overwrite=overwrite)
166+
if not out.exists() or not any(out.iterdir()) or not overwrite:
167+
return
168+
169+
manifest_path = out / _MANIFEST
170+
if manifest_path.is_symlink() or not manifest_path.is_file():
171+
_refuse_unsafe_overwrite(out, "missing a direct parse-manifest.json file")
172+
try:
173+
manifest = json.loads(manifest_path.read_text(encoding="utf-8"))
174+
except (OSError, ValueError) as error:
175+
_refuse_unsafe_overwrite(out, f"unreadable parse-manifest.json: {error}")
176+
if not isinstance(manifest, dict):
177+
_refuse_unsafe_overwrite(out, "parse-manifest.json is not a JSON object")
178+
179+
tool = manifest.get("tool")
180+
outputs = manifest.get("outputs")
181+
schema_version = manifest.get("schema_version")
182+
if (
183+
not isinstance(tool, dict)
184+
or tool.get("name") != "deckflow-extract"
185+
or not isinstance(schema_version, int)
186+
or schema_version < 1
187+
or not isinstance(outputs, dict)
188+
):
189+
_refuse_unsafe_overwrite(out, "manifest ownership or schema fields are invalid")
190+
191+
document_value = outputs.get("document")
192+
assets_value = outputs.get("assets_dir")
193+
if not isinstance(document_value, str) or not isinstance(assets_value, str):
194+
_refuse_unsafe_overwrite(out, "manifest output paths are missing or invalid")
195+
try:
196+
document = resolve_within(out, Path(document_value))
197+
assets = resolve_within(out, Path(assets_value))
198+
except CoreError:
199+
_refuse_unsafe_overwrite(out, "manifest output paths escape the bundle directory")
200+
if not document.is_file() or not assets.is_dir():
201+
_refuse_unsafe_overwrite(out, "manifest output files are incomplete")
202+
203+
204+
def _require_input_outside_output(source: Path, out: Path) -> None:
205+
if source == out or out in source.parents:
206+
raise CoreError(
207+
Diagnostic(
208+
rule_id="PARSE_OUTPUT_CONTAINS_INPUT",
209+
severity="error",
210+
message="The Parse Bundle output directory contains the input file.",
211+
location=str(out),
212+
expected="an output directory separate from the input file",
213+
actual=str(source),
214+
recovery="Choose --out outside the directory tree containing the input.",
215+
),
216+
exit_code=EXIT_OUTPUT,
217+
)
218+
219+
145220
def _carry_diagnostics(envelope: Envelope, provider_result: dict[str, Any]) -> None:
146221
"""Surface the provider's own findings without re-judging them."""
147222
for entry in provider_result.get("diagnostics") or ():
@@ -181,7 +256,8 @@ def run(options: Any) -> tuple[Envelope, str | None, int]:
181256
envelope = Envelope(command=COMMAND)
182257
source = _resolve_input(options.input)
183258
out = Path(options.out).expanduser().resolve()
184-
require_empty_dir(out, overwrite=options.overwrite)
259+
_require_input_outside_output(source, out)
260+
_require_safe_output(out, overwrite=options.overwrite)
185261

186262
spec = matrix.get("extract", options.provider_specs)
187263
resolution = resolver.resolve(

0 commit comments

Comments
 (0)