Repository navigation
feat: 将 32 份语法 WASM 接入有界 check all 候选路由 - #27
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 387a1ebaf5
ℹ️ 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".
| let path = PathBuf::from(source_path); | ||
| let source = match read_plain_source(&path) { | ||
| Ok(source) => source, | ||
| Err(reason) => return emit_incomplete(language, source_path, reason), |
There was a problem hiding this comment.
When invoked as codeguard grammar probe python "" --format=json, this branch accepts the empty argument, reading the current directory then fails and calls emit_incomplete with an empty path. The resulting report violates the newly published schema's path.minLength: 1, so schema-validating clients reject exactly this input-error response despite the command's guarantee that failures use the closed schema. Reject an empty path as a usage error or emit a schema-valid path representation.
Useful? React with 👍 / 👎.
| File::open(&absolute) | ||
| .map_err(|_| "source_unreadable")? | ||
| .take(1024 * 1024 + 1) | ||
| .read_to_end(&mut source) | ||
| .map_err(|_| "source_unreadable")?; |
There was a problem hiding this comment.
In an attacker-controlled workspace, the final path can be replaced after the preceding symlink_metadata/metadata checks but before this File::open. Replacing it with a symlink bypasses the stated symlink rejection, while replacing it with a FIFO can block here indefinitely because the 90-second deadline is only established after reading finishes. Open with no-follow/nonblocking semantics and validate metadata from the opened handle, as the runtime's bounded regular-file reader already does.
Useful? React with 👍 / 👎.
| "required": ["schema_version", "report_type", "status", "language", "path", "native", "delivery_decision"], | ||
| "oneOf": [ | ||
| {"required": ["source_sha256", "grammar_sha256", "grammar_qualified", "precheck", "recoveries", "next_action"], "not": {"required": ["reason"]}}, | ||
| {"required": ["reason"], "not": {"anyOf": [{"required": ["precheck"]}, {"required": ["recoveries"]}]}} |
There was a problem hiding this comment.
The failure arm only prohibits precheck and recoveries, so a report containing reason together with source_sha256, grammar_sha256, grammar_qualified, or next_action still validates against this supposedly closed union. A producer can therefore attach apparent parse identity and guidance to a failed probe, and consumers dispatching on those fields may treat an observation that never completed as partially successful. Make the failure branch reject all success-only properties, not just the two collection fields.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,2 @@ | |||
| # 来源补丁保留原始字节与固定摘要，不自动调整上下文空白。 | |||
| grammars/*/source.patch -whitespace | |||
There was a problem hiding this comment.
The -whitespace attribute only disables whitespace-error handling; it does not disable Git's text/EOL conversion. In a fresh checkout with core.autocrlf=true, both source.patch files are converted to CRLF, so their include_bytes! contents no longer match the hard-coded SHA-256 values and bundled_grammar_candidates() makes commands such as grammar status fail with a patch-byte mismatch. Mark these byte-pinned files as -text or binary so checkout configuration cannot alter them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3368d51fce
ℹ️ 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".
| value if !value.starts_with('-') && source.is_none() => { | ||
| source = Some(PathBuf::from(value)) |
There was a problem hiding this comment.
When invoked as codeguard lint zig "" --format=json, parse_args accepts the empty positional value, after which the extension check emits a report whose path is empty. That output violates this command's published schema (path.minLength: 1), so schema-validating clients cannot consume the input-error response; reject an empty source argument as usage error or emit a schema-valid path.
Useful? React with 👍 / 👎.
| "oneOf": [ | ||
| {"required": ["grammar_qualified", "grammar_sha256", "precheck", "recoveries"], "not": {"required": ["reason"]}}, | ||
| {"required": ["reason"], "not": {"required": ["precheck"]}} | ||
| ], |
There was a problem hiding this comment.
The failure branch only prohibits precheck, so a candidate containing reason together with grammar_qualified, grammar_sha256, or recoveries still validates. This lets a failed worker report carry apparent grammar identity or recovery observations, which consumers may interpret as a partially completed parse; make the failure branch reject every success-only candidate property.
Useful? React with 👍 / 👎.
| "status": {"enum": ["not_run", "incomplete", "completed", "diagnostics_observed"]}, | ||
| "reason": {"type": "string", "minLength": 1}, | ||
| "version": {"enum": [null, "0.16.0"]}, | ||
| "tool_sha256": {"anyOf": [{"type": "null"}, {"$ref": "#/$defs/digest"}]}, | ||
| "diagnostic_count": {"type": "integer", "minimum": 0, "maximum": 32}, |
There was a problem hiding this comment.
The native object has no status-specific constraints, so the schema accepts status: "completed" or "diagnostics_observed" with version: null and tool_sha256: null; it also accepts not_run with diagnostics. Because this protocol is meant to represent a verified Zig 0.16.0 invocation, schema-validating consumers cannot distinguish genuine native completion from an internally inconsistent report. Define conditional branches that require the version, digest, count, and diagnostics appropriate to each status.
Useful? React with 👍 / 👎.
| #[cfg(feature = "wasm-precheck")] | ||
| [command, operation, rest @ ..] if command == "grammar" && operation == "probe" => { | ||
| codeguard_cli::grammar_probe_command::run(rest) |
There was a problem hiding this comment.
On non-Unix builds such as the advertised windows_x86_64 candidate, this arm exposes grammar probe, but run_syntax_worker_candidate ultimately calls the runtime process runner that unconditionally returns Termination::UnsupportedPlatform outside Unix. Consequently every valid probe on those builds reports a worker failure instead of parsing the selected grammar; either gate the command/help with cfg(unix) like the Zig route or provide a supported worker launcher.
Useful? React with 👍 / 👎.
| let executable = match std::env::current_exe() { | ||
| Ok(path) => path, | ||
| Err(_) => return ExitCode::from(4), | ||
| }; |
There was a problem hiding this comment.
In environments where std::env::current_exe() is unavailable, such as a restricted Linux container without a usable procfs entry, a valid language and source file return exit code 4 with no JSON at all. This is a worker-startup failure, but the new command promises that failures for valid languages use the same versioned closed protocol, so callers waiting for an incomplete report cannot handle this case; route it through emit_incomplete with a specific reason.
Useful? React with 👍 / 👎.
| let tool_sha256 = format!( | ||
| "{:x}", | ||
| Sha256::digest(read_bounded_regular_file(&executable, 64 * 1024 * 1024).ok()?) |
There was a problem hiding this comment.
Official Zig distributions commonly ship a statically linked zig executable well above this 64 MiB bound, so passing the documented absolute path causes read_bounded_regular_file to fail before zig version is ever run. The caller then labels the explicitly supplied tool unavailable and silently falls back to the unqualified WASM candidate, making the native-first path unusable for standard installations; use a bound that accommodates the supported Zig distribution or hash the executable as a bounded stream.
Useful? React with 👍 / 👎.
| } | ||
| Some(json!({"line":row,"column":column,"rule_id":"zig.ast_check.error"})) | ||
| }) | ||
| .take(32) |
There was a problem hiding this comment.
For a source file producing more than 32 parseable ast-check errors, this unconditional cap drops the remaining diagnostics while observe_native still returns status: "diagnostics_observed" and sets diagnostic_count to 32. Because the report has no truncation field, consumers cannot tell that native results were discarded and may present the retained count as complete; expose truncation or the total count, or keep the native observation incomplete when the cap is reached.
Useful? React with 👍 / 👎.
loong10k
changed the title
loong10k
changed the title
1 participant
CodeGuard 已固定 32 份 CodeGraph 语法 WASM，但此前全项目
check all没有使用这些资产。此变更在源码可选wasm-precheck构建中，先执行现有原生检查，再按源码方言调用隔离 WASM worker：TSX、JavaScript、CFScript 分路由，CFQuery 仅解析完整标签体；有歧义的.h和普通.sql不猜测。候选阶段限 64 文件、64 片段、90 秒，超限保持未完成。本轮补上逐文件 Python 原生优先：Ruff 本轮对相同源码摘要完整执行时，
check all保留其发现并跳过重复 Python WASM；Ruff 不完整、路径或摘要不符时仍保留候选路径。Java P3C 等规范检查不被误当语法确认。封闭check_feedback协议升至 0.33.0，native_preferred_count记录该选择；0.32.0 Schema 单独归档。所有候选均未验收，不能生成已确认违规或交付通过。另保留全部 32 份资产的
grammar probe显式诊断入口，以及局部lint zig FILE --zig-tool ABS_PATH的 Zig 0.16.0 原生ast-check优先链。验证：本地源码可选构建的完整 CLI 测试集退出 0；四个有界真实
check all项目合计调用 32/32 固定 grammar；原生 Ruff 0.16.8 的真实发现保留且不重复解析，混合 Zig 文件继续运行候选 worker。默认无 WASM 构建的受影响聚合测试、全目标 Clippy-D warnings、格式、分层检查、OpenSpec strict 与真实 0.33.0 报告 Draft 2020-12 Schema 验证通过；伪造allow被拒。逐语言版本/方言语料、所有语言的原生能力匹配、误报漏报评测、候选任务与宿主反馈、发行包实装仍待验收。CFQuery 漏检、VB.NET 误报和 COBOL 资源成本仍阻止正式语法能力发布。