security: bind Brew cleanup audit records to private storage - #187
security: bind Brew cleanup audit records to private storage#187seonghobae wants to merge 17 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
|
@coderabbitai review Please review the unchanged exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Git identity and delta =='
git rev-parse HEAD
git rev-parse 8e676ea5e89dd473a7b0c6ca763ddb7f71599ea5
git merge-base be418bac9bfbdc8aa41b1a698713833e3dbccfdc 8e676ea5e89dd473a7b0c6ca763ddb7f71599ea5
git diff --name-status be418bac9bfbdc8aa41b1a698713833e3dbccfdc 8e676ea5e89dd473a7b0c6ca763ddb7f71599ea5
echo '== Rust structure =='
ast-grep outline src-tauri/src/brew_cleanup.rs --items all --type function,impl,struct,mod --view expanded
ast-grep outline src-tauri/src/brew_cleanup_audit_authority_tests.rs --items all --type function,mod --view expanded
ast-grep outline src-tauri/src/lib.rs --items all --type mod --view expanded
echo '== Focused audit implementation references =='
rg -n -C 5 --type rust \
'write_audit_record|brew-cleanup-audit-(parent|directory)-writable-by-others|brew-cleanup-records|DirBuilderExt|OpenOptionsExt|set_permissions|sync_all|remove_file' \
src-tauri/src/brew_cleanup.rs src-tauri/src/brew_cleanup_audit_authority_tests.rs src-tauri/src/lib.rs
echo '== PR and check state =='
gh pr view 187 --repo ContextualWisdomLab/disksage \
--json number,headRefOid,baseRefOid,mergeStateStatus,statusCheckRollup,files \
--jq '{number, headRefOid, baseRefOid, mergeStateStatus, checks: [.statusCheckRollup[] | {name, status, conclusion, detailsUrl}], files: [.files[].path]}'Length of output: 1981 🏁 Script executed: #!/bin/bash
set -u
echo '== Supported structural maps =='
ast-grep outline src-tauri/src/brew_cleanup.rs --items all --type function,struct,mod --view expanded || true
ast-grep outline src-tauri/src/brew_cleanup_audit_authority_tests.rs --items all --type function,struct,mod --view expanded || true
ast-grep outline src-tauri/src/lib.rs --items all --type function,struct,mod --view expanded || true
echo '== Exact source diff =='
git diff --no-ext-diff --unified=80 \
be418bac9bfbdc8aa41b1a698713833e3dbccfdc \
8e676ea5e89dd473a7b0c6ca763ddb7f71599ea5 \
-- src-tauri/src/brew_cleanup.rs src-tauri/src/brew_cleanup_audit_authority_tests.rs src-tauri/src/lib.rs
echo '== Focused implementation and regression source =='
rg -n -C 12 --type rust \
'fn write_audit_record|fn .*writable|brew-cleanup-audit-(parent|directory)-writable-by-others|brew-cleanup-records|DirBuilderExt|OpenOptionsExt|set_permissions|sync_all|remove_file|audit_authority_tests' \
src-tauri/src/brew_cleanup.rs src-tauri/src/brew_cleanup_audit_authority_tests.rs src-tauri/src/lib.rsLength of output: 37684 🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr view 187 --repo ContextualWisdomLab/disksage \
--json number,headRefOid,baseRefOid,mergeStateStatus,statusCheckRollup,files \
--jq '{number, headRefOid, baseRefOid, mergeStateStatus, checks: [.statusCheckRollup[] | {name, status, conclusion, detailsUrl}], files: [.files[].path]}'Length of output: 5512 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Existing audit-record tests =='
sed -n '620,760p' src-tauri/src/brew_cleanup.rs
echo '== Test configuration candidates =='
fd -a -t f 'Cargo.toml|Cargo.lock' . \
-E target -E node_modules \
| while IFS= read -r file; do
echo "--- $file"
rg -n -C 3 'tempfile|^\[dev-dependencies\]|^\[workspace' "$file" || true
done
echo '== Direct audit writer references and test module registration =='
rg -n -C 5 --type rust \
'\bwrite_audit_record\s*\(|mod\s+brew_cleanup_audit_authority_tests\b|audit_records_are_create_new_and_private' \
src-tauri/srcLength of output: 9131
The implementation rejects group and other write authority on the app-data directory and on an existing audit directory. It creates the audit directory with mode The existing audit test verifies create-once behavior and final Unix mode The Linux, macOS, and Windows Release builds succeeded. Test, Semgrep, and Security Scan checks remain queued. The PR remains
|
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headb633e3efc90fcb053db3b6ca4f3695304206cf2c. -
Head SHA:
b633e3efc90fcb053db3b6ca4f3695304206cf2c -
Workflow run: 31796494843
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (4 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (4 files)"]
R1 --> V1["required checks"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage Decision
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (4 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (4 files)"]
R1 --> V1["required checks"]
|
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headb633e3efc90fcb053db3b6ca4f3695304206cf2c. -
Head SHA:
b633e3efc90fcb053db3b6ca4f3695304206cf2c -
Workflow run: 31799045915
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (4 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (4 files)"]
R1 --> V1["required checks"]
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Dismissing: this REQUEST_CHANGES was posted against commit b633e3e where coverage-evidence failed. The PR's current head has coverage-evidence passing (verified via gh pr checks). No unresolved review threads remain. Re-request review to re-validate.
opencode-agent
Bot
added
area: security
* test: prove private evidence parent replacement race * fix: bind private evidence writes to opened parent * test: prove private evidence parent permission drift * fix: revalidate private evidence parent permissions * test: preserve replaced private evidence on failed cleanup * security: preserve identity-mismatched evidence replacements * security: invalidate failed evidence through opened descriptor * test: prove private evidence mode survives restrictive umask * fix: normalize private evidence mode after open * test: isolate restrictive umask to record creation * test: cover post-create parent replacement * refactor: centralize private parent validation * fix(stack): preserve current audit root in private evidence owner * test: expose pre-open private evidence parent race * fix: bind private evidence parent before canonicalization * refactor: expose object-bound evidence publication primitive * test: expose shared object-bound publication seam * fix: use C-compatible private evidence mode argument
1 participant
Purpose
Bind immutable Homebrew cleanup audit publication to the exact private directory object that was authorized, closing shared-writable storage and same-user pathname-replacement authority gaps without changing Homebrew command, model, human-approval, or evidence semantics.
Exact current state
8d1733849884bc517171e44065e1219568120e66main:93aa7477629753187da101a8af0e7276480f1a86main -> headis ahead-only with merge base exactly protected main; semantic delta remains exactly four paths:src-tauri/Cargo.toml,src-tauri/src/brew_cleanup.rs,src-tauri/src/brew_cleanup_audit_authority_tests.rs, andsrc-tauri/src/lib.rs32318545189completedsuccess; Test32318545203, Security Scan32318545190, and SAST Semgrep32318545195are queued and therefore non-passingTest-first security repair
The implementation proves the object-bound publication contract by opening the audit directory with no-follow directory semantics, validating opened object and pathname device/inode identity plus private mode, creating immutable records descriptor-relative with
openat(O_CREAT | O_EXCL | O_NOFOLLOW), writing/fsyncing and hardening through the exact opened record, fsyncing the exact directory, cleaning failures descriptor-relative, and revalidating directory/record identities before returning. A same-user rename-and-replacement race fails closed withbrew-cleanup-audit-directory-identity-driftand does not redirect the record into the replacement directory.Unsupported non-Unix publication remains explicit/fail-closed; Homebrew execution authority remains macOS-only.
Stack / ownership
This PR remains the dependency-root proving owner for issue #194's descriptor/handle-relative publication primitive. Downstream #205 has now been advanced onto this exact head while retaining only its Intel Homebrew compatibility delta. Downstream #228 has likewise been advanced onto this exact head while retaining only its shared
private_evidenceobject-bound publication delta. #206 remains downstream of #205.#174 owns generic cleanup, #156 repository-wide exact coverage/release evidence, #150 Podman desktop evidence, and #149 canonical acquisition documentation.
Required before merge
Keep Draft until the unchanged exact head satisfies every applicable live required workflow, repository-wide exact 100% owned-production coverage without threshold or production-scope weakening, zero valid current-head findings, fresh live-base ancestry, and actual live governance. Pending, queued, skipped-required, failed, stale, predecessor, model-only, status-only, diagnostic-only, synthetic, or author-only evidence is non-passing.