Skip to content

Merge upstream quattro (factory-reset hash scrub + hook/state path guards) - #25

Closed
cursor[bot] wants to merge 4 commits into
quattrofrom
cursor/upstream-changes-pr-5225
Closed

cursor[bot] wants to merge 4 commits into
quattrofrom
cursor/upstream-changes-pr-5225

Conversation

@cursor

@cursor cursor Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Merges 2 new first-parent commits (4 cherry-picked non-merge commits) from omacom/omarchy quattro since last night's check (2fbac0c8 / PR #23). Upstream HEAD is now 9c5482c5 (upstream #8170, merged 2026-09-16).

This is everything new since the 2026-09-15 cron. Leftover #18 already landed this morning as PR #24. Fork-only Cursor work is untouched: official-tarball installer, no mise wrapper, usage collector, and the theme-set fix from #22.

Factory reset now actually erases old hashes (upstream omacom#10379, fixes omacom#10378)

  • passwd --lock left the previous root hash in @factory/etc/shadow and the dash-suffixed backups, so the next owner could recover it
  • Shared scrub_factory_accounts now runs on both the staged reset root and the retained @factory baseline: userdel uid≥1000 accounts, set root's password field to !, then delete the shadow-utils backups
  • Follow-up on the same PR also deletes subuid- / subgid- (userdel left the previous owner's subordinate ID range in those backups)
  • Cleanup failures abort before boot rebuild or activation; a failed baseline scrub puts @factory back to read-only
  • First-boot provisioning still sets the new root password

Hook and state names cannot be paths (upstream omacom#8170)

  • omarchy-hook, omarchy-hook-install, and omarchy-state set now refuse an empty name, a name with /, or a name that is exactly . or .. (exit 2)
  • That stops omarchy-hook ../../evil from running ~/.config/evil and omarchy-state set ../../escape from creating ~/.local/escape
  • omarchy-hook-install got the same guard so a slashed type cannot mkdir/cp under hooks/<type>.d before the runner would refuse it
  • Names with dots in the middle (a..b) stay allowed. omarchy-state clear is unchanged: find -name matches basenames only
  • Robustness for future callers, not a shipped-caller fix — every in-repo call still uses a fixed label

Tests

Focused suites passed: hook-state-name-guard-test.sh (19 cases), factory-reset-accounts-test.sh (both roots, repeat reset, six injected cleanup failures; ran via user namespace), and ./test/cli (metadata unchanged).

Open in Web View Automation 

Note

High Risk
Changes factory-reset account and shadow handling on sale/handoff paths and hardens CLI utilities that write or execute under user config/state directories.

Overview
Merges upstream factory-reset credential scrubbing and hook/state path guards, with focused shell tests.

Factory reset no longer relies on passwd --lock, which left recoverable hashes in @factory and dash-suffixed shadow backups. A shared scrub_factory_accounts removes uid≥1000 users and homes, sets root’s password field to ! via usermod, and deletes passwd-/shadow-/subuid-/subgid- backups on both the staged reset root and the retained @factory baseline. Cleanup failures abort the reset (baseline is re-marked read-only on scrub failure). Tests cover normal/locked root, repeat reset, and injected userdel/usermod/rm failures.

omarchy-hook, omarchy-hook-install, and omarchy-state set now reject empty names, names containing /, or names exactly . or .. (exit 2), blocking path-style escape while still allowing dotted labels like a..b. omarchy-state clear is unchanged. New tests exercise valid names, traversal attempts, and install-side mkdir/cp guards.

Reviewed by Cursor Bugbot for commit 85cca3d. Configure here.

Adolanium and others added 4 commits September 16, 2026 23:04
omarchy-hook and omarchy-state set join a name straight into a path. A name
with a slash, or a bare . or .., points outside the hooks or state directory.
Every caller in the repo passes a fixed label, so this is a footgun guard for
future callers, not a fix for anything that ships today.

Names with dots in the middle (a..b) stay allowed. omarchy-state clear is
untouched: find -name matches basenames only.

(cherry picked from commit 0a65b45)
userdel rewrites /etc/subuid and /etc/subgid, and like every shadow-utils database write it leaves the previous contents behind in a dash-suffixed backup. The scrub removed four of the six backups those tools produce, so the retained @factory baseline still named the previous owner in /etc/subuid- and /etc/subgid- along with their subordinate ID range.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit a00be8f)
The runner already rejects a slash, a bare . or .. The installer still
joined the type into hooks/<type>.d before mkdir/cp, so a name nothing
can run could still land on disk.

(cherry picked from commit e522a18)
@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 1f06b9b4-c6a8-45dc-beae-02d5bbe10dbb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

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

@mark-groves
mark-groves marked this pull request as ready for review September 17, 2026 22:13

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agentic security review found one medium-severity path-containment issue in the new factory-reset account scrub. Hook/state name guards did not introduce a comparable residual risk.

Open in Web View Automation 

Sent by Cursor Security Agent: Security Reviewer

users=$(awk -F: '$3 >= 1000 && $3 < 60000 { print $1 }' "$root/etc/passwd") || return 1
for user in $users; do
userdel --root "$root" "$user" 2>>"$LOG_FILE" || return 1
rm -rf "${root:?}/home/$user" || return 1

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔒 Agentic Security Review
Severity: MEDIUM

scrub_factory_accounts reads uid 1000–59999 logins from $root/etc/passwd and, after userdel --root succeeds, runs rm -rf "${root}/home/$user" as root. Those names are not limited to a single path component (no /, not . or ..), unlike the hook/state name guards in this PR.

userdel --root can accept a login such as ../../OUTSIDE. The rm -rf is evaluated on the host, not inside the snapshot, so the path can leave $root. On the production layout ($TOP_MNT/@factory or $TOP_MNT/@omarchy-reset-next), that can resolve to live host paths.

Impact: A previous owner who can plant a nonstandard passwd entry in the factory snapshot can make factory reset delete files outside the snapshot while running as root.

Fix in Cursor Fix in Web

Reviewed by Cursor Security Reviewer for commit 85cca3d. Configure here.

@mark-groves

Copy link
Copy Markdown
Owner

Closing as superseded. The leftover from this sync already landed on quattro via #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.

Factory reset retains the previous owner's password hash

4 participants