Summary
lib/logging.sh appends to a fixed path in /tmp and the repo never sets a umask or chmod, so the log file is created world-readable (0644 under a default umask of 022). The log contains 1Password environment names, environment/mount UUIDs, and absolute .env file paths.
I have a fix ready and will open a PR against this issue.
Where
https://github.com/1Password/agent-hooks/blob/main/lib/logging.sh#L21-L22
local log_file="${LOG_FILE:-/tmp/1password-hooks.log}"
echo "$log_message" >> "$log_file" 2>/dev/null || true
grep -rn 'chmod\|umask' across the repository returns no matches, so nothing narrows the permissions after creation.
What ends up in the log
No secret values are written. The metadata is not nothing, though — from hooks/1password-validate-mounted-env-files/hook.sh#L605, once per mount on every hook invocation:
log "Checking local .env file with id ${uuid} at path \"${mount_path}\" for environment ${environment_uuid} (${environment_name})"
That is an environment name, two UUIDs, and an absolute path to a project .env file.
Why it matters
Both consequences need an unprivileged local account or an already-compromised local process, so this is hardening rather than a remotely exploitable flaw:
- Disclosure. Any local user can read the developer's project layout,
.env paths, and 1Password environment names from a world-readable file at a path they already know.
- Symlink-following append.
/tmp is world-writable with the sticky bit. The sticky bit prevents deleting or renaming another user's file; it does not prevent an attacker creating /tmp/1password-hooks.log first, as a symlink. Because the path is constant and known in advance, the append then lands on the symlink target with the invoking user's privileges.
Proposed fix
Harden the creation rather than moving the default path, so no existing install sees its log move:
- Create the file with
umask 077 before the first append, so it is never briefly world-readable.
- Refuse to append when the path is a symlink to a regular file or to a not-yet-existing target. A symlink to a character device or pipe (
/dev/stdout, /dev/stderr) stays honoured.
- Refuse to append to a regular file owned by another user.
chmod 600 a log left behind by an earlier version, so existing installs are fixed on upgrade rather than keeping a 0644 file forever.
- Skip the write silently on any refusal, preserving the existing rule that logging never affects a hook decision.
Non-regular paths are left exactly as they are — notably /dev/null, which tests/test_helper.bash uses to silence logging, so the suite keeps working unchanged.
An alternative would be moving the default off /tmp entirely, to something like ${XDG_STATE_HOME:-$HOME/.local/state}/1password/agent-hooks.log, alongside the ~/.config/1Password/ directory lib/telemetry.sh already writes to. That is arguably the cleaner shape but changes where existing users' logs appear, so I did not assume it. Happy to switch the PR to that if you prefer.
Secondary documentation note
The README states that telemetry collects no file paths, environment names, or other PII. That is accurate for events.jsonl — lib/telemetry.sh writes only hook name, version, client, event type, decision, a bucketed duration and a mount count. The log file is a separate sink with different contents, and a reader can carry the telemetry claim over to the tool as a whole. A sentence distinguishing the two sinks would prevent that. I have not touched the README in the PR; say the word and I will.
Workaround for users today
LOG_FILE is already honoured, so pointing it at a path inside a 0700 directory avoids both consequences with no code change.
How this was found
Static review of the repo at 81eb721 before installing it — a scanner pass plus a manual read of the shell. No scanner flagged this; it came out of the manual read.
Summary
lib/logging.shappends to a fixed path in/tmpand the repo never sets aumaskorchmod, so the log file is created world-readable (0644 under a default umask of 022). The log contains 1Password environment names, environment/mount UUIDs, and absolute.envfile paths.I have a fix ready and will open a PR against this issue.
Where
https://github.com/1Password/agent-hooks/blob/main/lib/logging.sh#L21-L22
grep -rn 'chmod\|umask'across the repository returns no matches, so nothing narrows the permissions after creation.What ends up in the log
No secret values are written. The metadata is not nothing, though — from
hooks/1password-validate-mounted-env-files/hook.sh#L605, once per mount on every hook invocation:log "Checking local .env file with id ${uuid} at path \"${mount_path}\" for environment ${environment_uuid} (${environment_name})"That is an environment name, two UUIDs, and an absolute path to a project
.envfile.Why it matters
Both consequences need an unprivileged local account or an already-compromised local process, so this is hardening rather than a remotely exploitable flaw:
.envpaths, and 1Password environment names from a world-readable file at a path they already know./tmpis world-writable with the sticky bit. The sticky bit prevents deleting or renaming another user's file; it does not prevent an attacker creating/tmp/1password-hooks.logfirst, as a symlink. Because the path is constant and known in advance, the append then lands on the symlink target with the invoking user's privileges.Proposed fix
Harden the creation rather than moving the default path, so no existing install sees its log move:
umask 077before the first append, so it is never briefly world-readable./dev/stdout,/dev/stderr) stays honoured.chmod 600a log left behind by an earlier version, so existing installs are fixed on upgrade rather than keeping a 0644 file forever.Non-regular paths are left exactly as they are — notably
/dev/null, whichtests/test_helper.bashuses to silence logging, so the suite keeps working unchanged.An alternative would be moving the default off
/tmpentirely, to something like${XDG_STATE_HOME:-$HOME/.local/state}/1password/agent-hooks.log, alongside the~/.config/1Password/directorylib/telemetry.shalready writes to. That is arguably the cleaner shape but changes where existing users' logs appear, so I did not assume it. Happy to switch the PR to that if you prefer.Secondary documentation note
The README states that telemetry collects no file paths, environment names, or other PII. That is accurate for
events.jsonl—lib/telemetry.shwrites only hook name, version, client, event type, decision, a bucketed duration and a mount count. The log file is a separate sink with different contents, and a reader can carry the telemetry claim over to the tool as a whole. A sentence distinguishing the two sinks would prevent that. I have not touched the README in the PR; say the word and I will.Workaround for users today
LOG_FILEis already honoured, so pointing it at a path inside a0700directory avoids both consequences with no code change.How this was found
Static review of the repo at
81eb721before installing it — a scanner pass plus a manual read of the shell. No scanner flagged this; it came out of the manual read.