Skip to content

daemon: creating a staging dir for installs#949

Merged
sharvilshah merged 5 commits into
mainfrom
sbs/snt-385
May 13, 2026
Merged

daemon: creating a staging dir for installs#949
sharvilshah merged 5 commits into
mainfrom
sbs/snt-385

Conversation

@sharvilshah

Copy link
Copy Markdown
Contributor

Test plan:

  • Unit Tests
  • Manual Testing in progress

Fixes SNT-385

@sharvilshah sharvilshah requested a review from a team as a code owner May 12, 2026 15:56
@github-actions github-actions Bot added comp/santad Issues or PRs related to the daemon comp/santactl Issues or PRs related to santactl lang/objc++ PRs modifying files in ObjC++ comp/common size/m Size: medium labels May 12, 2026
@sharvilshah sharvilshah added this to the 2026.4 milestone May 12, 2026
@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c7d81807-b1d5-46e1-9632-2751bd5192ae

📥 Commits

Reviewing files that changed from the base of the PR and between 9a642db and 997354e.

📒 Files selected for processing (5)
  • Source/common/SNTCommonEnums.h
  • Source/santactl/Commands/SNTCommandInstall.mm
  • Source/santad/EventProviders/SNTEndpointSecurityTamperResistance.mm
  • Source/santad/EventProviders/SNTEndpointSecurityTamperResistanceTest.mm
  • Source/santad/SNTDaemonControlController.mm
🚧 Files skipped from review as they are similar to previous changes (3)
  • Source/santad/EventProviders/SNTEndpointSecurityTamperResistanceTest.mm
  • Source/santactl/Commands/SNTCommandInstall.mm
  • Source/santad/SNTDaemonControlController.mm

📝 Walkthrough

Walkthrough

Staged installation: new path constants and CLI preflight checks; daemon enforces lstat/ownership/realpath allowlist, stages bundles, performs atomic replace with backup and rollback, and marks the staging directory as tamper-protected.

Changes

Staged Installation Flow

Layer / File(s) Summary
Path constants for backup, migration, and staging
Source/common/SNTCommonEnums.h
Adds kSantaAppBackupPath, kSantaMigrationAppPath and canonical form, plus kSantaStagingDir/kSantaStagingAppPath with canonical form.
Command-line tool headers and bundle preflight
Source/santactl/Commands/SNTCommandInstall.mm
Adds C/system headers and staged-bundle preflight checks in installSantaApp: lstat existence, reject symlinks/non-directories, enforce root:wheel ownership, realpath canonicalization to kSantaMigrationAppPath, and exit on failure.
Staging directory tamper resistance and tests
Source/santad/EventProviders/SNTEndpointSecurityTamperResistance.mm, Source/santad/EventProviders/SNTEndpointSecurityTamperResistanceTest.mm
Adds /private/var/db/santa/staging as a prefix-protected path and a unit test asserting staging is protected while migration is not.
Daemon headers and canonical verification
Source/santad/SNTDaemonControlController.mm
Adds POSIX/system headers and rewrites verifyPathIsSanta: to use lstat/ownership checks and realpath canonicalization against an allowlist, then runs codesign/team/signing-ID verification.
Daemon-side staged install and rollback
Source/santad/SNTDaemonControlController.mm
Implements staged install flow: prepare staging dir (root-owned, 0700), move migration bundle into staging, pre-verify, atomically replace /Applications/Santa.app with replaceItemAtURL using a backup name, set ownership/permissions, post-verify, rollback atomically on failure, and clean staging/backup before replying.

Possibly related PRs

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately captures the main change: the PR introduces staging directory creation for Santa app installs, which is the core functionality across all modified files.
Description check ✅ Passed The description is related to the changeset, mentioning test plan and a bug fix reference (SNT-385), though it lacks detailed technical context about the implementation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sbs/snt-385

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@Source/santactl/Commands/SNTCommandInstall.mm`:
- Around line 130-135: The current check calls
realpath(installFromPath.UTF8String, resolved) but always logs the contents of
resolved even when realpath returned NULL (undefined behavior); update the block
in SNTCommandInstall.mm to first test the return of realpath and handle the
error separately from the canonical-path mismatch: if realpath returns NULL call
TEE_LOGE with a clear message including strerror(errno) and the original
installFromPath.UTF8String, and exit; otherwise compare resolved to
kSantaMigrationAppCanonicalPath and if they differ log a different error message
that prints resolved and exit. Ensure you reference the same symbols (realpath,
resolved, kSantaMigrationAppCanonicalPath, TEE_LOGE, installFromPath) when
making the change.

In `@Source/santad/SNTDaemonControlController.mm`:
- Around line 933-934: The LOGE invocation in SNTDaemonControlController.mm is
missing a comma before the variadic argument rollbackErr causing a
syntax/compile error; update the LOGE call (the one logging "CRITICAL: rollback
failed: %@...") to include the missing comma between the format string and
rollbackErr so the format placeholder is bound to the argument and the error is
logged correctly.
🪄 Autofix (Beta)

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: CHILL

Plan: Pro

Run ID: 9aaeb437-7dfe-4fff-b00b-bd054665c0f8

📥 Commits

Reviewing files that changed from the base of the PR and between c967421 and 992735d.

📒 Files selected for processing (5)
  • Source/common/SNTCommonEnums.h
  • Source/santactl/Commands/SNTCommandInstall.mm
  • Source/santad/EventProviders/SNTEndpointSecurityTamperResistance.mm
  • Source/santad/EventProviders/SNTEndpointSecurityTamperResistanceTest.mm
  • Source/santad/SNTDaemonControlController.mm

Comment thread Source/santactl/Commands/SNTCommandInstall.mm
Comment thread Source/santad/SNTDaemonControlController.mm Outdated
@sharvilshah sharvilshah merged commit b0d90a8 into main May 13, 2026
8 checks passed
@sharvilshah sharvilshah deleted the sbs/snt-385 branch May 13, 2026 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/common comp/santactl Issues or PRs related to santactl comp/santad Issues or PRs related to the daemon lang/objc++ PRs modifying files in ObjC++ size/m Size: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants