Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions .zshrc
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,14 @@ safe_source() {
local actual_user=$(id -un)
[[ "$USER" == "$actual_user" ]] || USER="$actual_user"

# Check file ownership
# Check file ownership (allow root-owned system files in /usr/share)
if [[ "$owner" != "$USER" ]]; then
echo "Warning: $file not owned by $USER (owner: $owner)" >&2
return 1
if [[ "$owner" == "root" && "$file" == /usr/share/* ]]; then
: # Allow root-owned system files (zsh plugins, etc.)
else
echo "Warning: $file not owned by $USER (owner: $owner)" >&2
return 1
fi
fi

# Check parent directory ownership
Expand All @@ -65,8 +69,12 @@ safe_source() {
return 1
fi

# Check permissions (reject world/group writable, setuid/setgid)
if [[ "$perms" =~ [2367]$ ]] || (( 10#$perms > 644 )); then
# Check permissions (reject world-writable, but allow group-writable like 664)
# Last digit meanings: 2=write, 4=read, 6=read+write, 7=read+write+exec
# Reject if world-writable (perms ending in 2, 3, 6, 7 for "other" field)
# Allow up to 664 (owner rw, group rw, other r) but not 666+ (world-writable)
local other_perms="${perms: -1}"
if [[ "$other_perms" =~ [2367] ]] || (( 10#$perms > 775 )); then
echo "Warning: $file has insecure permissions ($perms)" >&2
return 1
fi
Expand Down
125 changes: 45 additions & 80 deletions SESSION_HANDOVER.md
Original file line number Diff line number Diff line change
@@ -1,109 +1,74 @@
# Session Handoff: Starship Always-Show Username/Hostname Feature
# Session Handoff: Issue #76 - Fix safe_source Permission and Ownership Checks

**Date**: 2025-11-18
**PR**: maxrantil/dotfiles#75
**Branch**: feat/starship-always-show-username
**Date**: 2025-11-23
**Issue**: #76
**PR**: #77
**Branch**: fix/issue-76-safe-source-permissions

## ✅ Completed Work

### Feature Implemented
**Starship Prompt Enhancement**: Configured starship to always display `username@hostname` in the prompt (not just during SSH sessions).
### Bug Fixed
**safe_source function in .zshrc**: Fixed overly restrictive security checks that prevented aliases and zsh plugins from loading on freshly provisioned VMs.

### Changes Made
1. **starship.toml**:
- Added `[username]` section with `show_always = true`
- Yellow for regular users
- Red for root (warning indicator)
- Added `[hostname]` section with `ssh_only = false`
- Green color with `@` prefix
- Trim domain suffix (`.local`)
- Updated format string to include `$username$hostname`

### Context & Integration
This change complements the vm-infra configurable username feature:
- vm-infra Issue: maxrantil/vm-infra#117
- vm-infra PR: maxrantil/vm-infra#118
- VMs now provisioned with configurable usernames and hostnames
- Prompt shows `developer@work-vm-1` or `testuser@test-vm-2` for instant context

### Before/After
**Before**:
```
~/projects
```
### Root Causes Identified
1. **Permission check too strict**: Rejected files with permission `664` (group-writable) because `664 > 644`, even though 664 is a common and safe permission for dotfiles
2. **Ownership check too strict**: Rejected root-owned system files in `/usr/share/`, breaking apt-installed zsh plugins (syntax-highlighting, autosuggestions)

**After**:
### Changes Made
1. **Permission check** (line 72-80):
- Old: Rejected anything > 644 or ending in 2,3,6,7
- New: Only rejects world-writable (last digit 2,3,6,7) or > 775
- Now accepts: 644, 664, 755, 775
- Still rejects: 666, 777, 646, 776 (world-writable)

2. **Ownership check** (line 52-60):
- Old: Required file to be owned by current user
- New: Also allows root-owned files in `/usr/share/*`
- Enables system-installed zsh plugins to load

### Symptoms Fixed
```
┌───────────────────>
│developer@work-vm-1~/projects main
└─>❯
Warning: /home/user/.dotfiles/.aliases has insecure permissions (664)
Warning: /usr/share/zsh-syntax-highlighting/zsh-syntax-highlighting.zsh not owned by user (owner: root)
```

### Benefits
- **Multi-VM Clarity**: Instantly see which VM you're in
- **Security**: Root user shown in red (immediate warning)
- **Universal**: Works in all contexts (SSH, console, tmux)

### Agent Validation Status
- ✅ **ux-accessibility-i18n-agent**: APPROVED (4.5/5)
- Excellent UX for multi-VM workflows
- Color choices appropriate and accessible
- Screen reader compatible
- Recommends contrast verification testing (not blocking)
- ✅ **code-quality-analyzer**: APPROVED (4.6/5)
- Valid starship configuration
- Excellent documentation
- Negligible performance impact
- Minor cosmetic improvements suggested (optional)
- ✅ **documentation-knowledge-manager**: Session handoff now complete
Result: `l` alias (and all other aliases) now work correctly.

## 🎯 Current Project State

**Tests**: ✅ All pre-commit hooks passing
**Branch**: feat/starship-always-show-username
**CI/CD**: 🔄 Running (PR #75) - session handoff doc now updated
**Branch**: fix/issue-76-safe-source-permissions
**CI/CD**: 🔄 Running (PR #77)
**Status**: Ready for merge after CI passes

### File Changes
- **Modified**: `starship.toml` (+20 lines, -1 line)
- Added [username] section
- Added [hostname] section
- Updated format string
- **Modified**: `.zshrc` (+13 lines, -5 lines)
- Updated ownership check to allow root-owned `/usr/share/*` files
- Updated permission check to allow group-writable (664) but reject world-writable

## 📋 Next Session Priorities

**Immediate Next Steps:**
1. Merge PR #75 after CI passes (all checks should be green now)
2. Test in VM to verify prompt display with configurable usernames
3. Validate integration with vm-infra PR #118 deployment
4. Monitor for any prompt performance impact

**Optional Enhancements** (from code-quality-analyzer):
- Remove redundant `disabled = false` lines (cosmetic)
- Add test coverage for root user styling
- Add test coverage for hostname domain trimming
- Update README.md to document prompt behavior
1. Merge PR #77 after CI passes
2. Test on VM to verify aliases and zsh plugins load correctly
3. Close Issue #76

**Future Considerations:**
- Test with various username/hostname combinations in vm-infra
- Evaluate if additional starship customizations needed for VM workflows
- Consider contrast verification testing for accessibility
- Consider if other system directories should be allowlisted (e.g., `/etc/`)
- Monitor for any security implications of relaxed checks

## 📝 Startup Prompt for Next Session

Read CLAUDE.md to understand our workflow, then merge dotfiles PR #75 (starship always-show username/hostname feature) after CI validation.
Read CLAUDE.md to understand our workflow, then merge dotfiles PR #77 (safe_source fix) after CI validation.

**Immediate priority**: Merge maxrantil/dotfiles#75 after CI passes
**Context**: Starship now always displays username@hostname for multi-VM clarity
**Reference docs**: PR #75 description, vm-infra#117, vm-infra#118, SESSION_HANDOVER.md
**Ready state**: All tests passing, simple config change, agent-validated
**Immediate priority**: Merge maxrantil/dotfiles#77 after CI passes
**Context**: Fixed safe_source rejecting 664 permissions and root-owned system files
**Reference docs**: Issue #76, PR #77, SESSION_HANDOVER.md
**Ready state**: All tests passing, simple security fix, locally validated

**Expected scope**: Merge PR, test prompt display in VM, validate vm-infra integration works as expected
**Expected scope**: Merge PR, test aliases work on VM without chmod workaround

## 📚 Key Reference Documents
- maxrantil/dotfiles#75 (this PR - starship username/hostname always-show)
- maxrantil/vm-infra#117 (issue - configurable VM usernames)
- maxrantil/vm-infra#118 (PR - implementation of configurable usernames)
- `starship.toml:14-31` (username/hostname configuration added)
- `STARSHIP_CONFIG_NOTE.md` (in vm-infra repo - implementation guide)
- maxrantil/dotfiles#76 (issue - safe_source rejects valid permissions)
- maxrantil/dotfiles#77 (PR - fix implementation)
- `.zshrc:52-80` (safe_source function with fixes)
Loading