From f49429b1021f53e684363a510d6d04876353fdf0 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 11 Feb 2026 04:49:13 +0000 Subject: [PATCH] fix(security): prevent TOCTOU race condition in SSH key creation - Use `umask 077` in a subshell when creating sensitive directories and files. - Ensure SSH private keys are created with 600 permissions atomically. - Add regression test `tests/test_ssh_creation.sh` to verify secure permissions. - Add security learning to `.jules/sentinel.md`. This prevents a brief window where the private key could be world-readable before `chmod` is applied. Co-authored-by: kidchenko <5432753+kidchenko@users.noreply.github.com> --- .jules/sentinel.md | 4 ++ tests/test_ssh_creation.sh | 78 ++++++++++++++++++++++++++++++++++++++ tools/setup-ssh-keys.sh | 13 +++++-- 3 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 .jules/sentinel.md create mode 100755 tests/test_ssh_creation.sh diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..9ec3688 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2025-02-11 - Secure File Creation with umask +**Vulnerability:** SSH private keys were created with default umask (often 022/002), making them world-readable for a brief window before `chmod` (TOCTOU race condition). +**Learning:** Redirection `>` in shell scripts respects current umask, creating files with potentially insecure permissions by default. `chmod` after creation is insufficient for high-security files. +**Prevention:** Wrap sensitive file creation commands in a subshell with `umask 077` (e.g., `(umask 077; command > file)`). This ensures atomic secure creation. diff --git a/tests/test_ssh_creation.sh b/tests/test_ssh_creation.sh new file mode 100755 index 0000000..e672e58 --- /dev/null +++ b/tests/test_ssh_creation.sh @@ -0,0 +1,78 @@ +#!/bin/bash +set -e + +# Setup mock environment +TEST_HOME=$(mktemp -d) +export HOME="$TEST_HOME" +export XDG_CONFIG_HOME="$TEST_HOME/.config" +mkdir -p "$XDG_CONFIG_HOME/dotfiles" + +# Create a wrapper script for op because export -f might not work if script calls op directly via PATH +mkdir -p "$TEST_HOME/bin" +cat <<'EOF' > "$TEST_HOME/bin/op" +#!/bin/bash +mock_op() { + if [[ "$1" == "account" && "$2" == "list" ]]; then + return 0 # Simulate signed in + elif [[ "$1" == "item" && "$2" == "get" ]]; then + return 0 # Simulate key exists + elif [[ "$1" == "read" ]]; then + if [[ "$2" == *"private_key"* ]]; then + echo "mock-private-key-content" + else + echo "mock-public-key-content" + fi + else + echo "mock-op-called-with: $@" >&2 + return 0 + fi +} +mock_op "$@" +EOF +chmod +x "$TEST_HOME/bin/op" +export PATH="$TEST_HOME/bin:$PATH" + +# Setup yq mock if needed (script uses yq to read config) +# But we pass --vault and --name so it might skip config reading or use defaults. +# If yq is missing, script might fail or fallback. +# load_config checks command -v yq. +# Let's verify if yq is installed in the environment. +if ! command -v yq &>/dev/null; then + # Mock yq + cat <<'EOF' > "$TEST_HOME/bin/yq" +#!/bin/bash +echo "null" +EOF + chmod +x "$TEST_HOME/bin/yq" +fi + +# Run the restore command +# We use --vault and --name to bypass interactive prompt if needed. +# Since local key doesn't exist, cmd_restore should run without prompting for overwrite. + +echo "Running setup-ssh-keys.sh restore..." +./tools/setup-ssh-keys.sh restore --vault test --name test-key + +# Check permissions +KEY_FILE="$TEST_HOME/.ssh/id_ed25519" +SSH_DIR="$TEST_HOME/.ssh" + +if [[ ! -f "$KEY_FILE" ]]; then + echo "FAIL: Key file not created" + exit 1 +fi + +PERMS=$(stat -c "%a" "$KEY_FILE") +if [[ "$PERMS" != "600" ]]; then + echo "FAIL: Private key permissions are $PERMS (expected 600)" + exit 1 +fi + +DIR_PERMS=$(stat -c "%a" "$SSH_DIR") +if [[ "$DIR_PERMS" != "700" ]]; then + echo "FAIL: SSH directory permissions are $DIR_PERMS (expected 700)" + exit 1 +fi + +echo "PASS: SSH key creation secure" +rm -rf "$TEST_HOME" diff --git a/tools/setup-ssh-keys.sh b/tools/setup-ssh-keys.sh index bde52fd..1399bca 100755 --- a/tools/setup-ssh-keys.sh +++ b/tools/setup-ssh-keys.sh @@ -148,12 +148,17 @@ cmd_restore() { say "Restoring SSH key from 1Password..." - # Create SSH directory - mkdir -p "$SSH_DIR" + # Create SSH directory securely + (umask 077 && mkdir -p "$SSH_DIR") + # Redundant chmod for defense in depth chmod 700 "$SSH_DIR" - # Read private key from 1Password and save locally - op read "op://$VAULT/$KEY_NAME/private_key" > "$PRIVATE_KEY_FILE" + # Read private key from 1Password and save locally securely + ( + umask 077 + op read "op://$VAULT/$KEY_NAME/private_key" > "$PRIVATE_KEY_FILE" + ) + # Redundant chmod for defense in depth chmod 600 "$PRIVATE_KEY_FILE" # Read public key from 1Password and save locally