Repository navigation
feat(review): inspect native artifacts without executing them - #675
Merged
Merged
Conversation
Add bounded ELF and gzip inspection with loader-table checks and byte ranges. Keep insufficient inspection uncovered and preserve the existing approval gate.
monit-reviewer
requested changes
Oct 17, 2026
monit-reviewer
left a comment
monit-reviewer
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: b7cc175ed376
Profile: open-cli-collective-review - Posting as: monit-reviewer
Summary
| Reviewer | Findings |
|---|---|
| go:implementation-tests | 1 |
| structure:repo-health | 0 |
| policies:conventions | 0 |
go:implementation-tests (1 finding)
Major - internal/pireviewtool/native.go:190
An empty SHT_DYNSYM can panic here instead of returning ErrDenied. validateNativeTables checks the linked table's type only while validating entries, so a zero-sized symbol section can link to SHT_NOBITS with a range beyond the file. With no dynamic section/program, runtime validation succeeds and this slice panics. Validate every dynamic/symbol section's linked string table and bounds regardless of entry count, and add a regression test for an empty DYNSYM linked to an out-of-range NOBITS section.
Reviewer Coverage
Complete coverage details are retained in the local coverage.json artifact.
go:implementation-tests— complete (broad); scope: 8 files; examples:internal/llmadapters/pi_rpc.go,internal/llmadapters/pi_rpc_extension_unix_test.go,internal/llmadapters/pi_rpc_test.go,internal/pipeline/prompts.go,internal/pireviewtool/native.go; 3 omitted; body-inspected: 8 files; examples:internal/llmadapters/pi_rpc.go,internal/llmadapters/pi_rpc_extension_unix_test.go,internal/llmadapters/pi_rpc_test.go,internal/pipeline/prompts.go,internal/pireviewtool/native.go; 3 omitted; skipped: none; context (not coverage): 1 file; examples:docs/development.md; constraints: Static review only; tests were not executed through the read-only review tools.structure:repo-health— complete (broad); scope: 6 files; examples:docs/checkout-native-review-contract.md,internal/llmadapters/pi_rpc.go,internal/pipeline/prompts.go,internal/pireviewtool/native.go,internal/pireviewtool/native_runtime.go; 1 omitted; body-inspected: 6 files; examples:docs/checkout-native-review-contract.md,internal/llmadapters/pi_rpc.go,internal/pipeline/prompts.go,internal/pireviewtool/native.go,internal/pireviewtool/native_runtime.go; 1 omitted; skipped: none; context (not coverage): 3 files; examples:internal/llmadapters/pi_rpc_extension_unix_test.go,internal/llmadapters/pi_rpc_test.go,internal/pireviewtool/native_test.go; constraints: Static review only; tests were inspected but not executed.policies:conventions— complete (broad); scope: 5 files; examples:docs/checkout-native-review-contract.md,internal/llmadapters/pi_rpc.go,internal/pipeline/prompts.go,internal/pireviewtool/native.go,internal/pireviewtool/tool.go; body-inspected: 5 files; examples:docs/checkout-native-review-contract.md,internal/llmadapters/pi_rpc.go,internal/pipeline/prompts.go,internal/pireviewtool/native.go,internal/pireviewtool/tool.go; skipped: none; constraints: Review limited to assigned convention and review-contract changes; shared organization standards were not provided in the review context. No tests were executed.
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 55s | $0.25 | openai-codex/gpt-6.1-sol | cr 0.10.339-native674
| Field | Value |
|---|---|
| Model | openai-codex/gpt-6.1-sol |
| Reviewers | go:implementation-tests, structure:repo-health, policies:conventions |
| Engine | pi_rpc · openai-codex/gpt-6.1-sol |
| Reviewed by | cr · monit-reviewer |
| Duration | 55s wall · 1m 17s compute |
| Cost | $0.25 |
| Tokens | 98.8k in / 2.9k out |
Per-workstream usage
orchestrator-selection— openai-codex/gpt-6.1-sol- In: 2.6k
- Out: 397
- Cache read: 0
- Cache create: 0
- Cost: $0.01
- Duration: 7s
go:implementation-tests— openai-codex/gpt-6.1-sol- In: 35.1k
- Out: 1.4k
- Cache read: 99.7k
- Cache create: 0
- Cost: $0.09
- Duration: 35s
structure:repo-health— openai-codex/gpt-6.1-sol- In: 28.0k
- Out: 535
- Cache read: 34.8k
- Cache create: 0
- Cost: $0.06
- Duration: 16s
policies:conventions— openai-codex/gpt-6.1-sol- In: 32.2k
- Out: 537
- Cache read: 70.0k
- Cache create: 0
- Cost: $0.08
- Duration: 14s
orchestrator-rollup— openai-codex/gpt-6.1-sol- In: 999
- Out: 79
- Cache read: 0
- Cache create: 0
- Cost: $0.00
- Duration: 2s
Validate linked string tables before processing any entries. Clarify that static coverage requires adequate evidence while unrun execution remains a stated validation limit.
monit-reviewer
approved these changes
Oct 17, 2026
monit-reviewer
left a comment
monit-reviewer
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: c62c4249afad
Profile: open-cli-collective-review - Posting as: monit-reviewer
Summary
| Reviewer | Findings |
|---|---|
| go:implementation-tests | 0 |
| structure:repo-health | 0 |
| policies:conventions | 0 |
Reviewer Coverage
Complete coverage details are retained in the local coverage.json artifact.
go:implementation-tests— complete (constrained); scope: 8 files; examples:internal/llmadapters/pi_rpc.go,internal/llmadapters/pi_rpc_extension_unix_test.go,internal/llmadapters/pi_rpc_test.go,internal/pipeline/prompts.go,internal/pireviewtool/native.go; 3 omitted; body-inspected: 8 files; examples:internal/llmadapters/pi_rpc.go,internal/llmadapters/pi_rpc_extension_unix_test.go,internal/llmadapters/pi_rpc_test.go,internal/pipeline/prompts.go,internal/pireviewtool/native.go; 3 omitted; skipped: none; context (not coverage): 1 file; examples:docs/development.md; constraints: Static review only; tests and fuzz targets could not be executed with the read-only inspection tools.structure:repo-health— complete (constrained); scope: 6 files; examples:docs/checkout-native-review-contract.md,internal/llmadapters/pi_rpc.go,internal/pipeline/prompts.go,internal/pireviewtool/native.go,internal/pireviewtool/native_runtime.go; 1 omitted; body-inspected: 6 files; examples:docs/checkout-native-review-contract.md,internal/llmadapters/pi_rpc.go,internal/pipeline/prompts.go,internal/pireviewtool/native.go,internal/pireviewtool/native_runtime.go; 1 omitted; skipped: none; context (not coverage): 3 files; examples:internal/llmadapters/pi_rpc_extension_unix_test.go,internal/llmadapters/pi_rpc_test.go,internal/pireviewtool/native_test.go; constraints: Static review only; tests, fuzzing, and builds were not executed.policies:conventions— complete (constrained); scope: 5 files; examples:docs/checkout-native-review-contract.md,internal/llmadapters/pi_rpc.go,internal/pipeline/prompts.go,internal/pireviewtool/native.go,internal/pireviewtool/tool.go; body-inspected: 5 files; examples:docs/checkout-native-review-contract.md,internal/llmadapters/pi_rpc.go,internal/pipeline/prompts.go,internal/pireviewtool/native.go,internal/pireviewtool/tool.go; skipped: none; context (not coverage): 3 files; examples:AGENTS.md,CONTRIBUTING.md,docs/development.md; constraints: Review limited to visible repository conventions; canonical shared standards were not available in the pinned checkout. Static inspection only; tests and lint were not executed.
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 39s | $0.21 | openai-codex/gpt-6.1-sol | cr 0.10.339-native674
| Field | Value |
|---|---|
| Model | openai-codex/gpt-6.1-sol |
| Reviewers | go:implementation-tests, structure:repo-health, policies:conventions |
| Engine | pi_rpc · openai-codex/gpt-6.1-sol |
| Reviewed by | cr · monit-reviewer |
| Duration | 39s wall · 48s compute |
| Cost | $0.21 |
| Tokens | 86.7k in / 1.2k out |
Per-workstream usage
go:implementation-tests— openai-codex/gpt-6.1-sol- In: 29.6k
- Out: 488
- Cache read: 88.1k
- Cache create: 0
- Cost: $0.07
- Duration: 20s
structure:repo-health— openai-codex/gpt-6.1-sol- In: 24.6k
- Out: 317
- Cache read: 36.2k
- Cache create: 0
- Cost: $0.06
- Duration: 13s
policies:conventions— openai-codex/gpt-6.1-sol- In: 31.7k
- Out: 299
- Cache read: 86.4k
- Cache create: 0
- Cost: $0.08
- Duration: 11s
orchestrator-rollup— openai-codex/gpt-6.1-sol- In: 809
- Out: 55
- Cache read: 0
- Cache create: 0
- Cost: $0.00
- Duration: 2s
zzwong
marked this pull request as ready for review
October 18, 2026 00:05
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #674.
Compiled libraries currently cannot be inspected through the restricted reviewer tools. An ordinary read rejects the bytes and leaves the assigned file uncovered. This adds explicit ELF inspection, including a single gzip-wrapped ELF, without loading the library or executing repository code.
cr_readwithview="binary"returns hashes for the original and decoded file, section hashes, segment permissions, dynamic symbols, dependencies and loader paths.view="binary-hex"exposes decoded byte ranges. Both views retain checkout confinement, symlink rejection, deadlines and bounded output with continuation offsets. Dynamic section data must agree with the runtime loader’s program mappings; parsing stops at its end marker. Empty symbol tables also require a valid linked string table.A checksum or structural report does not prove executable behavior or equivalence to source. The review guidance requires the complete report and applicable source, build, integrity and consumer evidence. Unsupported or inadequately inspected artifacts remain skipped. The coverage and approval gates are unchanged.
Local validation covers 32/64-bit ELF in both byte orders, compressed and uncompressed inputs, dependency/export/loader-path reporting, report and hex pagination, unsafe paths, corrupt gzip, decompression limits and malformed ELF tables. The restricted-extension test checks diff-first enforcement and exact binary-view requests. A separate real-library probe matched the compressed and decoded manifest hashes without executing the artifact. The full local make check passes with Go 1.26.3 and two concurrent test packages; the native-reader race check passes too. The review found and resolved an empty-table bounds bug; regression coverage exercises both binary views.
All 12 checks passed in CI run 39081500818, including Linux/macOS/Windows builds, lint, the main test suite, repeated race checks and the installed runtime tests. The final CI commit is empty; the reviewed implementation is unchanged.