fix(streaming-server): harden host-native conversion service (CH-1: #3 #4 #10 #11 #13) - #140
Conversation
CH-1 收斂 host-native conversion service 5 個 hardening 點(#3 artifacts 防穿越 / #4 誠實 health / #10 placeholder 全檔掃描 / #11 HOOPS 失敗診斷結構化 / #13 storage sandbox root 顯式)。proposal/design/tasks + spec delta(host-native-conversion-authority-service ADD 4 條、streaming-ifc-usdc-conversion-authority ADD 1 條);openspec validate --strict 通過。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#11 #13) CH-1 apply — conversion service 5 點 hardening: - #3 /artifacts 改 scoped GET /artifacts/{job_id}/{filename} + relative_to 防穿越,移除 StaticFiles flat mount 與 broad except - #4 /health 誠實回報:呼叫 converter.preflight(),未就緒回 degraded+reason(HTTP 維持 200);HeadlessConverterNotConfigured 補 preflight - #10 placeholder 偵測改全檔掃描(移除 4096 上限);_PLACEHOLDER_MARKERS 下放 conversion_authority 單一 source - #11 HOOPS 失敗 log path 改 ##CONV_META## sentinel JSON 抽取,取代脆弱 prose regex - #13 storage sandbox root 顯式:未設 STORAGE_ROOT 即 raise(不退化 cwd);start-host-native-conversion-service.ps1 補設 STORAGE_ROOT 驗證: pytest 80 passed(baseline 61 + 19 新增 CH-1 案例,零回歸); openspec validate --strict 通過; gitnexus impact 6 symbol 全 LOW。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 20 minutes and 45 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR hardens the host-native conversion service across five dimensions: enforcing explicit ChangesHost-native conversion service hardening
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR hardens the bim-streaming-server host-native conversion authority service by making its observable behavior more “honest” and safer: scoped artifact serving with traversal protection, /health reflecting real converter readiness via preflight, full-file placeholder detection with a single source of truth, structured extraction of Kit/HOOPS log paths, and an explicit STORAGE_ROOT sandbox requirement.
Changes:
- Replace broad
/artifactsstatic mount with a scopedGET /artifacts/{job_id}/{filename}handler and path validation. - Make
/healthcall converterpreflight()and reportokvsdegraded(withreason) instead of hard-coding readiness. - Move placeholder markers to a single constant, scan full
model.usdc, and switch Kit log-path extraction to a##CONV_META## {json}sentinel.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| openspec/changes/harden-host-native-conversion-service/tasks.md | New change task checklist for CH-1 hardening work. |
| openspec/changes/harden-host-native-conversion-service/specs/streaming-ifc-usdc-conversion-authority/spec.md | Spec delta requiring structured sentinel for failure log paths. |
| openspec/changes/harden-host-native-conversion-service/specs/host-native-conversion-authority-service/spec.md | Spec delta for scoped artifacts, honest health, full placeholder scan, explicit sandbox root. |
| openspec/changes/harden-host-native-conversion-service/proposal.md | Proposal describing the five hardening items and impact. |
| openspec/changes/harden-host-native-conversion-service/design.md | Design notes and validation strategy for the hardening changes. |
| bim-streaming-server/tests/test_host_native_conversion_service.py | Adds CH-1 scenarios (artifacts traversal, health degraded/ok, placeholder beyond 4096, sentinel parsing, STORAGE_ROOT contract). |
| bim-streaming-server/tests/test_conversion_authority_api.py | Adds store-path regression tests for full-file placeholder scan and artifact URL shape. |
| bim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/ifc2usdc_powershell_adapter.py | Enforces explicit STORAGE_ROOT, full-file placeholder scan, and sentinel-based log-path extraction. |
| bim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/host_native_conversion_service.py | Implements the new scoped /artifacts/{job_id}/{filename} serving route. |
| bim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/conversion_authority.py | Adds _PLACEHOLDER_MARKERS, adds preflight() to headless converter, updates /health, updates publish gate scanning. |
| bim-streaming-server/scripts/start-host-native-conversion-service.ps1 | Defaults STORAGE_ROOT for host-native startup. |
| bim-streaming-server/scripts/convert-ifc-to-usdc.ps1 | Emits ##CONV_META## JSON sentinel for log paths on success/failure paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @app.get("/artifacts/{job_id}/{filename}") | ||
| def _serve_artifact(job_id: str, filename: str): | ||
| candidate = (_artifacts_root / job_id / filename).resolve() | ||
| try: | ||
| candidate.relative_to(_artifacts_root) | ||
| except ValueError: | ||
| raise HTTPException(status_code=404) | ||
| if not candidate.is_file(): | ||
| raise HTTPException(status_code=404) | ||
| return FileResponse(str(candidate)) |
| # storage sandbox root: the conversion authority refuses to fall back to cwd, so | ||
| # default STORAGE_ROOT to the repo's storage/ dir when the operator did not set it. | ||
| if ([string]::IsNullOrWhiteSpace($env:STORAGE_ROOT)) { | ||
| $repoRoot = Split-Path -Parent $serverRoot | ||
| $env:STORAGE_ROOT = Join-Path $repoRoot "storage" | ||
| } | ||
| Write-Host "STORAGE_ROOT: $($env:STORAGE_ROOT)" |
| converter = store.converter | ||
| preflight = getattr(converter, "preflight", None) | ||
| if callable(preflight): | ||
| try: | ||
| preflight() | ||
| except ConversionAuthorityError as exc: | ||
| status = "degraded" | ||
| conversion_ready = False | ||
| reason: str | None = exc.code or exc.message | ||
| else: | ||
| status = "ok" | ||
| conversion_ready = True | ||
| reason = None | ||
| else: |
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ae1c7af9b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| candidate = (_artifacts_root / job_id / filename).resolve() | ||
| try: | ||
| candidate.relative_to(_artifacts_root) |
There was a problem hiding this comment.
On the Windows host-native service, a filename segment containing encoded backslashes such as ..%5Cother_job%5Cmodel.usdc is still a single URL segment, but Path treats it as directory traversal. Because the guard only verifies candidate is under _artifacts_root, that request can resolve into a sibling job directory and pass, exposing another job's artifact; the per-job route needs to validate against (_artifacts_root / job_id).resolve() instead.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
bim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/host_native_conversion_service.py (1)
146-146: ⚡ Quick winUse explicit exception suppression with
from None.When intentionally changing exception types in an except block, use
from Noneto explicitly suppress the original exception context. This prevents confusing tracebacks and clarifies intent.🐍 Proposed fix
except ValueError: - raise HTTPException(status_code=404) + raise HTTPException(status_code=404) from None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/host_native_conversion_service.py` at line 146, The except block that re-raises an HTTPException currently uses "raise HTTPException(status_code=404)" which preserves the original exception context; update that raise to "raise HTTPException(status_code=404) from None" to explicitly suppress the original exception context (do this where HTTPException is raised in host_native_conversion_service.py, e.g., inside the HostNativeConversionService handler/function that currently raises HTTPException).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@bim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/conversion_authority.py`:
- Around line 177-180: In the except ConversionAuthorityError handler update how
the health reason is set: change the order so the human-readable preflight
message is preferred by assigning reason = exc.message or exc.code (instead of
exc.code or exc.message); keep the surrounding logic that sets status =
"degraded" and conversion_ready = False and ensure the reason variable (used for
/health.reason) is populated from that updated expression in the except block
handling ConversionAuthorityError.
- Around line 453-455: The current code reads the entire model file into memory
via output_paths["model_path"].read_bytes().lower() and searches
_PLACEHOLDER_MARKERS, which causes huge transient allocations; replace this with
a streaming/chunked scanner function (e.g., contains_placeholder or
scan_for_placeholders) that opens the path in binary ('rb'), reads fixed-size
chunks (e.g., 64KB), lowercases each chunk (bytes.lower()), and searches for any
marker (ensure _PLACEHOLDER_MARKERS are bytes and lowercased) while preserving
an overlap buffer between chunks equal to the longest marker length so markers
spanning chunk boundaries are detected; call this scanner from the place that
currently uses model_bytes and raise
ConversionAuthorityError("placeholder_usdc", ...) if it returns true, and
expose/reuse the scanner so the adapter can call the same function instead of
buffering the full file.
In `@bim-streaming-server/tests/test_conversion_authority_api.py`:
- Around line 241-245: Replace all fullwidth punctuation in the
comment/docstring blocks that reference harden-host-native-conversion-service
CH-1 and the host-native-conversion-authority-service note (the block mentioning
conversion_authority._PLACEHOLDER_MARKERS and "Placeholder detection SHALL scan
the full published artifact") with standard ASCII punctuation (commas,
parentheses, quotes, etc.); do the same for the other nearby comment ranges
flagged by Ruff (the similar comment blocks noted around the same section).
Ensure the text still references conversion_authority._PLACEHOLDER_MARKERS
exactly and retains the same wording, only changing punctuation characters to
their ASCII equivalents so RUF002/RUF003 warnings are resolved.
In `@bim-streaming-server/tests/test_host_native_conversion_service.py`:
- Around line 569-572: Replace the fullwidth closing parenthesis character `）`
in the test comment block with the ASCII `)` to satisfy Ruff's RUF003 lint rule;
locate the occurrence mentioned in the test_host_native_conversion_service.py
comment (the line that currently ends with `shape(對齊
convert-ifc-to-usdc.ps1::Invoke-KitConversion）`) and change it to `shape(對齊
convert-ifc-to-usdc.ps1::Invoke-KitConversion)` so the file passes linting.
---
Nitpick comments:
In
`@bim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/host_native_conversion_service.py`:
- Line 146: The except block that re-raises an HTTPException currently uses
"raise HTTPException(status_code=404)" which preserves the original exception
context; update that raise to "raise HTTPException(status_code=404) from None"
to explicitly suppress the original exception context (do this where
HTTPException is raised in host_native_conversion_service.py, e.g., inside the
HostNativeConversionService handler/function that currently raises
HTTPException).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 392102f1-ed2e-4b56-9e33-c24af4fbfe87
📒 Files selected for processing (12)
bim-streaming-server/scripts/convert-ifc-to-usdc.ps1bim-streaming-server/scripts/start-host-native-conversion-service.ps1bim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/conversion_authority.pybim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/host_native_conversion_service.pybim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/ifc2usdc_powershell_adapter.pybim-streaming-server/tests/test_conversion_authority_api.pybim-streaming-server/tests/test_host_native_conversion_service.pyopenspec/changes/harden-host-native-conversion-service/design.mdopenspec/changes/harden-host-native-conversion-service/proposal.mdopenspec/changes/harden-host-native-conversion-service/specs/host-native-conversion-authority-service/spec.mdopenspec/changes/harden-host-native-conversion-service/specs/streaming-ifc-usdc-conversion-authority/spec.mdopenspec/changes/harden-host-native-conversion-service/tasks.md
| model_bytes = output_paths["model_path"].read_bytes().lower() | ||
| if any(marker in model_bytes for marker in _PLACEHOLDER_MARKERS): | ||
| raise ConversionAuthorityError("placeholder_usdc", "Generated model.usdc looks like a placeholder output.") |
There was a problem hiding this comment.
Stream placeholder detection instead of buffering the whole artifact.
read_bytes().lower() allocates another full copy of model.usdc, and the adapter now does the same scan before the store repeats it. On large production models this can create multi-GB transient allocations just to search for a short marker. Please move this to a shared chunked scanner so the full-file contract stays intact without the memory spike.
Chunked scan sketch
+def _contains_marker(path: Path, markers: tuple[bytes, ...], chunk_size: int = 1024 * 1024) -> bool:
+ lowered = tuple(marker.lower() for marker in markers)
+ overlap = max(len(marker) for marker in lowered) - 1
+ tail = b""
+ with path.open("rb") as handle:
+ while chunk := handle.read(chunk_size):
+ window = (tail + chunk).lower()
+ if any(marker in window for marker in lowered):
+ return True
+ tail = window[-overlap:] if overlap > 0 else b""
+ return False
+
- model_bytes = output_paths["model_path"].read_bytes().lower()
- if any(marker in model_bytes for marker in _PLACEHOLDER_MARKERS):
+ if _contains_marker(output_paths["model_path"], _PLACEHOLDER_MARKERS):
raise ConversionAuthorityError(
"placeholder_usdc",
"Generated model.usdc looks like a placeholder output.",
)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@bim-streaming-server/source/extensions/ezplus.bim_review_stream.messaging/ezplus/bim_review_stream/messaging/conversion_authority.py`
around lines 453 - 455, The current code reads the entire model file into memory
via output_paths["model_path"].read_bytes().lower() and searches
_PLACEHOLDER_MARKERS, which causes huge transient allocations; replace this with
a streaming/chunked scanner function (e.g., contains_placeholder or
scan_for_placeholders) that opens the path in binary ('rb'), reads fixed-size
chunks (e.g., 64KB), lowercases each chunk (bytes.lower()), and searches for any
marker (ensure _PLACEHOLDER_MARKERS are bytes and lowercased) while preserving
an overlap buffer between chunks equal to the longest marker length so markers
spanning chunk boundaries are detected; call this scanner from the place that
currently uses model_bytes and raise
ConversionAuthorityError("placeholder_usdc", ...) if it returns true, and
expose/reuse the scanner so the adapter can call the same function instead of
buffering the full file.
| # --- harden-host-native-conversion-service CH-1（store 路徑：#10 全檔掃描 / #3 URL 形狀）--- | ||
| # placeholder 偵測 SHALL 掃完整 model.usdc，不得只看前綴；publish store 與 adapter | ||
| # 共用 conversion_authority._PLACEHOLDER_MARKERS 單一 source（見 spec | ||
| # host-native-conversion-authority-service「Placeholder detection SHALL scan the | ||
| # full published artifact」）。 |
There was a problem hiding this comment.
Normalize the fullwidth punctuation in these comments/docstrings.
Ruff is already flagging these ranges with RUF002/RUF003, so this file will keep emitting lint warnings until the fullwidth punctuation is replaced with ASCII equivalents or explicitly ignored.
Also applies to: 249-250, 261-261, 275-275, 288-290
🧰 Tools
🪛 Ruff (0.15.14)
[warning] 241-241: Comment contains ambiguous （ (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF003)
[warning] 241-241: Comment contains ambiguous ： (FULLWIDTH COLON). Did you mean : (COLON)?
(RUF003)
[warning] 241-241: Comment contains ambiguous ） (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF003)
[warning] 242-242: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
[warning] 242-242: Comment contains ambiguous ； (FULLWIDTH SEMICOLON). Did you mean ; (SEMICOLON)?
(RUF003)
[warning] 243-243: Comment contains ambiguous （ (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF003)
[warning] 245-245: Comment contains ambiguous ） (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF003)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bim-streaming-server/tests/test_conversion_authority_api.py` around lines 241
- 245, Replace all fullwidth punctuation in the comment/docstring blocks that
reference harden-host-native-conversion-service CH-1 and the
host-native-conversion-authority-service note (the block mentioning
conversion_authority._PLACEHOLDER_MARKERS and "Placeholder detection SHALL scan
the full published artifact") with standard ASCII punctuation (commas,
parentheses, quotes, etc.); do the same for the other nearby comment ranges
flagged by Ruff (the similar comment blocks noted around the same section).
Ensure the text still references conversion_authority._PLACEHOLDER_MARKERS
exactly and retains the same wording, only changing punctuation characters to
their ASCII equivalents so RUF002/RUF003 warnings are resolved.
| # 真實 ps1 throw shape(對齊 convert-ifc-to-usdc.ps1::Invoke-KitConversion）。 | ||
| # harden-host-native-conversion-service #11:log path 改由結構化 sentinel | ||
| # `##CONV_META## {json}` 抽取(非脆弱 prose regex);保留人類可讀 prose 兩行 | ||
| # 供 operator 閱讀。JSON string 內 Windows path 的反斜線須跳脫(json.dumps 處理)。 |
There was a problem hiding this comment.
Replace the fullwidth parenthesis here or Ruff will keep warning.
Line 569 contains ）, which Ruff flags as RUF003. Swapping it for ASCII ) (or explicitly suppressing the rule) will keep this test file lint-clean.
🧰 Tools
🪛 Ruff (0.15.14)
[warning] 569-569: Comment contains ambiguous ） (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF003)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bim-streaming-server/tests/test_host_native_conversion_service.py` around
lines 569 - 572, Replace the fullwidth closing parenthesis character `）` in the
test comment block with the ASCII `)` to satisfy Ruff's RUF003 lint rule; locate
the occurrence mentioned in the test_host_native_conversion_service.py comment
(the line that currently ends with `shape(對齊
convert-ifc-to-usdc.ps1::Invoke-KitConversion）`) and change it to `shape(對齊
convert-ifc-to-usdc.ps1::Invoke-KitConversion)` so the file passes linting.
…t 死碼 / sentinel regex) 對抗式 review(opus 5 lens)findings: - [major] storage_root=空字串繞過 cwd-fallback 契約: __init__ 改 strip + truthy 檢查,空白(含純空白 env)即 raise;補 test_adapter_ctor_raises_when_storage_root_blank 鎖回歸 - [minor] preflight storage_root 檢查是死碼(Path 物件恆 truthy): 移除,__init__ 為 single gate - [minor] #11 sentinel regex 非貪婪遇 log path 含字面右括號會截斷: 改貪婪 + 行錨 + MULTILINE - 連帶: l4_verify_sidecar_pass.py 直接建構 adapter 補 storage_root;proposal Impact 聲明此 caller 驗證: pytest 81 passed(80 + 1 新回歸測試,零回歸)。review agent 殘留 scratch 檔已清除。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac8b04ae2a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| explicit_root = str(storage_root).strip() if storage_root is not None else "" | ||
| env_root = (os.environ.get("STORAGE_ROOT") or "").strip() | ||
| chosen_root = explicit_root or env_root |
There was a problem hiding this comment.
When host_local_path/local_path is absent or points at a missing file, _resolve_local_ifc falls back to ifc_artifact.url; for edge-local:// and file:// URLs that path is still checked by _anchor() against work_dir (defaulting to repo_root) rather than this newly required storage_root. In that fallback scenario a request can still convert any readable file under the repo/work dir even though STORAGE_ROOT was configured as the IFC sandbox, so the hardening does not actually bound all local IFC inputs to the explicit storage root.
Useful? React with 👍 / 👎.
| model_bytes = output_paths["model_path"].read_bytes().lower() | ||
| if any(marker in model_bytes for marker in _PLACEHOLDER_MARKERS): |
There was a problem hiding this comment.
For large BIM conversions, model.usdc can be hundreds of MB or more, and this now reads the entire artifact into memory just to search a small marker before publishing. A large but otherwise valid conversion can therefore spike memory or fail the job/service; the same full-artifact requirement can be met with a chunked/streaming scan without materializing the whole USDC at once.
Useful? React with 👍 / 👎.
…on / mkdir) CodeRabbit + Copilot + Codex review findings: - [P2 真 security] /artifacts 跨 job 穿越: Windows filename 含 encoded backslash(%5C)被 Path 當分隔可讀 sibling job;改兩層 relative_to(job_dir 在 artifacts_root 內 + candidate 在 job_dir 內)enforce per-job;補 test_artifacts_route_rejects_cross_job_backslash_with_404 - [minor] /health reason 改 message or code(actionable);加 broad except → degraded(不因 converter 異常回 500) - [minor] start-host-native-conversion-service.ps1 補 STORAGE_ROOT mkdir 不改(已記錄權衡): placeholder 全檔 read_bytes(explore demo-scope YAGNI,follow-up);全形標點 lint(repo 中文註解慣例)。 驗證: pytest 82 passed(81 + 1 跨 job 回歸測試)。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
|
CH-1 merge 決策:2 個 P2 defer 為獨立 follow-up。 (1) placeholder 全檔 read_bytes 記憶體(CodeRabbit major / Codex P2):explore 階段已記錄的 demo-scope 權衡(design.md Key Decisions #10:全檔 read_bytes、不預先加 chunk 抽象、實測超大檔再改)。production chunked/streaming scan 為效能 follow-up,非本 change 誠實性/安全性 hardening 目標。 (2) _anchor URL fallback 用 work_dir 非 storage_root(Codex P2,屬實):pre-existing(#13 未引入)+ 超出 #13 spec scope(requirement 明確只收斂 host_local_path)。CH-1 對 sandbox 為淨改善(host_local_path 已 bound storage_root + 不退化 cwd)。IFC input sandbox 完整統一(_anchor 也 bound storage_root)為獨立 follow-up,可併入後續 internal-auth change。 驗證:pytest 82 passed;openspec validate --strict 通過。 |
OpenSpec sync/archive(PR #140 merged squash c7b1dae 後): - change openspec/changes/harden-... → archive/2026-06-01-harden-host-native-conversion-service - spec delta 併入 openspec/specs/: host-native-conversion-authority-service +4 requirement、streaming-ifc-usdc-conversion-authority +1(specs 維持 32 capability,additive) - tasks.md 21 tasks 全勾;roadmap saas-roadmap-2026-05.md §1.6 加 2026-06-01 archive entry openspec validate --specs --strict 通過。 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
摘要
CH-1（風險處理 pilot 第一個 change）：收斂 host-native conversion service 的 5 個誠實性 / 安全性 hardening 點，全純程式 L1，不新增 production dependency。
改動（#編號對應 2026-06-01 風險報告）
/artifacts改 scopedGET /artifacts/{job_id}/{filename}+resolve().relative_to防路徑穿越，移除整包StaticFilesflat mount 與 broadexcept: pass/health誠實回報：呼叫converter.preflight()，未就緒回degraded+reason（HTTP 維持 200）；HeadlessConverterNotConfigured補preflight_PLACEHOLDER_MARKERS下放conversion_authority作單一 source##CONV_META##sentinel JSON 抽取，取代脆弱 prose regexSTORAGE_ROOT即 fail honest（不退化 cwd）；start-host-native-conversion-service.ps1補設STORAGE_ROOT=<repo>/storageOpenSpec
change
harden-host-native-conversion-service；spec delta：host-native-conversion-authority-service（ADD 4 條）+streaming-ifc-usdc-conversion-authority（ADD 1 條）。openspec validate --strict通過。驗證
Review 留意點
/health對「converter 無preflightmethod」報degraded（誠實優先；production 兩種 converter 皆有 preflight，僅影響未實作 preflight 的新 converter）#13改__init__契約後，既有測試以 autouse fixture 設STORAGE_ROOT；缺值情境用monkeypatch.delenv覆蓋🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Bug Fixes
Security & Reliability