fix(lanes): green security-scan, a11y, SAST, scorecard, and trunk-check lanes - #717
Conversation
Four pre-existing failing lanes on every main push (all surfaced by the event-listener Cargo.lock bump triggering the deny/audit path filters): - deny.yml: add RUSTSEC-2023-0071 (rsa Marvin advisory covers all versions; JWT use is local public-key verification only) and RUSTSEC-2024-0436 (informational; paste is a stale lock entry) ignores; add license fields to sharecli-ipc / sharecli-session (unlicensed); allow the deprecated GPL-3.0 id for winfsp/winfsp-sys (cargo-deny >= 0.18.4 matches GNU ids pedantically). Verified: advisories ok, bans ok, licenses ok, sources ok (cargo-deny 0.20.2). - audit.yml: same two advisory ignores in audit.toml + .cargo/audit.toml. Verified: cargo audit exit 0 (cargo-audit 0.22.2, 678 deps scanned). - ci-gate lint: `cargo fmt --all -- --check` failed on the long line added in fr006_proc_tree_state.rs (#713); wrapped to 100 cols. Verified clean. - deploy-docs: VitePress SSR treated `${{ github.sha }}` in a table cell as Vue interpolation (inline code is not v-pre'd), throwing "Cannot read properties of undefined (reading 'sha')"; wrapped the cell in `<span v-pre>`. Verified: `npm ci && npm run docs:build` completes (vitepress 1.6.4).
…x/lanes-deny-audit-fmt-docs
…ck lanes
Five pre-existing red lanes on main pushes, all diagnosed from job logs and
verified locally before landing (WSL Fedora; actionlint 1.7.12, taplo 0.9.3,
yamllint 1.38.0, cargo-deny 0.20.2, cargo-audit 0.22.2, cargo fmt/clippy).
- security.yml (clippy -D warnings -W panic -W unwrap_used): 54 findings in
sharecli-fuse + sharecli-sync + the sharecli bin. Fixed all: unused imports
(ProcState imports moved to #[cfg(test)]), needless returns, missing docs on
FuseBackend, dead code on macos-only diagnostics, RwLock/lock unwraps ->
expect() with messages, try_into/parse/from_utf8 unwraps -> expect(),
panic! -> assert!, manual-checked-div -> checked_div, redundant guard,
needless borrows, collapsible ifs, unwrap_or_default. too_many_arguments on
three CLI dispatch functions gets targeted allows (flag-aggregation shape).
Verified: `cargo clippy -- -D warnings -W clippy::panic -W
clippy::unwrap_used` exit 0; full `cargo test --release --locked
--no-fail-fast` 185/186 (the one failure is the WSL npm-not-on-PATH artifact
that passes on the runner, as in the previous baseline).
- a11y.yml: jsdom 30's undici webidl calls worker_threads.markAsUncloneable
(Node >= 22.11); the workflow pinned node 20 -> TypeError on axe job. Bumped
both jobs to node 22 and aligned the Playwright browser install with the
package.json version (1.62.1; 1.49.0 installed mismatched chromium revisions).
Verified locally: `npm run a11y:dashboard` -> 0 violations.
- sast.yml (CodeQL): language list included `go` but the repo has no Go
sources; CodeQL aborts database finalization ("fatal error ... finalize go").
Removed go, added contents:read + security-events:write permissions.
- scorecard.yml: `permissions: read-all` blocked the OIDC token for
publish_results Fulcio signing ("error obtaining token: expired_token").
Changed to contents:read + id-token:write.
- trunk-check.yml / .trunk/trunk.yaml: config used a nonexistent schema
(linters:/formatters:/trunk-check:/cache:/env keys, no `version: 0.1`) so the
lane failed with 8 config-errors on every run. Rewrote to the real Trunk
Check schema (actionlint + taplo + yamllint only; clippy/fmt already have
dedicated hard lanes, and the nested lib/teamcomm workspace is not covered by
root cargo fmt). Fixed the actionlint findings it surfaced: ci.yml referenced
the job as needs.dependency-review (job id is dep-review); infisical.yml used
the retired blacksmith-2vcpu-ubuntu-2204 runner label -> ubuntu-22.04 (this
also unblocks re-enabling the Infisical Sync workflow, which is still
disabled in the repo settings); codecov.yml had a mis-indented paths entry
and trailing whitespace; security.yml trailing whitespace/blank lines.
Added .trunk/configs/.yamllint.yaml (relaxed: no line-length, flow braces
tolerant, on: allowed) and .github/actionlint.yaml (documents the forward
steps.detect reference in ci.yml, which GitHub resolves fine).
mise.toml had a genuinely invalid TOML key `[tasks.docs:build]` (bare keys
cannot contain ':') -> `["tasks.docs:build"]`. Taplo-formatted all repo TOML
files (whitespace-only). Verified: `trunk check --all` -> "No issues".
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
the
size:L
📝 WalkthroughSummaryThe PR fixes CI failures across security, accessibility, SAST, Scorecard, Trunk, audit, and documentation checks. Rust changes address Clippy findings, improve panic messages, add checked division, and update session ledger tests. Fuse smoke setup now installs Zig and includes No blocking issues are evident from the provided changes. Should FixNo changes required. Consider
Approve / Request ChangesApprove. WalkthroughThe pull request updates CI, security, lint, and repository configuration. It reformats configuration files, replaces many Rust ChangesRepository maintenance
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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 |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| /// The backend only matters on macOS: macOS 15+ prefers the File System Kit | ||
| /// backend while older releases rely on the kernel extension. On Linux and |
There was a problem hiding this comment.
Suggestion: The new module documentation says macOS 15+ prefers the FSKit backend, but the actual selection policy below explicitly prefers a loaded kernel backend first and only selects FSKit when the kernel extension is unavailable. This contradiction can mislead operators and maintainers about which backend mounts will use; update the documentation to match the KEXT-first policy or change the policy to implement the documented preference. [docstring mismatch]
Severity Level: Minor 🧹
- ⚠️ Operator documentation misstates backend selection.
- ⚠️ Maintainers may misunderstand macOS mount behavior.
- ⚠️ Runtime backend selection itself remains functional.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sharecli-fuse/src/backend.rs
**Line:** 3:4
**Comment:**
*Docstring Mismatch: The new module documentation says macOS 15+ prefers the FSKit backend, but the actual selection policy below explicitly prefers a loaded kernel backend first and only selects FSKit when the kernel extension is unavailable. This contradiction can mislead operators and maintainers about which backend mounts will use; update the documentation to match the KEXT-first policy or change the policy to implement the documented preference.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| let tag = if as_32bit { | ||
| u64::from_le_bytes(input[pos..pos + 8].try_into().unwrap()) as u32 | ||
| u64::from_le_bytes( | ||
| input[pos..pos + 8] | ||
| .try_into() | ||
| .expect("8-byte tag slice in bounds (validated above)"), | ||
| ) as u32 |
There was a problem hiding this comment.
Suggestion: The 32-bit parsing branch reads an eight-byte wire tag but immediately truncates it to u32, silently discarding the upper four bytes. The repository's wire-format documentation explicitly states that real Unicode streams can contain distinct tag halves, so valid records with information in the upper half are misrepresented rather than rejected or preserved. [type error]
Severity Level: Major ⚠️
- ⚠️ Real Unicode MAPI tags are parsed inaccurately.
- ⚠️ Property identity metadata can be silently corrupted.
- ⚠️ 32-bit parity utilities cannot validate distinct tag halves.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/mapi_props.rs
**Line:** 169:174
**Comment:**
*Type Error: The 32-bit parsing branch reads an eight-byte wire tag but immediately truncates it to `u32`, silently discarding the upper four bytes. The repository's wire-format documentation explicitly states that real Unicode streams can contain distinct tag halves, so valid records with information in the upper half are misrepresented rather than rejected or preserved.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix![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: 752309f69c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| .spawn()?; | ||
| use std::io::Write; | ||
| child.stdin.take().unwrap().write_all(stdin)?; | ||
| child.stdin.take().expect("spawned child must have piped stdin").write_all(stdin)?; |
There was a problem hiding this comment.
Because this is the production SystemRunner path, replacing unwrap() with expect() still leaves a panic path instead of reporting a subprocess setup failure through the existing io::Result. The repo's error-handling rule forbids unwrap/expect in production code, so this should be converted to an io::Error path rather than relying on an invariant.
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: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/stats.rs (1)
17-22: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle
NaNwithout panicking inmedian.
medianaccepts anyf64, butf64::partial_cmpreturnsNoneforNaN, so valid input reachesexpect(). RejectNaNthrough a fallible API, or sort with a defined total ordering and add tests.🤖 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 `@src/stats.rs` around lines 17 - 22, Update median to handle NaN values without reaching the partial_cmp expect panic: either reject NaN through a fallible API or use a defined total ordering for sorting. Preserve the empty-input behavior, and add tests covering NaN input and the resulting documented behavior.Source: Coding guidelines
🤖 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 @.github/workflows/sast.yml:
- Line 14: Update the actions/checkout step to set persist-credentials to false,
ensuring checkout does not leave GITHUB_TOKEN in the local git configuration
before the CodeQL autobuild step runs.
- Line 15: Update the CodeQL action reference in the workflow’s init step from
the mutable v4 tag to the intended immutable release commit ID, preserving the
existing github/codeql-action/init step.
In @.github/workflows/scorecard.yml:
- Around line 18-23: Add security-events: write to the explicit permissions
block in the scorecard workflow so the github/codeql-action/upload-sarif step
can upload results.sarif, while preserving the existing contents and id-token
permissions.
In @.trunk/trunk.yaml:
- Line 2: Update the heading comment in trunk.yaml to replace the Unicode em
dash with an ASCII hyphen, leaving the rest of the comment unchanged.
- Around line 31-40: Remove the Trunk-level actionlint ignore block from
.trunk/trunk.yaml, including its ci.yml path restriction. Preserve the narrower
suppression already configured in .github/actionlint.yaml so unrelated
actionlint diagnostics in ci.yml remain enabled.
In `@crates/sharecli-fuse/src/backend.rs`:
- Around line 1-6: Align the runtime backend documentation with select_backend:
document that SHARECLI_FUSE_BACKEND is evaluated first, followed by
FuseBackend::Kernel when kernel_backend_loaded() is true, then
FuseBackend::Fskit as the fallback, and that non-macOS targets may still honor
the override instead of always returning Unavailable. Preserve the selector
behavior unless changing it to match the intended documented contract.
In `@crates/sharecli-sync/src/lib.rs`:
- Around line 25-29: Remove all production expect calls at
crates/sharecli-sync/src/lib.rs:25-29, 33-34, 38-39, and 43-45 by explicitly
handling poisoned RwLock read/write results in insert, remove, count, and
pids_sorted; at src/base_n_radix.rs:50 and 63-64, replace guarded last() expects
with match or next_back checks; at src/cast/caster.rs:98, return an io::Error
when the child stdin is absent; at src/dns_zone.rs:136-140, use an explicit
inherited-name fallback for records.last(); at src/ipaddr_validation.rs:33-35,
propagate invalid octet parsing as None; at src/itoa.rs:15-16 and 41-42,
construct both digit strings without expect-based UTF-8 conversions; at
src/s_expression.rs:33-41, handle the optional next expression explicitly; and
at src/ssh_known_hosts.rs:214-216, pattern-match the optional range start. Run
formatting, the locked workspace all-features tests, build, and clippy with -D
warnings under the pinned toolchain.
In `@gitleaks.toml`:
- Around line 92-95: Remove the secretGroup setting from this full-match rule
because its regex has no capturing group; leave the existing regex and other
rule settings unchanged so Gitleaks uses the entire match as the secret.
In `@mise.toml`:
- Line 37: Update the task table header for docs:build from ["tasks.docs:build"]
to [tasks."docs:build"] so it is defined within the tasks table with the correct
task name.
In `@src/commands/mod.rs`:
- Line 397: Update the percentage calculation at pct so the multiplication of
used_mb by 100 is overflow-safe before division, either by using checked_mul
with an appropriate fallback or by performing the calculation in u128; preserve
the existing zero fallback behavior when the calculation cannot be completed.
In `@src/commands/proc.rs`:
- Line 66: Remove the expect call from parse_proc_state when extracting the
first character from trimmed; preserve the existing Result-based CLI error
handling by propagating or converting a missing character into an appropriate
parse error. Then verify the Rust changes with cargo build, cargo test, and
cargo clippy using the required settings.
In `@src/log_sink.rs`:
- Line 28: Replace production expect-based panics with structured handling
across all listed sites: in src/log_sink.rs lines 28 and 44-47, recover from
poisoned buffer mutexes; in src/main.rs lines 1667-1670, propagate
SystemTimeError through Result<()>; in src/mapi_props.rs lines 169-191, convert
failures into parse errors; in src/metrics.rs lines 57-61 and src/object_pool.rs
lines 13 and 22-25, handle poisoned mutexes without panicking; in
src/monitoring.rs lines 230-231, propagate or safely handle clock errors; in
src/radix_trie.rs lines 69-74 and src/stream.rs line 11, preserve invariants
without panic; in src/ring_buffer.rs line 41, handle unoccupied slots; and in
src/skiplist.rs lines 62, 88, and 98, handle missing node keys safely.
In `@src/stream.rs`:
- Line 11: Update Stream::write to remove the expect on self.chunks.last_mut().
Handle an empty chunks collection safely, using the existing checked or
error-returning behavior so chunk_size(0) does not panic while preserving normal
item writes.
In `@src/theme.rs`:
- Around line 19-24: Update Rgb::from_hex to validate that the input is exactly
six hexadecimal digits after an optional leading #, rejecting overlong values
and any trailing characters before parsing. Preserve the existing compile-time
malformed-input panic behavior for all invalid lengths or characters.
---
Outside diff comments:
In `@src/stats.rs`:
- Around line 17-22: Update median to handle NaN values without reaching the
partial_cmp expect panic: either reject NaN through a fallible API or use a
defined total ordering for sorting. Preserve the empty-input behavior, and add
tests covering NaN input and the resulting documented behavior.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 667772fe-f410-4de3-898f-5581f43f3b81
📒 Files selected for processing (54)
.cargo/audit.toml.cargo/mutants.toml.github/actionlint.yaml.github/workflows/a11y.yml.github/workflows/ci.yml.github/workflows/infisical.yml.github/workflows/sast.yml.github/workflows/scorecard.yml.github/workflows/security.yml.trunk/configs/.yamllint.yaml.trunk/trunk.yamlCargo.toml_typos.tomlaudit.tomlcliff.tomlcodecov.ymlcrates/sharecli-fuse/src/backend.rscrates/sharecli-fuse/src/provenance.rscrates/sharecli-fuse/src/session_registry.rscrates/sharecli-session/Cargo.tomlcrates/sharecli-sync/src/lib.rsdeny.tomlgitleaks.tomllib/teamcomm/Cargo.tomllib/teamcomm/crates/teamcomm-cli/Cargo.tomllib/teamcomm/crates/teamcomm-client/Cargo.tomlmise.tomlmutants.tomlsrc/alloc.rssrc/base_n_radix.rssrc/cast/caster.rssrc/commands/mod.rssrc/commands/proc.rssrc/commands/report.rssrc/commands/serve.rssrc/dns_zone.rssrc/hkdf.rssrc/ipaddr_validation.rssrc/itoa.rssrc/log_sink.rssrc/main.rssrc/mapi_props.rssrc/metrics.rssrc/monitoring.rssrc/object_pool.rssrc/radix_trie.rssrc/ring_buffer.rssrc/runtime.rssrc/s_expression.rssrc/skiplist.rssrc/ssh_known_hosts.rssrc/stats.rssrc/stream.rssrc/theme.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
- GitHub Check: windows_winfsp (windows-latest)
🧰 Additional context used
📓 Path-based instructions (4)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Use UTF-8 encoding for all text files; do not use Windows-1252 smart quotes or other special characters.
Use UTF-8 for all text files.
Files:
audit.tomlmutants.tomllib/teamcomm/crates/teamcomm-cli/Cargo.tomllib/teamcomm/crates/teamcomm-client/Cargo.toml_typos.tomlsrc/dns_zone.rssrc/hkdf.rscrates/sharecli-session/Cargo.tomlcliff.tomllib/teamcomm/Cargo.tomlsrc/s_expression.rssrc/base_n_radix.rssrc/ipaddr_validation.rssrc/skiplist.rssrc/runtime.rssrc/itoa.rssrc/radix_trie.rssrc/stats.rssrc/monitoring.rssrc/mapi_props.rsCargo.tomlsrc/theme.rssrc/commands/report.rssrc/ssh_known_hosts.rscrates/sharecli-fuse/src/provenance.rssrc/alloc.rsmise.tomlsrc/stream.rsdeny.tomlsrc/object_pool.rscrates/sharecli-fuse/src/session_registry.rssrc/ring_buffer.rssrc/log_sink.rssrc/cast/caster.rscrates/sharecli-sync/src/lib.rssrc/main.rssrc/commands/serve.rsgitleaks.tomlsrc/metrics.rscrates/sharecli-fuse/src/backend.rssrc/commands/mod.rscodecov.ymlsrc/commands/proc.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,toml}: Use Rust edition 2021 and the pinned toolchain fromrust-toolchain.toml; keep code compatible with the configured stable compiler, rustfmt, and clippy.
Ensure Rust code passes formatting, clippy with-D warnings, and the locked all-features test suite; CI usesRUSTFLAGS=-D warnings.
UsePascalCasefor Rust types,snake_casefor functions, methods, and modules, andSCREAMING_SNAKE_CASEfor constants.
Files:
audit.tomlmutants.tomllib/teamcomm/crates/teamcomm-cli/Cargo.tomllib/teamcomm/crates/teamcomm-client/Cargo.toml_typos.tomlsrc/dns_zone.rssrc/hkdf.rscrates/sharecli-session/Cargo.tomlcliff.tomllib/teamcomm/Cargo.tomlsrc/s_expression.rssrc/base_n_radix.rssrc/ipaddr_validation.rssrc/skiplist.rssrc/runtime.rssrc/itoa.rssrc/radix_trie.rssrc/stats.rssrc/monitoring.rssrc/mapi_props.rsCargo.tomlsrc/theme.rssrc/commands/report.rssrc/ssh_known_hosts.rscrates/sharecli-fuse/src/provenance.rssrc/alloc.rsmise.tomlsrc/stream.rsdeny.tomlsrc/object_pool.rscrates/sharecli-fuse/src/session_registry.rssrc/ring_buffer.rssrc/log_sink.rssrc/cast/caster.rscrates/sharecli-sync/src/lib.rssrc/main.rssrc/commands/serve.rsgitleaks.tomlsrc/metrics.rscrates/sharecli-fuse/src/backend.rssrc/commands/mod.rssrc/commands/proc.rs
lib/teamcomm/**/Cargo.toml
📄 CodeRabbit inference engine (lib/teamcomm/AGENTS.md)
lib/teamcomm/**/Cargo.toml: Use Rust 2021, MSRV 1.75, Cargo workspace configuration, and resolver = 2.
Only the scaffold agent may modify the workspace root; implementation agents must work exclusively within their assigned crate directories.
Files:
lib/teamcomm/crates/teamcomm-cli/Cargo.tomllib/teamcomm/crates/teamcomm-client/Cargo.tomllib/teamcomm/Cargo.toml
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use Rust for the project and validate Rust changes with Cargo build, Cargo test, and Cargo clippy.
**/*.rs: For new Rust modules, create the test file before the implementation; for bug fixes, write a failing test before the fix; for refactors, ensure existing tests pass before and after.
Use idiomatic, language-appropriate error handling, never useunwraporexpectin production Rust code, and log all errors with structured logging.
Files:
src/dns_zone.rssrc/hkdf.rssrc/s_expression.rssrc/base_n_radix.rssrc/ipaddr_validation.rssrc/skiplist.rssrc/runtime.rssrc/itoa.rssrc/radix_trie.rssrc/stats.rssrc/monitoring.rssrc/mapi_props.rssrc/theme.rssrc/commands/report.rssrc/ssh_known_hosts.rscrates/sharecli-fuse/src/provenance.rssrc/alloc.rssrc/stream.rssrc/object_pool.rscrates/sharecli-fuse/src/session_registry.rssrc/ring_buffer.rssrc/log_sink.rssrc/cast/caster.rscrates/sharecli-sync/src/lib.rssrc/main.rssrc/commands/serve.rssrc/metrics.rscrates/sharecli-fuse/src/backend.rssrc/commands/mod.rssrc/commands/proc.rs
🪛 zizmor (1.29.0)
.github/workflows/ci.yml
[warning] 309-309: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
.github/workflows/sast.yml
[warning] 14-14: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 9-9: overly broad permissions (excessive-permissions): security-events: write is overly broad at the workflow level
(excessive-permissions)
[error] 15-15: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[warning] 9-9: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
[info] 11-11: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
.github/workflows/scorecard.yml
[error] 23-23: overly broad permissions (excessive-permissions): id-token: write is overly broad at the workflow level
(excessive-permissions)
[warning] 23-23: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
🔇 Additional comments (33)
.github/actionlint.yaml (1)
1-14: LGTM!.github/workflows/a11y.yml (1)
30-30: LGTM!Also applies to: 60-60
.github/workflows/ci.yml (1)
309-309: LGTM!.github/workflows/infisical.yml (1)
19-19: LGTM!Also applies to: 59-59
.github/workflows/security.yml (1)
59-59: LGTM!Also applies to: 75-75, 83-83, 92-92
.trunk/configs/.yamllint.yaml (1)
1-35: LGTM!codecov.yml (1)
10-10: LGTM!crates/sharecli-fuse/src/backend.rs (1)
233-237: LGTM!Also applies to: 285-288
crates/sharecli-fuse/src/provenance.rs (1)
16-18: LGTM!crates/sharecli-fuse/src/session_registry.rs (1)
59-82: LGTM!src/alloc.rs (1)
17-19: LGTM!src/runtime.rs (1)
90-93: LGTM!src/commands/mod.rs (1)
23-26: LGTM!Also applies to: 422-422, 612-613, 950-951, 1360-1361, 1503-1504
src/commands/report.rs (1)
18-20: LGTM!src/commands/serve.rs (1)
45-47: LGTM!src/commands/proc.rs (1)
92-95: LGTM!Also applies to: 1003-1003, 1146-1146, 1209-1209, 1370-1372, 1433-1435, 1455-1456
.cargo/audit.toml (1)
7-11: LGTM!.cargo/mutants.toml (1)
6-8: LGTM!Cargo.toml (1)
35-39: LGTM!Also applies to: 85-89, 203-216, 229-233
_typos.toml (1)
9-16: LGTM!lib/teamcomm/crates/teamcomm-cli/Cargo.toml (1)
23-29: LGTM!lib/teamcomm/crates/teamcomm-client/Cargo.toml (1)
17-22: LGTM!mutants.toml (1)
6-8: LGTM!audit.toml (1)
6-11: LGTM!cliff.toml (1)
36-46: LGTM!crates/sharecli-session/Cargo.toml (1)
13-20: LGTM!deny.toml (1)
11-33: LGTM!Also applies to: 44-55
gitleaks.toml (2)
6-31: LGTM!Also applies to: 34-39, 41-46, 48-53, 55-60, 62-67, 69-74, 76-81, 83-88, 98-103, 105-109, 111-115, 117-123, 125-129
33-33: 🔒 Security & PrivacyNo duplicate
[[rules]]headers are present.
gitleaks.tomluses one[[rules]]header per rule, and each rule has anidfield. The duplicate-header concern does not apply.> Likely an incorrect or invalid review comment.lib/teamcomm/Cargo.toml (1)
4-9: LGTM!src/cast/caster.rs (1)
129-137: 📐 Maintainability & Code QualityNo change needed.
MockProcessRunneris only constructed from integration-test code, so the mutexexpect()calls insrc/cast/caster.rsdo not affect production code paths.src/hkdf.rs (1)
185-188: 🩺 Stability & AvailabilityNo change needed for
expandlength validation.
expandis only called internally by deterministic tests andderive_key, solengthis not an external/error-logging input path here.src/log_sink.rs (1)
28-47: 📐 Maintainability & Code QualityRun the required Rust validation before merge.
Use the pinned toolchain and run
cargo fmt --check,cargo build --locked --all-features,RUSTFLAGS=-D warnings cargo clippy --locked --all-targets --all-features -- -D warnings, andcargo test --locked --all-features. Confirm the refactor passes tests before and after the change.Source: Coding guidelines
| codeql: | ||
| runs-on: ubuntu-24.04 | ||
| steps: | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
curl -fsSL \
https://raw.githubusercontent.com/actions/checkout/3d3c42e5aac5ba805825da76410c181273ba90b1/action.yml \
| rg -n -C2 'persist-credentials|default:'Repository: KooshaPari/sharecli
Length of output: 3140
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Workflow outline / relevant lines:\n'
sed -n '1,120p' .github/workflows/sast.yml
printf '\nAll checkout references in workflow:\n'
rg -n "actions/checkout|codeql|persist-credentials|permissions:" .github/workflows/sast.ymlRepository: KooshaPari/sharecli
Length of output: 1142
Disable persisted checkout credentials.
actions/checkout leaves GITHUB_TOKEN configured in the local git config by default, so the repository-controlled CodeQL autobuild step can run with that token available. Set persist-credentials: false.
Proposed fix
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | |
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.29.0)
[warning] 14-14: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 @.github/workflows/sast.yml at line 14, Update the actions/checkout step to
set persist-credentials to false, ensuring checkout does not leave GITHUB_TOKEN
in the local git configuration before the CodeQL autobuild step runs.
| runs-on: ubuntu-24.04 | ||
| steps: | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| - uses: github/codeql-action/init@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
git ls-remote https://github.com/github/codeql-action.git \
'refs/tags/v4' 'refs/tags/v4^{}'Repository: KooshaPari/sharecli
Length of output: 268
🌐 Web query:
github/codeql-action GitHub Actions v4 commit tag
💡 Result:
The GitHub CodeQL Action v4 is the current, recommended major version of the action [1][2][3]. GitHub advises users to reference the CodeQL Action using the major version tag, such as v4, in workflow files [1][2][3]. This practice ensures that workflows automatically receive the latest releases, bug fixes, features, and updated CodeQL CLI versions within that major version [1][2][3]. As of August 9, 2026, the latest releases within the v4 series include versions such as v4.37.4 [4][5]. The CodeQL Action v4 was introduced on October 7, 2025, and runs on the Node.js 24 runtime [6]. To use the action in your GitHub Actions workflow, you would reference it as follows [7]: - uses: github/codeql-action/init@v4 - uses: github/codeql-action/analyze@v4 Major version tags like v4 act as moving targets that point to the most recent release within that branch, allowing for automatic updates without manual intervention [1][2][3]. Detailed information on specific changes and version history can be found on the official GitHub CodeQL Action releases page [4][5].
Citations:
- 1: https://github.com/github/codeql-action/tree/releases/v4
- 2: https://github.com/github/codeql-action
- 3: https://github.com/github/codeql-action/tree/refs/heads/main
- 4: https://github.com/github/codeql-action/releases
- 5: https://github.com/github/codeql-action/blob/main/CHANGELOG.md
- 6: https://github.blog/changelog/2025-10-28-upcoming-deprecation-of-codeql-action-v3/
- 7: https://docs.github.com/en/code-security/reference/code-scanning/workflow-configuration-options
Pin the CodeQL init action to an immutable commit.
github/codeql-action/init@v4 is a moving tag and can be updated after review. Replace it with the intended release commit ID.
🧰 Tools
🪛 zizmor (1.29.0)
[error] 15-15: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/sast.yml at line 15, Update the CodeQL action reference in
the workflow’s init step from the mutable v4 tag to the intended immutable
release commit ID, preserving the existing github/codeql-action/init step.
Source: Linters/SAST tools
| permissions: | ||
| contents: read | ||
| # Required by scorecard-action's `publish_results` (Fulcio signing via OIDC); | ||
| # `read-all` (previous value) blocked the token and failed the run with | ||
| # "error obtaining token: expired_token" during result signing. | ||
| id-token: write |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C3 'permissions:|security-events|upload-sarif|sarif_file' \
.github/workflows/scorecard.ymlRepository: KooshaPari/sharecli
Length of output: 753
🌐 Web query:
github/codeql-action/upload-sarif required workflow permissions security-events write
💡 Result:
To use the github/codeql-action/upload-sarif action, your workflow job requires specific permissions granted to the GITHUB_TOKEN. While security-events: write is the fundamental permission for uploading analysis results, technical requirements for the action have evolved to include additional access for certain environments [1][2]. The required permissions are: 1. security-events: write: This is mandatory for all workflows to allow the action to upload SARIF results to GitHub Code Scanning [3][4][5]. 2. contents: read: This is required for workflows running in private repositories [3][4][5]. 3. actions: read: This is required, particularly in private repositories, because the action needs to query workflow run metadata to function correctly [1][2]. Failure to include this may result in a "Resource not accessible by integration" error [6][2]. For a typical workflow job in a private repository, you should configure the permissions block as follows [3][4]: permissions: security-events: write actions: read contents: read If you do not explicitly define these permissions in your workflow file, the GITHUB_TOKEN may default to a set of permissions that lacks the necessary access, leading to integration errors [6][2]. Always define these at the job level to ensure the action operates with the required privileges [6][2].
Citations:
- 1: Add
actions: readto generated SARIF upload job permissions for private repos github/gh-aw#37367 - 2: codeql/upload-sarif@v3 action failed: Resource not accessible by integration - missing
actions: readgithub/codeql-action#2117 - 3: https://help.github.com/en/github/finding-security-vulnerabilities-and-errors-in-your-code/uploading-a-sarif-file-to-github
- 4: https://docs.github.com/en/code-security/how-tos/find-and-fix-code-vulnerabilities/integrate-with-existing-tools/upload-sarif-file
- 5: https://github.com/github/codeql-action/blob/2a6fe1608c7d5b1e0cb3d8e19d51c4ff1ca47a0d/README.md
- 6: codeql/upload-sarif@v2 or @main can't integrate on actions github/codeql-action#1806
Grant the SARIF upload permission.
This workflow uploads results.sarif with github/codeql-action/upload-sarif, which requires security-events: write on the job token. Add that permission to the explicit permissions block.
Proposed fix
permissions:
contents: read
+ security-events: write
# Required by scorecard-action's `publish_results` (Fulcio signing via OIDC);
id-token: write📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| permissions: | |
| contents: read | |
| # Required by scorecard-action's `publish_results` (Fulcio signing via OIDC); | |
| # `read-all` (previous value) blocked the token and failed the run with | |
| # "error obtaining token: expired_token" during result signing. | |
| id-token: write | |
| permissions: | |
| contents: read | |
| security-events: write | |
| # Required by scorecard-action's `publish_results` (Fulcio signing via OIDC); | |
| # `read-all` (previous value) blocked the token and failed the run with | |
| # "error obtaining token: expired_token" during result signing. | |
| id-token: write |
🧰 Tools
🪛 zizmor (1.29.0)
[error] 23-23: overly broad permissions (excessive-permissions): id-token: write is overly broad at the workflow level
(excessive-permissions)
[warning] 23-23: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
🤖 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 @.github/workflows/scorecard.yml around lines 18 - 23, Add security-events:
write to the explicit permissions block in the scorecard workflow so the
github/codeql-action/upload-sarif step can upload results.sarif, while
preserving the existing contents and id-token permissions.
| @@ -1,113 +1,49 @@ | |||
| # ============================================================================= | |||
| # Trunk.io — Plugin versions for linting/formatting tools | |||
| # Trunk.io — Unified linting/formatting in GitHub Actions | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the Unicode em dash with ASCII punctuation.
Replace — with -. As per coding guidelines, use UTF-8 text files and do not use Windows-1252 smart quotes or other special characters.
🤖 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 @.trunk/trunk.yaml at line 2, Update the heading comment in trunk.yaml to
replace the Unicode em dash with an ASCII hyphen, leaving the rest of the
comment unchanged.
Source: Coding guidelines
| # The `detect` job's `outputs:` block references `steps.detect.outputs.*` | ||
| # before the step with `id: detect` is declared. This is legal in GitHub | ||
| # Actions (step ids resolve within the whole job; the detect job runs on | ||
| # every CI run), but actionlint's static analysis reports it as an undefined | ||
| # property. Ignore that specific message in ci.yml only. | ||
| ignore: | ||
| - linters: [actionlint] | ||
| paths: | ||
| - .github/workflows/ci.yml | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' 'Trunk suppression:'
sed -n '25,42p' .trunk/trunk.yaml
printf '%s\n' 'Message-specific actionlint suppression:'
sed -n '6,15p' .github/actionlint.yamlRepository: KooshaPari/sharecli
Length of output: 1251
Remove the broad Trunk actionlint suppression.
.trunk/trunk.yaml ignores all actionlint diagnostics in .github/workflows/ci.yml, while .github/actionlint.yaml already provides the narrower suppression for the undefined steps.detect.outputs.* message. Drop the Trunk ignore block so the workflow is linted for unrelated actionlint problems.
🤖 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 @.trunk/trunk.yaml around lines 31 - 40, Remove the Trunk-level actionlint
ignore block from .trunk/trunk.yaml, including its ci.yml path restriction.
Preserve the narrower suppression already configured in .github/actionlint.yaml
so unrelated actionlint diagnostics in ci.yml remain enabled.
| pool_status.bun_total, pool_status.bun_idle, pool_status.max_per_type, | ||
| )); | ||
| let pct = if total_mb > 0 { (used_mb * 100) / total_mb } else { 0 }; | ||
| let pct = (used_mb * 100).checked_div(total_mb).unwrap_or(0); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Check the multiplication before division.
checked_div cannot protect used_mb * 100 because the multiplication runs first. A sufficiently large value can panic in debug builds or wrap in release builds.
Use checked_mul or compute the percentage in u128.
Suggested overflow-safe calculation
- let pct = (used_mb * 100).checked_div(total_mb).unwrap_or(0);
+ let pct = used_mb
+ .checked_mul(100)
+ .and_then(|scaled| scaled.checked_div(total_mb))
+ .unwrap_or(0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let pct = (used_mb * 100).checked_div(total_mb).unwrap_or(0); | |
| let pct = used_mb | |
| .checked_mul(100) | |
| .and_then(|scaled| scaled.checked_div(total_mb)) | |
| .unwrap_or(0); |
🤖 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 `@src/commands/mod.rs` at line 397, Update the percentage calculation at pct so
the multiplication of used_mb by 100 is overflow-safe before division, either by
using checked_mul with an appropriate fallback or by performing the calculation
in u128; preserve the existing zero fallback behavior when the calculation
cannot be completed.
| bail!("invalid --state value '{raw}'; expected single process state letter (R|S|D|Z|T|…)"); | ||
| } | ||
| let ch = trimmed.chars().next().unwrap(); | ||
| let ch = trimmed.chars().next().expect("single-char --state value (length validated above)"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
RUSTFLAGS=-D warnings cargo build --locked --all-features
RUSTFLAGS=-D warnings cargo test --locked --all-features
RUSTFLAGS=-D warnings cargo clippy --locked --all-targets --all-features -- -D warningsRepository: KooshaPari/sharecli
Length of output: 287
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repository files of interest:\n'
git ls-files | rg '(^|/)src/commands/proc\.rs$|Cargo\.toml|rust-toolchain\.toml|\.githooks|pre-commit|flake|README|\.github/workflows' || true
printf '\nRelevant proc.rs section:\n'
if [ -f src/commands/proc.rs ]; then
nl -ba src/commands/proc.rs | sed -n '1,120p'
fi
printf '\nOccurrences of parse_proc_state / expect in rust files:\n'
rg -n "parse_proc_state|--state|expect\\(|unwrap\\(" -g '*.rs' . || true
printf '\nCargo files:\n'
wc -l Cargo.toml rust-toolchain.toml 2>/dev/null || true
sed -n '1,220p' Cargo.toml 2>/dev/null || true
sed -n '1,80p' rust-toolchain.toml 2>/dev/null || trueRepository: KooshaPari/sharecli
Length of output: 3367
🏁 Script executed:
RUSTFLAGS=-D warnings cargo build --locked --all-features
RUSTFLAGS=-D warnings cargo test --locked --all-features
RUSTFLAGS=-D warnings cargo clippy --locked --all-targets --all-features -- -D warningsRepository: KooshaPari/sharecli
Length of output: 287
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'src/commands/proc.rs:\n'
awk '{ printf "%6d\t%s\n", NR, $0 }' src/commands/proc.rs | sed -n '1,130p'
printf '\nOccurrences in Rust files:\n'
grep -RIn --include='*.rs' 'parse_proc_state|--state|\.expect\(|\.unwrap\(' . || true
printf '\nCargo and toolchain snippets:\n'
awk '{ printf "%6d\t%s\n", NR, $0 }' Cargo.toml | sed -n '1,220p'
awk '{ printf "%6d\t%s\n", NR, $0 }' rust-toolchain.toml | sed -n '1,80p'
printf '\nCI Rust build section snippets:\n'
for f in .github/workflows/*.yml .github/workflows/*.yaml; do
[ -f "$f" ] || continue
if grep -qE 'cargo (build|test|clippy)|RUSTFLAGS=' "$f"; then
echo "--- $f"
awk '/cargo (build|test|clippy)|RUSTFLAGS=/ { start=NR-5 } NR>=start && NR<=start+40 { printf "%6d\t%s\n", NR, $0 }' "$f" | sed -n '1,120p'
fi
doneRepository: KooshaPari/sharecli
Length of output: 50375
Remove the remaining expect from parse_proc_state.
trimmed.chars().next() is already guarded by the empty/length checks, so this can keep returning a Result with CLI-style failures. A missing chars().next() on this path would still panic with the current code.
Suggested replacement
- let ch = trimmed.chars().next().expect("single-char --state value (length validated above)");
+ let Some(ch) = trimmed.chars().next() else {
+ bail!("--state must not be empty");
+ };After the change, ensure Rust changes pass cargo build, cargo test, and cargo clippy with the required settings.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let ch = trimmed.chars().next().expect("single-char --state value (length validated above)"); | |
| let Some(ch) = trimmed.chars().next() else { | |
| bail!("--state must not be empty"); | |
| }; |
🤖 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 `@src/commands/proc.rs` at line 66, Remove the expect call from
parse_proc_state when extracting the first character from trimmed; preserve the
existing Result-based CLI error handling by propagating or converting a missing
character into an appropriate parse error. Then verify the Rust changes with
cargo build, cargo test, and cargo clippy using the required settings.
Source: Coding guidelines
| } | ||
| pub fn write(&self, level: LogLevel, msg: impl Into<String>) { | ||
| let mut buf = self.buffer.lock().unwrap(); | ||
| let mut buf = self.buffer.lock().expect("LogSink buffer mutex poisoned"); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Rust toolchain/config files:"
git ls-files | rg '(^|/)(rust-toolchain(-.*)?\.toml|Cargo\.toml|Cargo\.lock)$' || true
if [ -f rust-toolchain.toml ]; then cat -n rust-toolchain.toml; fi
echo
echo "Repository Rust expect/unwrap occurrences (excluding tests maybe):"
rg -n '\bexpect\(|\bunwrap\(' -g '*.rs' . | head -n 220
echo
echo "Relevant file snippets:"
for f in src/log_sink.rs src/main.rs src/mapi_props.rs src/metrics.rs src/monitoring.rs src/object_pool.rs src/radix_trie.rs src/ring_buffer.rs src/skiplist.rs src/stream.rs; do
if [ -f "$f" ]; then
echo "--- $f ---"
wc -l "$f"
rg -n '\bexpect\(|\bunwrap\(' "$f" || true
fi
doneRepository: KooshaPari/sharecli
Length of output: 22488
Replace production expect calls with structured error handling.
The listed Rust paths panic on error instead of returning or logging errors, including poisoned mutexes, clock failures, and invalid conversion/parse data.
src/log_sink.rs#L28-L28/L44-L47: handle poisoned mutexes without panicking.src/main.rs#L1669: propagateSystemTimeErrorthroughResult<()>.src/mapi_props.rs#L173-L190: return conversion failures as parse errors.src/metrics.rs#L57-L61: handle poisoned registry mutexes without panicking.src/monitoring.rs#L231: propagate or safely handle the clock error.src/object_pool.rs#L13-L25: handle poisoned mutexes without panicking.src/radix_trie.rs#L72: maintain the invariant without panicking.src/ring_buffer.rs#L41: handle an unoccupied slot.src/skiplist.rs#L62-L98: handle missing node keys.src/stream.rs#L11: maintain the chunk invariant without panicking.
📍 Affects 10 files
src/log_sink.rs#L28-L28(this comment)src/log_sink.rs#L44-L47src/main.rs#L1667-L1670src/mapi_props.rs#L169-L191src/metrics.rs#L57-L61src/monitoring.rs#L230-L231src/object_pool.rs#L13-L13src/object_pool.rs#L22-L25src/radix_trie.rs#L69-L74src/ring_buffer.rs#L41-L41src/skiplist.rs#L62-L62src/skiplist.rs#L88-L88src/skiplist.rs#L98-L98src/stream.rs#L11-L11
🤖 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 `@src/log_sink.rs` at line 28, Replace production expect-based panics with
structured handling across all listed sites: in src/log_sink.rs lines 28 and
44-47, recover from poisoned buffer mutexes; in src/main.rs lines 1667-1670,
propagate SystemTimeError through Result<()>; in src/mapi_props.rs lines
169-191, convert failures into parse errors; in src/metrics.rs lines 57-61 and
src/object_pool.rs lines 13 and 22-25, handle poisoned mutexes without
panicking; in src/monitoring.rs lines 230-231, propagate or safely handle clock
errors; in src/radix_trie.rs lines 69-74 and src/stream.rs line 11, preserve
invariants without panic; in src/ring_buffer.rs line 41, handle unoccupied
slots; and in src/skiplist.rs lines 62, 88, and 98, handle missing node keys
safely.
Source: Coding guidelines
| } | ||
| pub fn write(&mut self, item: T) { | ||
| self.chunks.last_mut().unwrap().push(item); | ||
| self.chunks.last_mut().expect("Stream always has at least one chunk").push(item); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== toolchain =="
cat rust-toolchain.toml 2>/dev/null || true
echo "== src/stream.rs outline =="
ast-grep outline src/stream.rs --view compact || true
echo "== src/stream.rs relevant lines =="
wc -l src/stream.rs
sed -n '1,80p' src/stream.rs
echo "== expect occurrences in src =="
rg -n 'expect\(' src || trueRepository: KooshaPari/sharecli
Length of output: 31320
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== git diff stat =="
git diff --stat || true
echo "== changed src/stream.rs lines =="
git diff --unified=20 -- src/stream.rs | sed -n '1,160p' || trueRepository: KooshaPari/sharecli
Length of output: 211
Remove expect from Stream::write.
chunk_size(0) can leave self.chunks empty, so this panics even without future mutation. Use a checked or error-returning path instead of relying on the invariant.
🤖 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 `@src/stream.rs` at line 11, Update Stream::write to remove the expect on
self.chunks.last_mut(). Handle an empty chunks collection safely, using the
existing checked or error-returning behavior so chunk_size(0) does not panic
while preserving normal item writes.
Source: Coding guidelines
| /// Build an `Rgb` from a `#rrggbb` or `rrggbb` hex literal at compile time. | ||
| /// Panics on malformed input — only call with string literals. | ||
| // The panic is a documented contract for malformed color literals (the | ||
| // caller passes string literals, so the branch is unreachable in practice); | ||
| // deliberately kept so misconfiguration fails loudly at startup. | ||
| #[allow(clippy::panic)] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject trailing characters in Rgb::from_hex.
The documented forms contain exactly six hexadecimal digits, with an optional leading #. The implementation accepts overlong inputs and silently ignores the suffix. For example, Rgb::from_hex("1234567") parses 123456 instead of rejecting malformed input.
Proposed validation
pub const fn from_hex(hex: &str) -> Self {
let bytes = hex.as_bytes();
+ if !(bytes.len() == 6 || (bytes.len() == 7 && bytes[0] == b'#')) {
+ panic!("invalid RGB hex length");
+ }
// accept '`#rrggbb`' (7) or 'rrggbb' (6)
- let start = if bytes[0] == b'#' { 1 } else { 0 };
+ let start = if bytes.len() == 7 { 1 } else { 0 };🤖 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 `@src/theme.rs` around lines 19 - 24, Update Rgb::from_hex to validate that the
input is exactly six hexadecimal digits after an optional leading #, rejecting
overlong values and any trailing characters before parsing. Preserve the
existing compile-time malformed-input panic behavior for all invalid lengths or
characters.
|
Closing due to merge conflicts. |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous Review Summary (commit 752309f)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 752309f)Status: No Issues Found | Recommendation: Merge Files Reviewed (54 files)
Reviewed by step-3.7-flash · Input: 115.7K · Output: 21K · Cached: 2M |
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@crates/sharecli-session/tests/session_ledger.rs`:
- Around line 41-48: Scope the reopened SessionStore and its query results in a
nested block so all SQLite handles are dropped before
std::fs::remove_file(&path). Keep the existing observations and confidence
assertions inside that scope, and leave cleanup after the block.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 03f8370e-9ef8-450a-9b01-cc6c4e584b4a
📒 Files selected for processing (3)
.github/workflows/fuse-mount-smoke.ymlContainerfile.fuse-smokecrates/sharecli-session/tests/session_ledger.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (30)
- GitHub Check: Python
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: cargo deny
- GitHub Check: cargo bench (gate)
- GitHub Check: hyperfine healthz (soft)
- GitHub Check: dashboard keyboard Tab-cycle
- GitHub Check: cargo bench (soft)
- GitHub Check: Reproducible build (L52)
- GitHub Check: build unsigned .deb (linux)
- GitHub Check: Dependency Audit
- GitHub Check: cargo audit (RustSec)
- GitHub Check: SAST Analysis
- GitHub Check: healthz load burst (soft)
- GitHub Check: idle RSS soft budget
- GitHub Check: test
- GitHub Check: lint
- GitHub Check: linux_native (ubuntu-24.04)
- GitHub Check: windows_winfsp (windows-latest)
- GitHub Check: coverage
- GitHub Check: healthz soak (soft)
- GitHub Check: Offline build after fetch (soft)
- GitHub Check: dhat heap soft budget
- GitHub Check: codeql
- GitHub Check: Unit Tests
- GitHub Check: dashboard PNG hard diff
- GitHub Check: live pool probe (soft)
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
- GitHub Check: windows_winfsp (windows-latest)
- GitHub Check: linux_native (ubuntu-24.04)
⚠️ CI failures not shown inline (4)
GitHub Actions: PR Lint / 0_FR reference in PR body.txt: fix(lanes): green security-scan, a11y, SAST, scorecard, and trunk-check lanes
Conclusion: failure
##[group]Run actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3
with:
script: const body = context.payload.pull_request.body || "";
const labels = (context.payload.pull_request.labels || []).map((l) => l.name);
const skip =
labels.includes("skip-fr-lint") ||
labels.includes("dependencies") ||
labels.includes("chore");
if (skip) {
core.info("Skipping FR lint due to label: " + labels.join(", "));
return;
}
// Match FR-001, FR-CAST-003, FR-PROC-001 (legacy), etc.
const frPattern = /\bFR-[A-Z0-9]+(?:-[A-Z0-9]+)*\b/;
if (!frPattern.test(body)) {
core.setFailed(
"PR body must reference at least one FR ID (e.g. FR-001). " +
"See FUNCTIONAL_REQUIREMENTS.md / docs/specs/FR.md. " +
"Docs-only chores may use label skip-fr-lint."
);
return;
}
core.info("FR reference found in PR body.");
github-***REDACTED_SECRET_ASSIGNMENT***
debug: false
user-agent: actions/github-script
result-encoding: json
retries: 0
retry-exempt-status-codes: 400,401,403,404,422
##[endgroup]
##[error]PR body must reference at least one FR ID (e.g. FR-001). See FUNCTIONAL_REQUIREMENTS.md / docs/specs/FR.md. Docs-only chores may use label skip-fr-lint.
GitHub Actions: PR Lint / FR reference in PR body: fix(lanes): green security-scan, a11y, SAST, scorecard, and trunk-check lanes
Conclusion: failure
##[group]Run actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3
with:
script: const body = context.payload.pull_request.body || "";
const labels = (context.payload.pull_request.labels || []).map((l) => l.name);
const skip =
labels.includes("skip-fr-lint") ||
labels.includes("dependencies") ||
labels.includes("chore");
if (skip) {
core.info("Skipping FR lint due to label: " + labels.join(", "));
return;
}
// Match FR-001, FR-CAST-003, FR-PROC-001 (legacy), etc.
const frPattern = /\bFR-[A-Z0-9]+(?:-[A-Z0-9]+)*\b/;
if (!frPattern.test(body)) {
core.setFailed(
"PR body must reference at least one FR ID (e.g. FR-001). " +
"See FUNCTIONAL_REQUIREMENTS.md / docs/specs/FR.md. " +
"Docs-only chores may use label skip-fr-lint."
);
return;
}
core.info("FR reference found in PR body.");
github-***REDACTED_SECRET_ASSIGNMENT***
debug: false
user-agent: actions/github-script
result-encoding: json
retries: 0
retry-exempt-status-codes: 400,401,403,404,422
##[endgroup]
##[error]PR body must reference at least one FR ID (e.g. FR-001). See FUNCTIONAL_REQUIREMENTS.md / docs/specs/FR.md. Docs-only chores may use label skip-fr-lint.
GitHub Actions: Trunk Check / Lint & Format: fix(lanes): green security-scan, a11y, SAST, scorecard, and trunk-check lanes
Conclusion: failure
##[group]Run cat >>$GITHUB_ENV <<EOF
�[36;1mcat >>$GITHUB_ENV <<EOF�[0m
�[36;1mGITHUB_***REDACTED_SECRET_ASSIGNMENT***
�[36;1mTRUNK_LAUNCHER_QUIET=false�[0m
�[36;1mEOF�[0m
�[36;1m�[0m
�[36;1m# First arg is field to fetch, second arg is default value or empty�[0m
�[36;1mpayload() {�[0m
�[36;1m if [ $# -lt 2 ]; then�[0m
�[36;1m DEFAULT_VALUE=empty�[0m
�[36;1m else�[0m
�[36;1m DEFAULT_VALUE=\"$2\"�[0m
�[36;1m fi�[0m
�[36;1m if command -v jq >/dev/null; then�[0m
�[36;1m jq -r ".inputs.payload | fromjson | .$1 // ${DEFAULT_VALUE}" ${TEST_GITHUB_EVENT_PATH:-${GITHUB_EVENT_PATH}}�[0m
�[36;1m else�[0m
�[36;1m echo "::error::jq not installed on system!"�[0m
GitHub Actions: Trunk Check / 0_Lint & Format.txt: fix(lanes): green security-scan, a11y, SAST, scorecard, and trunk-check lanes
Conclusion: failure
##[group]Run cat >>$GITHUB_ENV <<EOF
�[36;1mcat >>$GITHUB_ENV <<EOF�[0m
�[36;1mGITHUB_***REDACTED_SECRET_ASSIGNMENT***
�[36;1mTRUNK_LAUNCHER_QUIET=false�[0m
�[36;1mEOF�[0m
�[36;1m�[0m
�[36;1m# First arg is field to fetch, second arg is default value or empty�[0m
�[36;1mpayload() {�[0m
�[36;1m if [ $# -lt 2 ]; then�[0m
�[36;1m DEFAULT_VALUE=empty�[0m
�[36;1m else�[0m
�[36;1m DEFAULT_VALUE=\"$2\"�[0m
�[36;1m fi�[0m
�[36;1m if command -v jq >/dev/null; then�[0m
�[36;1m jq -r ".inputs.payload | fromjson | .$1 // ${DEFAULT_VALUE}" ${TEST_GITHUB_EVENT_PATH:-${GITHUB_EVENT_PATH}}�[0m
�[36;1m else�[0m
�[36;1m echo "::error::jq not installed on system!"�[0m
🧰 Additional context used
📓 Path-based instructions (3)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Use UTF-8 encoding for all text files; do not use Windows-1252 smart quotes or other special characters.
Use UTF-8 for all text files.
Files:
Containerfile.fuse-smokecrates/sharecli-session/tests/session_ledger.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use Rust for the project and validate Rust changes with Cargo build, Cargo test, and Cargo clippy.
**/*.rs: For new Rust modules, create the test file before the implementation; for bug fixes, write a failing test before the fix; for refactors, ensure existing tests pass before and after.
Use idiomatic, language-appropriate error handling, never useunwraporexpectin production Rust code, and log all errors with structured logging.
Files:
crates/sharecli-session/tests/session_ledger.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,toml}: Use Rust edition 2021 and the pinned toolchain fromrust-toolchain.toml; keep code compatible with the configured stable compiler, rustfmt, and clippy.
Ensure Rust code passes formatting, clippy with-D warnings, and the locked all-features test suite; CI usesRUSTFLAGS=-D warnings.
UsePascalCasefor Rust types,snake_casefor functions, methods, and modules, andSCREAMING_SNAKE_CASEfor constants.
Files:
crates/sharecli-session/tests/session_ledger.rs
🔇 Additional comments (3)
.github/workflows/fuse-mount-smoke.yml (1)
35-37: LGTM!Also applies to: 38-43
Containerfile.fuse-smoke (1)
18-20: LGTM!crates/sharecli-session/tests/session_ledger.rs (1)
26-67: 📐 Maintainability & Code QualityRun the required Rust validation.
The supplied context does not include validation output for this added integration test. Run Cargo build, the locked all-features test suite, rustfmt, and Clippy with warnings denied, using the pinned toolchain.
Source: Coding guidelines
| let reopened = SessionStore::open(&path).unwrap(); | ||
| let rows = reopened.observations("codex:abc").unwrap(); | ||
| let rows = reopened.observations(None).unwrap(); | ||
| assert_eq!(rows.len(), 1); | ||
| assert!(rows[0].resumable); | ||
| assert_eq!(rows[0].confidence, ResolutionConfidence::Exact); | ||
| let session = rows[0].session.as_ref().expect("observation carries session"); | ||
| assert!(session.auto_resumable(), "Exact-confidence session must be auto-resumable"); | ||
| assert_eq!(session.confidence, ResolutionConfidence::Exact); | ||
|
|
||
| std::fs::remove_file(&path).unwrap(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Close the SQLite connection before file cleanup.
At Line 48, reopened is still alive. Windows cannot delete a database file with an active SQLite handle. Scope the reopened store and its query results before remove_file.
Proposed fix
- let reopened = SessionStore::open(&path).unwrap();
- let rows = reopened.observations(None).unwrap();
- assert_eq!(rows.len(), 1);
- let session = rows[0].session.as_ref().expect("observation carries session");
- assert!(session.auto_resumable(), "Exact-confidence session must be auto-resumable");
- assert_eq!(session.confidence, ResolutionConfidence::Exact);
+ {
+ let reopened = SessionStore::open(&path).unwrap();
+ let rows = reopened.observations(None).unwrap();
+ assert_eq!(rows.len(), 1);
+ let session = rows[0].session.as_ref().expect("observation carries session");
+ assert!(session.auto_resumable(), "Exact-confidence session must be auto-resumable");
+ assert_eq!(session.confidence, ResolutionConfidence::Exact);
+ }
std::fs::remove_file(&path).unwrap();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let reopened = SessionStore::open(&path).unwrap(); | |
| let rows = reopened.observations("codex:abc").unwrap(); | |
| let rows = reopened.observations(None).unwrap(); | |
| assert_eq!(rows.len(), 1); | |
| assert!(rows[0].resumable); | |
| assert_eq!(rows[0].confidence, ResolutionConfidence::Exact); | |
| let session = rows[0].session.as_ref().expect("observation carries session"); | |
| assert!(session.auto_resumable(), "Exact-confidence session must be auto-resumable"); | |
| assert_eq!(session.confidence, ResolutionConfidence::Exact); | |
| std::fs::remove_file(&path).unwrap(); | |
| { | |
| let reopened = SessionStore::open(&path).unwrap(); | |
| let rows = reopened.observations(None).unwrap(); | |
| assert_eq!(rows.len(), 1); | |
| let session = rows[0].session.as_ref().expect("observation carries session"); | |
| assert!(session.auto_resumable(), "Exact-confidence session must be auto-resumable"); | |
| assert_eq!(session.confidence, ResolutionConfidence::Exact); | |
| } | |
| std::fs::remove_file(&path).unwrap(); |
🤖 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 `@crates/sharecli-session/tests/session_ledger.rs` around lines 41 - 48, Scope
the reopened SessionStore and its query results in a nested block so all SQLite
handles are dropped before std::fs::remove_file(&path). Keep the existing
observations and confidence assertions inside that scope, and leave cleanup
after the block.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Quality Gate Report❌ Unit Tests: FAILED |
User description
Summary
Greens five pre-existing red lanes on every main push (Security Scan, Accessibility, SAST, OpenSSF Scorecard, Trunk Check). All diagnosed from job logs and verified locally before landing.
security.yml —
cargo clippy -- -D warnings -W clippy::panic -W clippy::unwrap_used(54 findings, 3 crates)PathBufimport (cfg-gated), unusedbackendparam, unneededreturn, missing docs onFuseBackend+ variants, dead-code on macOS-onlyruntime_diagnostics/parse_bundle_version.unwrap()→expect().unwrap()→expect()across caster/log_sink/metrics/monitoring/util modules;panic!→assert!in hkdf; manual checked-div →checked_div; redundant guard, needless borrows, collapsible ifs,unwrap_or_default; unusedProcStateimports moved to#[cfg(test)].too_many_argumentsgets targeted allows on the three CLI dispatch functions (deliberate flag-aggregation shape).cargo test --release --locked --no-fail-fast→ 185/186 (sole failure is the WSL npm-PATH artifact that passes on the runner, unchanged from baseline).a11y.yml — axe job crashed with
webidl.util.markAsUncloneable is not a functionworker_threads.markAsUncloneable(Node ≥ 22.11); the workflow pinned node 20. Bumped both jobs to node 22 and aligned Playwright browser install with package.json (1.62.1; the workflow installed 1.49.0 browsers, mismatching the script's 1.62.1 → missing chromium_headless_shell).npm run a11y:dashboard→ 0 violations.sast.yml — CodeQL "fatal error … finalize go"
languages: typescript, python, rust, gobut the repo has zero Go sources. Removedgo; addedcontents: read+security-events: write.scorecard.yml — "error obtaining token: expired_token" while signing results
permissions: read-allblocked the OIDC token forpublish_resultsFulcio signing. Changed tocontents: read+id-token: write.trunk-check.yml — config used a nonexistent schema (8 config-errors per run)
.trunk/trunk.yamlto the real Trunk Check schema; enabled actionlint + taplo + yamllint (clippy/fmt have dedicated hard lanes; the nestedlib/teamcommworkspace isn't covered by rootcargo fmt).needs.dependency-review→needs.dep-reviewin ci.yml; infisical.yml used the retiredblacksmith-2vcpu-ubuntu-2204runner label →ubuntu-22.04(also unblocks re-enabling Infisical Sync, still disabled in repo settings); codecov.yml mis-indentedpathsentry; trailing whitespace in security.yml/codecov.yml.mise.tomlgenuinely invalid TOML ([tasks.docs:build]→["tasks.docs:build"]); taplo-formatted all TOMLs (whitespace-only)..trunk/configs/.yamllint.yaml+.github/actionlint.yaml(documents the forwardsteps.detectreference in ci.yml).trunk check --all→ "No issues".CodeAnt-AI Description
Restore failing quality gates and harden CI checks
What Changed
Impact
✅ Green security and dependency checks✅ Reliable accessibility and SAST scans✅ Passing documentation and configuration validation💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.