Skip to content

fix: preserve sticky bits in macOS trust storage checks - #263

Merged
shiqui merged 1 commit into
mainfrom
codex/fix-macos-sticky-trust
Nov 10, 2026
Merged

shiqui merged 1 commit into
mainfrom
codex/fix-macos-sticky-trust

Conversation

@shiqui

@shiqui shiqui commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Description

On macOS, git gtr trust rejected private trust storage beneath sticky directories such as /tmp. BSD stat %Lp omits the sticky bit, so the validator treated a protected 01777 ancestor as unprotected 0777.

Read the full mode with %p in the CLI and generated Bash/Zsh/Fish helpers. Bump the shell-integration cache schema from 15 to 16 so existing installations regenerate affected helpers, and extend cache-upgrade coverage.

Motivation

Restore the intended sticky-parent exception while retaining rejection of unprotected writable ancestors, writable trust directories, unsafe ownership, and write-capable ACLs.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Testing

Tested on macOS 26.6.2 with Bash 3.2.57, Git 2.50.1, Zsh 5.9, and Fish 4.9.3.

  • Four new regressions failed before the fix and passed afterward: CLI approval and generated Bash/Zsh/Fish consumers beneath sticky ancestors. Each also checks that removing the sticky protection invalidates approval.
  • Full BATS suite: 1,046 passed, zero failures or skips, across 46 files run in parallel with isolated Git/XDG configuration per file.
  • Existing trust-storage tests passed, including ACLs, ownership, symlinks, and revocation.
  • ShellCheck, generated-completion consistency, and git diff --check passed.
  • Disposable repository beneath /tmp: trust approval, new --porcelain, postCreate hooks, AI postCd hooks using --ai true, Bash gtr cd, schema-16 cache generation, and removal all passed.
  • Cache migration and documented startup recipes passed for Bash, Zsh, and Fish.

Linux and Windows runtime validation was not performed for this change.

Checklist

  • Read CONTRIBUTING.md and followed the project's style.
  • Added regression coverage and tested both CLI entry points.
  • Performed manual testing on macOS and noted platform limits.
  • No new dependencies, commands, or flags.
  • Existing documentation describes the restored behavior; no documentation change needed.

License Acknowledgment

This contribution is submitted under the repository's Apache License 2.0.

Summary by CodeRabbit

  • Bug Fixes
    • Trust-storage approvals now correctly recognize sticky-directory protections. Approvals remain usable while the protection is active and are rejected if it is removed, consistently across the CLI and generated shell integrations.
    • Setup now refreshes outdated cache data to the latest format, including when the CLI version also needs updating. Older cache formats are recognized as stale.

@coderabbitai

coderabbitai Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

BSD trust-storage checks now read special permission bits, so sticky ancestors are distinguished from unprotected writable ancestors. The shell-integration cache schema advances to 16. Tests cover CLI and generated-shell behavior, plus cache regeneration.

Changes

Sticky trust-storage validation

Layer / File(s) Summary
Read and validate sticky ancestor modes
lib/commands/init.sh, lib/hooks.sh, tests/trust_storage.bats
The BSD stat paths use %p to include special permission bits. Tests cover CLI and generated Bash, Zsh, and Fish behavior under sticky ancestors, and verify rejection after sticky protection is removed.
Refresh cached shell integrations
lib/commands/init.sh, tests/init.bats
The default cache schema changes from 15 to 16. Tests verify regeneration for stale schemas and an outdated CLI version.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: natoboram

Merge Risk: Minimal · up to 52362

The trust-storage change has no identified merge-blocking issue; the macOS tests cover the sticky-parent behavior it restores.

Security architecture risk: Low · up to 52362

The change restores approval storage beneath protected sticky directories while retaining ownership, permission, and ACL checks. No new approval bypass was identified in the inspected paths. Cross-platform execution and some regeneration failure states remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The affected authority is command execution in the invoking user's environment, including programs and scripts delegated to by approved commands. It is not confined to repository files. The newly accepted storage layout does not itself grant another OS user permission to create or replace valid approval markers.

Security Findings and Attack Paths

  • inferred — No PR-introduced approval bypass was established in the inspected flow from repository-controlled definitions to execution. The canonical security input contains no retained findings, but its unknown coverage does not establish that all attack paths were examined.

Trust Boundaries and Controls

  • observed — Approval still captures reviewed definitions and repository identity before confirmation, writes a marker through a private temporary file, and rechecks the current approval key afterward. Storage is validated again when approvals are consumed. New regressions assert that removing sticky protection disables existing approval reads, writes, and hook execution.

Resilience and Maintainability Implications

  • inferred — Atomic marker and cache replacement contain ordinary write failures without exposing partially written destination files. Forced interruption can leave private temporary files, while generator failure detection remains an inherited uncertainty: generated output is captured through a pipeline without an explicit generator-success check before caching. The PR does not change those mechanisms.
Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly summarizes the main change: preserving sticky bits in macOS trust-storage checks.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 4 files.

Finishing Touches

Generate docstrings

  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks the modes at night,
A sticky bit keeps paths all right.
Bash, Zsh, and Fish join the show,
With fresh cache headers set to grow.
The rabbit hops where safe bits glow.

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 2001d95 and 523621a.

Files selected for processing: 4

  • lib/commands/init.sh
  • lib/hooks.sh
  • tests/init.bats
  • tests/trust_storage.bats

Included review availability: This review used your included allowance. Your plan provides up to 100 included reviews per hour; 99 remain after this review.

@shiqui
shiqui merged commit fc8afc0 into main Nov 10, 2026
5 checks passed
@shiqui
shiqui deleted the codex/fix-macos-sticky-trust branch November 10, 2026 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant