fix: harden state after direct start modes exit - #16
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (3)**/*.md⚙️ CodeRabbit configuration file
Files:
scripts/**/*.sh⚙️ CodeRabbit configuration file
Files:
images/**/Dockerfile⚙️ CodeRabbit configuration file
Files:
🪛 Shellcheck (0.11.0)scripts/smoke-test.sh[warning] 57-57: attempt appears unused. Verify use (or export if used externally). (SC2034) [info] 179-179: Expressions don't expand in single quotes, use double quotes for that. (SC2016) [info] 283-283: Expressions don't expand in single quotes, use double quotes for that. (SC2016) 🔇 Additional comments (9)
Summary by CodeRabbit
WalkthroughDirect Codex and shell tmux sessions now run through a wrapper that reapplies credential hardening after the foreground command exits. The wrapper is packaged into the image, and smoke tests verify command status, session termination, and ChangesDirect session hardening
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Tmux
participant DirectSession as run-direct-session
participant ForegroundCommand as codex or bash --login
participant PersistentState as secure-persistent-state
Tmux->>DirectSession: start foreground command
DirectSession->>ForegroundCommand: execute command
ForegroundCommand-->>DirectSession: return exit status
DirectSession->>PersistentState: reapply credential hardening
PersistentState-->>DirectSession: return hardening status
DirectSession-->>Tmux: return final status
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
What changed
run-direct-sessionwrapper for foreground direct sessions.START_MODE=codexandSTART_MODE=shellthrough that wrapper instead of executing Codex or Bash directly.secure-persistent-statewhenever the direct foreground process exits.auth.jsonto mode0600.Why
The menu path already hardened persistent credentials after interactive actions, but the supported direct
codexandshellstart modes usedexecon the foreground process. A token refresh orcodex logincould therefore leaveauth.jsongroup-readable on permissive TrueNAS dataset ACLs until the entire container restarted.Impact
Validation
23; it preserved that status and changed a deliberately permissiveauth.jsonfrom0660to0600.mainwith no unrelated changes.Root cause
The direct tmux commands executed
codexorbash --loginthemselves, so no parent process remained to invokesecure-persistent-stateafter they exited. The new wrapper remains as the parent and performs hardening from anEXITtrap.Follow-up
After CI and review pass, merge this fix and retest the newly published edge image on TrueNAS. No stable release is created by this PR.