Skip to content

Hook log is created world-readable at a fixed path in /tmp (lib/logging.sh) #40

Description

@dberzan

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:

  1. 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.
  2. 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.jsonllib/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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions