From 23d9374d9a186095b81ac6bc099cf09accafda04 Mon Sep 17 00:00:00 2001
From: "google-labs-jules[bot]"
 <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date: Sun, 1 Feb 2026 04:41:50 +0000
Subject: [PATCH] Fix: Secure SSH private key creation using umask

Identified and fixed a race condition in `tools/setup-ssh-keys.sh` where the private key file was created with default permissions (potentially world-readable) before being restricted with `chmod 600`.

Changes:
- Wrapped sensitive file creation in `umask 077` block to ensure files are created with 600 permissions immediately.
- Added `tests/verify_ssh_permissions.sh` to verify that the script restores keys with correct permissions.
- Updated `.jules/sentinel.md` with the security learning.

This eliminates the window of exposure for private keys during restoration.

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

diff --git a/.jules/sentinel.md b/.jules/sentinel.md
new file mode 100644
index 0000000..c1c99f1
--- /dev/null
+++ b/.jules/sentinel.md
@@ -0,0 +1,4 @@
+## 2024-05-22 - [SSH Private Key Race Condition]
+**Vulnerability:** Found a TOCTOU (Time-of-Check to Time-of-Use) race condition in `tools/setup-ssh-keys.sh`. The script created the private key file using output redirection (`>`) which uses the default umask (typically 022), resulting in the file being world-readable for a brief window before `chmod 600` was executed.
+**Learning:** Shell redirection and standard file creation tools respect the current `umask`. Relying on a subsequent `chmod` to secure sensitive files leaves a window of exposure.
+**Prevention:** Always set `umask 077` (or similar restrictive mask) *before* creating sensitive files in shell scripts to ensure they are born secure. Restore the original umask afterwards if necessary.
diff --git a/tests/verify_ssh_permissions.sh b/tests/verify_ssh_permissions.sh
new file mode 100755
index 0000000..65d48e3
--- /dev/null
+++ b/tests/verify_ssh_permissions.sh
@@ -0,0 +1,82 @@
+#!/bin/bash
+set -e
+
+# Setup mock environment
+TEST_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+REPO_ROOT="$(cd "$TEST_DIR/.." && pwd)"
+MOCK_BIN="$TEST_DIR/bin"
+TEMP_HOME="$TEST_DIR/temp_home"
+
+# Clean up previous run
+rm -rf "$TEMP_HOME"
+rm -rf "$MOCK_BIN"
+mkdir -p "$TEMP_HOME"
+mkdir -p "$MOCK_BIN"
+
+# Mock 'op' command
+cat <<EOF > "$MOCK_BIN/op"
+#!/bin/bash
+if [[ "\$1" == "account" && "\$2" == "list" ]]; then
+    exit 0
+elif [[ "\$1" == "item" && "\$2" == "get" ]]; then
+    exit 0
+elif [[ "\$1" == "read" ]]; then
+    if [[ "\$2" == *"private_key"* ]]; then
+        echo "PRIVATE KEY CONTENT"
+    elif [[ "\$2" == *"public_key"* ]]; then
+        echo "PUBLIC KEY CONTENT"
+    fi
+fi
+EOF
+chmod +x "$MOCK_BIN/op"
+
+# Mock 'yq' command (optional, but good to have)
+cat <<EOF > "$MOCK_BIN/yq"
+#!/bin/bash
+exit 1 # simulate not installed or failing, script handles it
+EOF
+chmod +x "$MOCK_BIN/yq"
+
+# Add mock bin to PATH
+export PATH="$MOCK_BIN:$PATH"
+export HOME="$TEMP_HOME"
+export XDG_CONFIG_HOME="$TEMP_HOME/.config"
+
+# Run the script
+echo "Running setup-ssh-keys.sh restore..."
+"$REPO_ROOT/tools/setup-ssh-keys.sh" restore
+
+# Verify files exist
+PRIVATE_KEY="$TEMP_HOME/.ssh/id_ed25519"
+PUBLIC_KEY="$TEMP_HOME/.ssh/id_ed25519.pub"
+
+if [[ ! -f "$PRIVATE_KEY" ]]; then
+    echo "ERROR: Private key not found at $PRIVATE_KEY"
+    exit 1
+fi
+
+if [[ ! -f "$PUBLIC_KEY" ]]; then
+    echo "ERROR: Public key not found at $PUBLIC_KEY"
+    exit 1
+fi
+
+# Verify permissions
+# Private key should be 600 (-rw-------)
+PERMS=$(stat -c "%a" "$PRIVATE_KEY" 2>/dev/null || stat -f "%Lp" "$PRIVATE_KEY")
+if [[ "$PERMS" != "600" ]]; then
+    echo "ERROR: Private key permissions are $PERMS, expected 600"
+    exit 1
+fi
+
+# Public key should be 644 (-rw-r--r--)
+PERMS=$(stat -c "%a" "$PUBLIC_KEY" 2>/dev/null || stat -f "%Lp" "$PUBLIC_KEY")
+if [[ "$PERMS" != "644" ]]; then
+    echo "ERROR: Public key permissions are $PERMS, expected 644"
+    exit 1
+fi
+
+echo "SUCCESS: SSH keys restored with correct permissions."
+
+# Cleanup
+rm -rf "$TEMP_HOME"
+rm -rf "$MOCK_BIN"
diff --git a/tools/setup-ssh-keys.sh b/tools/setup-ssh-keys.sh
index bde52fd..b12a836 100755
--- a/tools/setup-ssh-keys.sh
+++ b/tools/setup-ssh-keys.sh
@@ -148,14 +148,28 @@ cmd_restore() {
 
     say "Restoring SSH key from 1Password..."
 
-    # Create SSH directory
-    mkdir -p "$SSH_DIR"
+    # Create SSH directory safely
+    if [[ ! -d "$SSH_DIR" ]]; then
+        mkdir -p "$SSH_DIR"
+        chmod 700 "$SSH_DIR"
+    fi
+    # Ensure directory permissions are restrictive
     chmod 700 "$SSH_DIR"
 
+    # Set umask to ensure new files are only readable by owner
+    local old_umask
+    old_umask=$(umask)
+    umask 077
+
     # Read private key from 1Password and save locally
+    # umask 077 ensures file is created with 600 permissions
     op read "op://$VAULT/$KEY_NAME/private_key" > "$PRIVATE_KEY_FILE"
+    # Redundant chmod just to be sure, but file was created securely
     chmod 600 "$PRIVATE_KEY_FILE"
 
+    # Restore umask for public key (we want 644)
+    umask "$old_umask"
+
     # Read public key from 1Password and save locally
     op read "op://$VAULT/$KEY_NAME/public_key" > "$PUBLIC_KEY_FILE"
     chmod 644 "$PUBLIC_KEY_FILE"