Skip to content

fix: fail open instead of aborting under set -e on 1Password DB lookup #31

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
J-MaFf wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: fail open instead of aborting under set -e on 1Password DB lookup #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b836b5c
fix: fail open instead of aborting under set -e on 1Password DB lookup
J-MaFf Jun 16, 2026
3845ac7
test: pin configured-mode deny on DB-less machines; dedupe test helpers
J-MaFf Jul 6, 2026
84151fe
test: match telemetry-aware allow output after rebase onto main
J-MaFf Jul 7, 2026
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
10 changes: 8 additions & 2 deletions hooks/1password-validate-mounted-env-files/hook.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,9 +424,15 @@ db_path=""
mount_hex_data=""

if [[ "$os_type" != "unknown" ]]; then
db_path=$(find_1password_db "$os_type")
# find_1password_db and query_mounts return non-zero as a normal "not found /
# unavailable" signal (e.g. no 1Password desktop database on disk, or sqlite3
# not installed). Under `set -euo pipefail`, a non-zero command substitution in
# an assignment aborts the whole script — which would skip the fail-open logic
# below and crash instead of allowing. Guard with `|| true` so we fall through
# to the existing emptiness checks and fail open as designed.
db_path=$(find_1password_db "$os_type") || true
if [[ -n "$db_path" ]]; then
mount_hex_data=$(query_mounts "$db_path")
mount_hex_data=$(query_mounts "$db_path") || true
fi
fi

Expand Down
109 changes: 91 additions & 18 deletions tests/hooks/1password-validate-mounted-env-files.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,43 @@ load "../test_helper"

HOOK_SCRIPT="${PROJECT_ROOT}/hooks/1password-validate-mounted-env-files/hook.sh"

# Minimal SQLite DB at the path find_1password_db expects; query_mounts requires objects_associated.
create_minimal_1password_sqlite_fixture() {
local fake_home="$1"
local db_path
# Per-OS 1Password data directory under a given home (mirrors the primary
# paths find_1password_db searches).
_1password_data_dir() {
local home="$1"
case "$(uname -s)" in
Darwin*)
db_path="${fake_home}/Library/Group Containers/2BUA8C4S2C.com.1password/Library/Application Support/1Password/Data/1Password.sqlite"
echo "${home}/Library/Group Containers/2BUA8C4S2C.com.1password/Library/Application Support/1Password/Data"
;;
*)
db_path="${fake_home}/.config/1Password/1Password.sqlite"
echo "${home}/.config/1Password"
;;
esac
}

# Minimal SQLite DB at the path find_1password_db expects; query_mounts requires objects_associated.
create_minimal_1password_sqlite_fixture() {
local fake_home="$1"
local db_path
db_path="$(_1password_data_dir "$fake_home")/1Password.sqlite"
mkdir -p "$(dirname "$db_path")"
sqlite3 "$db_path" 'CREATE TABLE objects_associated (key_name TEXT, data BLOB);'
}

# Build a canonical-input payload for one workspace root (python3 for
# JSON-safe escaping of tmpdir paths).
_canonical_for_root() {
python3 -c "import json,sys; print(json.dumps({
'client': 'cursor',
'event': 'before_shell_execution',
'type': 'command',
'workspace_roots': [sys.argv[1]],
'cwd': sys.argv[1],
'command': 'echo hi',
'raw_payload': {},
}))" "$1"
}

canonical_empty_roots='{"client":"cursor","event":"before_shell_execution","type":"command","workspace_roots":[],"cwd":"","command":"echo hi","raw_payload":{}}'
canonical_one_root='{"client":"cursor","event":"before_shell_execution","type":"command","workspace_roots":["/tmp"],"cwd":"/tmp","command":"echo hi","raw_payload":{}}'

Expand Down Expand Up @@ -50,18 +71,7 @@ canonical_one_root='{"client":"cursor","event":"before_shell_execution","type":"
mkdir -p "$ws/.1password"
printf '%s\n' 'mount_paths = [".env.missing"]' > "$ws/.1password/environments.toml"

local payload
payload=$(python3 -c "import json,sys; print(json.dumps({
'client': 'cursor',
'event': 'before_shell_execution',
'type': 'command',
'workspace_roots': [sys.argv[1]],
'cwd': sys.argv[1],
'command': 'echo hi',
'raw_payload': {},
}))" "$ws")

run env HOME="$HOME" bash "$HOOK_SCRIPT" <<<"$payload"
run env HOME="$HOME" bash "$HOOK_SCRIPT" <<<"$(_canonical_for_root "$ws")"
[[ $status -eq 1 ]]
[[ $(printf '%s\n' "$output" | wc -l) -eq 1 ]]
printf '%s' "$output" | python3 -c 'import json,sys; d=json.load(sys.stdin); assert d.get("decision")=="deny" and d.get("message"), d'
Expand Down Expand Up @@ -213,3 +223,66 @@ TOML
assert_lines ".env"
}

# ============================================================================
# Regression tests for `set -e` aborts on the 1Password DB lookup
# ============================================================================
#
# find_1password_db and query_mounts return non-zero as a normal "not found /
# unavailable" signal; the hook must handle that and decide, not abort under
# `set -euo pipefail` — see the comment above the `|| true` guards in hook.sh.
# These tests run without sqlite3 (the exact path the `deny` test above skips).

@test "fails open (allow) when no 1Password database is present" {
# Regression: db_path=$(find_1password_db ...) returns non-zero when no
# database exists on disk.
local home="${BATS_TEST_TMPDIR}/home"
mkdir -p "$home" # intentionally no 1Password sqlite database

local ws="${BATS_TEST_TMPDIR}/workspace"
mkdir -p "$ws" # no .1password/environments.toml -> default mode

run env HOME="$home" bash "$HOOK_SCRIPT" <<<"$(_canonical_for_root "$ws")"

[[ $status -eq 0 ]]
[[ "$output" == '{"decision":"allow","message":"","mode":"default","mount_count":0,"deny_reason":null}' ]]
}

@test "fails open (allow) when the 1Password database cannot be queried" {
# Regression: mount_hex_data=$(query_mounts ...) returns non-zero when
# sqlite3 is missing or the database is invalid/unreadable. A
# present-but-invalid db file reaches query_mounts and forces the
# non-zero path regardless of whether sqlite3 is installed.
local home="${BATS_TEST_TMPDIR}/home"
local db_dir
db_dir="$(_1password_data_dir "$home")"
mkdir -p "$db_dir"
printf 'not-a-valid-sqlite-database' > "${db_dir}/1Password.sqlite"

local ws="${BATS_TEST_TMPDIR}/workspace"
mkdir -p "$ws"

run env HOME="$home" bash "$HOOK_SCRIPT" <<<"$(_canonical_for_root "$ws")"

[[ $status -eq 0 ]]
[[ "$output" == '{"decision":"allow","message":"","mode":"default","mount_count":0,"deny_reason":null}' ]]
}

@test "denies TOML-required mounts even when no 1Password database is present" {
# Pins configured-mode deny on machines without the desktop database —
# the behavior the `|| true` guards make reachable. Before the fix the
# hook aborted before validating, so a required-but-missing mount could
# never block on these machines.
local home="${BATS_TEST_TMPDIR}/home"
mkdir -p "$home" # intentionally no 1Password sqlite database

local ws="${BATS_TEST_TMPDIR}/workspace"
mkdir -p "$ws/.1password"
printf '%s\n' 'mount_paths = [".env.missing"]' > "$ws/.1password/environments.toml"

run env HOME="$home" bash "$HOOK_SCRIPT" <<<"$(_canonical_for_root "$ws")"

[[ $status -eq 1 ]]
[[ $(printf '%s\n' "$output" | wc -l) -eq 1 ]]
printf '%s' "$output" | python3 -c 'import json,sys; d=json.load(sys.stdin); assert d.get("decision")=="deny" and d.get("message"), d'
}