Conversation
Qodana for GoIt seems all right 👌 No new problems were found according to the checks applied 💡 Qodana analysis was run in the pull request mode: only the changed files were checked Contact Qodana teamContact us at qodana-support@jetbrains.com
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (1)ℹ️ 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. |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review Review loop resumed on the latest head. The prior code/security reviews covered only a3c4d92. This round fixes the stale release-notice assertion, Windows ARM64 guard build configuration, Windows helper startup environment/profile registration, and Windows cache-test/partial-inventory behavior. The sandbox capabilities, memory limits, full native suites and required guard checks remain enforced. Native Windows CI must establish runtime qualification. Qodana's 14 findings were assessed against current source: five errors.As warnings are false positives because *heicdecode.Failure implements error; the nine maintained-codec findings preserve exact source identity and existing explicit handling/API. Exact-file exclusions record these dispositions. Entry-head CodeQL Go and Actions SARIF each contain zero findings, with no PR-ref alerts. Focused regression tests, Windows amd64/arm64 cross-compilation, native/Windows vet, formatting/Qodana configuration checks and GoLand inspections pass. The revised notice guard was negatively verified. Please perform a fresh code review and the configured accompanying security review on this head. |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review Please review current head 11e0583. Both preceding reviews of ae1f7f5 completed without findings; subsequent Windows native results required two startup corrections. AppContainer startup now receives LOCALAPPDATA from the Windows known-folder API alongside the OS SystemRoot and fixed GOMAXPROCS. No parent environment or search path is inherited. The native process now launches but the previous hidden-console mode exited before Go startup with STATUS_DLL_INIT_FAILED, so the pipe-only helper now uses DETACHED_PROCESS. Zero capabilities, child-process denial, inherited-handle allowlist and all job limits are unchanged. Native CI is qualifying this correction. The full Windows guard suite's cache cases now pass. Qodana at ae1f7f5 had zero post-suppression findings. Changed Windows code cross-compiles/vets and has clear GoLand inspections. Fresh security review should accompany this code review. |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 11e0583cd9
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
| & $signtool.FullName verify /pa /all /v /tw $helper | ||
| if ($LASTEXITCODE -ne 0) { throw 'HEIC helper signature verification failed.' } | ||
| $arch = if ($archive.BaseName.EndsWith('-amd64')) { 'amd64' } else { 'arm64' } | ||
| go run ./scripts/heicpackage -mode finalize -os windows -arch $arch -out $unpacked |
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Security: Move manifest finalization outside the signing job
With a malicious change to scripts/heicpackage (or an imported package) merged into a release tag and the release-signing environment approved, this go run executes repository-controlled code after SimplySign authentication and before archive upload. It can reuse the live signer for arbitrary payloads or mutate and re-sign picfetch.exe; signature checks occur before it, with no check before Compress-Archive, so the protected environment and read-only token do not block certificate or release compromise. Move finalization to an unprivileged job, or use fixed trusted commands without a checkout.
SECURITY.md reference: .github/SECURITY.md:L5-L8
Useful? React with 👍 / 👎.
Change
Build an isolated HEIC restoration candidate around the maintained h265 source, compiled only into a reproducible WASI guest. A disposable helper establishes native restrictions before image input; the parent pins its executable and bounds input, output, metadata, diagnostics, jobs and lifetime. Validated NRGBA8/NRGBA64 pixels enter the canonical imaging path through one application-owned admission lane shared with analysis subprocesses.
macOS uses an independently entitled App Sandbox helper. Linux uses a no-cgo helper with synchronized seccomp and resource limits. Windows uses suspended AppContainer creation, explicit inherited pipes and a Job Object. macOS's absent hard total native-memory cap is an accepted limitation; all other bounds remain enforced. The request deadline is 60 seconds. The WASM linear-memory ceiling is separate from native memory: buffer growth can temporarily retain both old and replacement storage.
Package assembly preserves the helper, post-signing hash manifest and exact notices. The updater verifies companion files through persistence and installs them transactionally; macOS replaces the complete signed bundle. The released updater cannot perform the first helper-bearing upgrade: it omits the helper, deletes staging and leaves an invalid enclosing macOS signature. That transition requires a complete package reinstall or a separately qualified migration step before release.
HEIC viewing remains disabled. Production activation awaits current native CI, distribution-package verification and broader camera qualification. ICC/wide-gamut/PQ/HLG/gain-map display is not implemented; these color classes are not explicitly rejected.
Historical reconciliation
Account for the original
feature/heic-hardeningcommitsfc127b44and73cb3c9without merging their superseded worker architecture. All 107 maintained decoder source/license files are byte-identical to the original hardening commit. Restore the current app-wide threat model, Qodana YAML validation, four unchanged ordinary compatibility fixtures and the generated Fyne metadata ignore. Strengthen alpha-ramp coverage. Historical eager WASM allocation was evaluated and excluded after it broke both native Linux architectures; the prior runtime configuration and every security limit are preserved.The path-by-path decisions, fixture provenance and limits of test equivalence are recorded in
docs/heic/history-reconciliation.md. This does not claim the full historical decoder suite has been restored or passed.Validation
make verify-buildpasses formatting, TUF, generated assets/notices, exact Qodana exclusions, source/artifact reproducibility, import guards, vet and build. Regeneration changes the fixture input manifest but leaves the guest binary unchanged.make heic-native-macospasses all four mandatory Apple Silicon helper/runtime and signed-update guards. Both the baseline runtime and the subsequently withdrawn allocation trial passed these local controls. The compiler decodes the ordinary 12MP fixture; the interpreter is terminated and joined at its finite deadline.52ed2df, Linux amd64/arm64 and macOS Intel/Apple Silicon native guards, all UI race shards and ordinary Windows tests passed. Windows HEIC guards and the non-UI Linux race job failed. The eager-allocation trial at30f5b10also failed Linux native decoding; correction08a71f2restores the prior runtime. Both Linux amd64 and arm64 native isolation/decode guards now pass on08a71f2in CI run 35070025776. Other fresh CI and review gates remain open. Local canonical shard validation could not run because Docker was unavailable; no UI test or shard assignment changed.ae1f7f5has zero post-suppression findings; both Codex reviews of that commit reported no findings. Subsequent Windows startup corrections are undergoing fresh native CI and code/security review on11e0583.Evidence:
THREAT-MODEL.md,docs/heic/history-reconciliation.md,docs/heic/qualification.md,docs/heic/compatibility-2026-09-16.mdandplans/2026-09-15-isolated-heic-restoration.md.Implementation checkpoint
52ed2df, reconciliation30f5b10and native-regression correction08a71f2are signed. The initial cloud foundation is unsigned; signing the complete PR range remains a merge prerequisite. No history rewrite, merge or release is included.