From 82d81581e5e078536f05cda48db6f5c8a974a26a Mon Sep 17 00:00:00 2001
From: "google-labs-jules[bot]"
 <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date: Mon, 16 Feb 2026 04:49:32 +0000
Subject: [PATCH] fix(security): prevent TOCTOU race condition in SSH key
 creation

- Use `(umask 077; ...)` subshell to ensure private keys are created with 600 permissions atomically.
- Remove existing file before creation to ensure permissions are reset.
- Add regression test `tests/security_ssh_permissions.sh`.
- Update Sentinel journal.

Co-authored-by: kidchenko <5432753+kidchenko@users.noreply.github.com>
---
 .jules/sentinel.md                |  4 ++
 tests/security_ssh_permissions.sh | 62 +++++++++++++++++++++++++++++++
 tools/setup-ssh-keys.sh           | 10 ++++-
 3 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 .jules/sentinel.md
 create mode 100644 tests/security_ssh_permissions.sh

diff --git a/.jules/sentinel.md b/.jules/sentinel.md
new file mode 100644
index 0000000..0d5c1c0
--- /dev/null
+++ b/.jules/sentinel.md
@@ -0,0 +1,4 @@
+## 2025-02-16 - TOCTOU Race Condition in Shell Scripts
+**Vulnerability:** Private SSH keys were created using redirection (`> file`) before restricting permissions (`chmod 600`), leaving a race window where keys were world-readable.
+**Learning:** Shell redirection creates files with default umask permissions (often 644) *before* any subsequent `chmod` command runs.
+**Prevention:** Use `(umask 077; command > file)` in a subshell to ensure files are created with restrictive permissions atomically. Also ensure existing files are removed before recreation.
diff --git a/tests/security_ssh_permissions.sh b/tests/security_ssh_permissions.sh
new file mode 100644
index 0000000..f4304f5
--- /dev/null
+++ b/tests/security_ssh_permissions.sh
@@ -0,0 +1,62 @@
+#!/bin/bash
+# Test for secure file creation logic
+set -e
+
+TEST_FILE="tests/ssh_key_test"
+
+# Function to check permissions
+check_permissions() {
+    local file="$1"
+    local expected="$2"
+    local perms
+    if [[ "$(uname -s)" == "Darwin" ]]; then
+        perms=$(stat -f "%Lp" "$file")
+    else
+        perms=$(stat -c "%a" "$file")
+    fi
+
+    if [[ "$perms" != "$expected" ]]; then
+        echo "FAIL: Permissions for $file are $perms, expected $expected"
+        return 1
+    else
+        echo "PASS: Permissions for $file are $perms"
+        return 0
+    fi
+}
+
+echo "=== Test 1: Insecure Creation (Baseline) ==="
+rm -f "$TEST_FILE"
+# Standard creation (vulnerable pattern)
+echo "secret" > "$TEST_FILE"
+# Check if permissions are 644 (or 664 depending on umask)
+# Assuming default umask 022 -> 644
+# We check if it is NOT 600
+perms=$(stat -c "%a" "$TEST_FILE" 2>/dev/null || stat -f "%Lp" "$TEST_FILE")
+if [[ "$perms" != "600" ]]; then
+    echo "PASS: Default creation is insecure ($perms)"
+else
+    echo "WARN: Default umask is already strict (077)? This test assumes default umask allows group/other read."
+fi
+
+echo "=== Test 2: Secure Creation (Fix Verification) ==="
+rm -f "$TEST_FILE"
+# Secure creation
+(umask 077; echo "secret" > "$TEST_FILE")
+check_permissions "$TEST_FILE" "600"
+
+echo "=== Test 3: Existing File (Regression Check) ==="
+rm -f "$TEST_FILE"
+# Create with 644
+echo "old content" > "$TEST_FILE"
+chmod 644 "$TEST_FILE"
+
+# Try to overwrite with umask 077 (simulating the fix applied blindly)
+(umask 077; echo "new content" > "$TEST_FILE")
+# Should still be 644 because file existed
+if check_permissions "$TEST_FILE" "644"; then
+    echo "Confirmed: Overwriting existing file preserves insecure permissions (Must delete file first!)"
+else
+    echo "Unexpected: Permissions changed to $perms?"
+fi
+
+rm -f "$TEST_FILE"
diff --git a/tools/setup-ssh-keys.sh b/tools/setup-ssh-keys.sh
index bde52fd..473b5dc 100755
--- a/tools/setup-ssh-keys.sh
+++ b/tools/setup-ssh-keys.sh
@@ -153,8 +153,14 @@ cmd_restore() {
     chmod 700 "$SSH_DIR"
 
     # Read private key from 1Password and save locally
-    op read "op://$VAULT/$KEY_NAME/private_key" > "$PRIVATE_KEY_FILE"
-    chmod 600 "$PRIVATE_KEY_FILE"
+    # Note: Use umask in subshell to ensure file is created with 600 permissions
+    # to avoid race condition where file is briefly world-readable.
+    # Remove existing file first to ensure permissions are reset.
+    [[ -f "$PRIVATE_KEY_FILE" ]] && rm -f "$PRIVATE_KEY_FILE"
+    (
+        umask 077
+        op read "op://$VAULT/$KEY_NAME/private_key" > "$PRIVATE_KEY_FILE"
+    )
 
     # Read public key from 1Password and save locally
     op read "op://$VAULT/$KEY_NAME/public_key" > "$PUBLIC_KEY_FILE"