CLI: the credentials file's directory is created without 0700 #1024

Activity

1. github-actions

   added

   needs-triageAwaiting maintainer triageAwaiting maintainer triage

   on Oct 16, 2026
2. ragnorc commented on Oct 17, 2026

   

   ragnorc

   on Oct 17, 2026 – with Cursor

   ContributorAuthor

   More actions

   Reproduction and fix assessment

   Verdict: reproduced, and the scope is wider than the report. The CLI creates the operator directory with the default mode, so the umask sets it (0755 at umask 022). The CLI also accepts an existing directory with any owner or mode. When another account can write to that directory, that account can read the token that omnigraph login stores. I show this below with a second local account.

   Problem

   omnigraph login <server> writes credentials with mode 0600. It does not control the mode of the directory that holds the file. It also does not check the owner or mode of that directory when the directory already exists. The managed session lock directory has these checks, but it holds only lock files. The operator directory holds the bearer tokens and has no checks.

   Environment

   • Main: d883c49e741f0c2d2b863c1d500601c023bd41f5 (latest main when I tested), clean detached worktree.
   • Ubuntu 24.04.5, Linux 6.12.94+, x86_64, rustc 1.97.1, omnigraph 0.14.0 (debug build of -p omnigraph-cli --bin omnigraph).
   • Operator: ubuntu (uid 1000). A second account og_other (uid 1001) tested what other users can do. fs.protected_regular = 2.

   Steps and results

   Each case uses a new path and echo <token> | OMNIGRAPH_HOME=<dir> omnigraph login prod, unless the case says otherwise. Modes are from stat -c '%A %a %U:%G'.

   Case	Setup	Directory result	credentials	login exit
   A	Missing OMNIGRAPH_HOME, umask 022	drwxr-xr-x 755	600	0
   B	Missing ~/.omnigraph (HOME moved, OMNIGRAPH_HOME unset), umask 022	755	600	0
   C	umask 077	700	600	0
   D	umask 002	drwxrwxr-x 775 (group-writable)	600	0
   E	Existing own directory with mode 0777	stays 777, no warning	600	0
   F	OMNIGRAPH_HOME=<tmp>/f/x/y, all three missing	f, x, y all 755	600	0

   In case A, og_other can list the directory but cannot read the file. This agrees with the report:

   $ sudo -u og_other ls -la <tmp>/a
   drwxr-xr-x 2 ubuntu ubuntu 4096 Oct 17 15:57 .
   -rw------- 1 ubuntu ubuntu   21 Oct 17 15:57 credentials
   $ sudo -u og_other cat <tmp>/a/credentials
   cat: <tmp>/a/credentials: Permission denied
   

   Token disclosure when the directory is writable by another account (E2 and G). The temporary file name is predictable (credentials.tmp.<pid>). The CLI opens it without O_EXCL or O_NOFOLLOW. mode(0o600) applies only when the open creates the file. In E2, the operator's own directory is 0777. og_other makes an empty file named with the PID of the login process before login opens it:

   $ stat <tmp>/og-own-loose.pfLy
   drwxrwxrwx 777 ubuntu:ubuntu
   $ OMNIGRAPH_HOME=<dir> bash -c 'sleep 1; exec omnigraph login prod --token tok-E2-secret' & PID=$!
   $ sudo -u og_other bash -c "umask 000; : > <dir>/credentials.tmp.$PID"
   stored credential for 'prod' in <dir>/credentials        # exit 0
   $ stat <dir>/credentials
   -rw-rw-rw- 666 og_other:og_other
   $ sudo -u og_other cat <dir>/credentials
   [prod]
   token = tok-E2-secret
   

   G gives the same result when OMNIGRAPH_HOME names a 0777 directory that og_other owns. login exits 0 and prints no warning. The next credential read fails closed (credentials file ... is group/world-accessible (mode 666); run chmod 600), but the token is already disclosed. By the same mechanism, this also applies to the group in case D if the group has other members (not tested).

   Differences from the report. The report says other users can list the directory. That is correct only when they can traverse its ancestors. On this VM /home/ubuntu is 0750, so the default ~/.omnigraph is protected by the home directory. Relocated paths (OMNIGRAPH_HOME, or HOME under /tmp) are not protected. umask 077 hides the defect (case C). The bigger risk is the write access in cases D, E and G, not the listing.

   Failing test on main

   I added two unit tests next to credentials_written_0600_and_over_permissive_refused in operator.rs. I kept them local and did not push them. I ran:

   cargo test -p omnigraph-cli --locked --bin omnigraph -- operator::tests:: (umask 0022):

   test operator::tests::issue_1024_credentials_dir_created_0700 ... FAILED
   test operator::tests::issue_1024_credentials_refused_in_dir_other_accounts_can_write ... FAILED
   ---- issue_1024_credentials_dir_created_0700 ----
   assertion `left == right` failed: operator dir created 755
     left: 493
    right: 448
   ---- issue_1024_credentials_refused_in_dir_other_accounts_can_write ----
   token written into a world-writable operator dir
   test result: FAILED. 16 passed; 2 failed; 0 ignored; 0 measured; 158 filtered out
   

   Both tests fail on the behavior. They do not fail at setup. The other 16 tests in the module pass.

   Test bodies

   #[cfg(unix)]
   #[test]
   fn issue_1024_credentials_dir_created_0700() {
       use std::os::unix::fs::PermissionsExt;
       let root = tempfile::tempdir().unwrap();
       let dir = root.path().join("operator-home");
       rewrite_credentials_at(&dir.join("credentials"), "prod", Some("tok")).unwrap();
       let mode = fs::metadata(&dir).unwrap().permissions().mode() & 0o777;
       assert_eq!(mode, 0o700, "operator dir created {mode:o}");
   }
   
   #[cfg(unix)]
   #[test]
   fn issue_1024_credentials_refused_in_dir_other_accounts_can_write() {
       use std::os::unix::fs::PermissionsExt;
       let root = tempfile::tempdir().unwrap();
       let dir = root.path().join("operator-home");
       fs::create_dir(&dir).unwrap();
       fs::set_permissions(&dir, fs::Permissions::from_mode(0o777)).unwrap();
       let result = rewrite_credentials_at(&dir.join("credentials"), "prod", Some("tok"));
       assert!(result.is_err(), "token written into a world-writable operator dir");
   }

   The current end-to-end login test does not find this defect. It sets OMNIGRAPH_HOME to a tempfile directory, which already exists with mode 0700 (system_local.rs:2440). Thus the CLI never creates the directory in that test.

   Root cause

   All links are at d883c49e.

   • operator.rs:481-483: std::fs::create_dir_all(parent) creates the operator directory and every missing ancestor with mode 0777 & !umask. If the directory exists, the call does nothing and checks nothing.
   • operator.rs:484-486: the temporary name credentials.tmp.<pid> is predictable. rename then publishes whatever file has that name.
   • operator.rs:496-508: write_owner_only uses create(true).truncate(true).mode(0o600) without create_new (O_EXCL) or O_NOFOLLOW. It opens an existing file or symlink and writes into it. The mode is not applied to an existing file.
   • operator.rs:518-531: refuse_over_permissive checks the mode of the file through metadata (follows symlinks). It does not check the owner. It does not check the directory.
   • operator.rs:510-514 and 533-536: on non-Unix targets the write is std::fs::write and the check does nothing. Windows is a release target.
   • Callers: login and logout at main.rs:494 and :499. The directory comes from operator_dir ($OMNIGRAPH_HOME, or else ~/.omnigraph).

   Comparison with the managed code. managed/auth/coordination.rs:49-103 creates one level with DirBuilder::mode(0o700) and accepts AlreadyExists. It then refuses a symlink or a non-directory, and refuses uid != geteuid() or mode & 0o077 != 0. It opens the lock file with O_NOFOLLOW and 0600, and checks owner and mode again on the open handle. On Windows it uses FILE_FLAG_OPEN_REPARSE_POINT. Its location does not use OMNIGRAPH_HOME (:127-135). The managed tokens are in the OS keychain, so this directory holds only locks. managed/lifecycle/source.rs:259-266 goes further: it uses mkdirat(..., 0o700) and openat(O_NOFOLLOW | O_DIRECTORY) relative to a held directory. Thus the CLI has three implementations of private local state, and the one that holds secrets is the weakest.

   Recommended fix: one private-state primitive

   Put one small module in omnigraph-cli (for example private_fs), and use it for the operator directory and the managed lock directory. The local session directory in RFC #1017 can use it too.

   1. ensure_private_dir(path): create missing components with DirBuilder::new().recursive(true).mode(0o700). Then open the final directory and check it: it must be a directory, owned by the effective uid, and not group- or world-writable.
   2. Existing directories (upgrade path): current releases create the directory with the umask mode, usually 0755. Refusing such directories would break most current users. The recommended behavior:
      • Directory owned by the user and too open: change it to 0700 on login/logout, and report the change on stderr.
      • Directory owned by another account, or writable by group or others on a read path: refuse with an error that tells the user what to do. This follows the current chmod 600 message for the file and OpenSSH StrictModes.
   3. write_private_file(dir, name, bytes): use tempfile::Builder::new().permissions(0o600).tempfile_in(dir) (random name, O_EXCL), then sync_all, persist, and crate::sync_dir. save_context in managed.rs:304-310 already uses this pattern, and tempfile is already a normal dependency. This removes the planted-file disclosure even if the directory check fails. It also makes the write durable; today the write does not sync the file or the directory.
   4. read_private_file: open with O_NOFOLLOW and check owner and mode on the open handle (fstat), not on the path. This adds the missing owner check and removes the race between the check and the read.
   5. Non-Unix: keep the current behavior (Windows profile ACLs, with the reparse-point flag as in coordination.rs). State in the docs that a moved OMNIGRAPH_HOME on Windows gets only the ACL of its parent folder. An explicit DACL needs a new Windows dependency. Track it separately if it is needed.
   6. Tests: put the two regression tests in operator.rs with an _issue_1024 suffix. Add tests for an existing owned 0755 directory (changed to 0700) and a directory owned by root (for example /tmp, which needs no root access to set up). Change the end-to-end login test to use a missing subdirectory of its temp directory. Update the operator-config paragraph in docs/user/cli/reference.md:189 and RFC 0007 §D4, and add a changelog.d "fixed" note.

   Alternatives and tradeoffs

   Option	Fixes	Does not fix	Liability
   Replace create_dir_all with DirBuilder mode(0o700) only	Cases A, B, D, F for new installs	E, E2, G; existing 0755 installs; temp-file planting	Low cost now. Every future private-file writer (#980, #1017) copies its own checks.
   Above, plus chmod 0700 on an existing owned directory	Existing installs and E	G (foreign owner), symlinked directories, temp-file planting	A partial patch at one call site.
   Shared primitive (recommended)	All cases above, plus owner checks and durability	Windows ACLs for moved directories (documented)	One module and one test owner. It replaces the separate checks in operator.rs and coordination.rs, and #1017 can use it.
   Move static tokens to the OS keychain (as managed sessions do)	Removes the file	Headless Linux, CI and containers without Secret Service; changes the RFC 0007 §D4 format and the omnigraph credential design in #1017	Large and partly irreversible. Needs an RFC.

   Open decision for maintainers: some users link ~/.omnigraph to a dotfiles repository. If the primitive refuses symlinks, as coordination.rs does, those users break. I recommend: resolve the symlink for the directory only, and check the target directory. Never follow symlinks for the files in it.

   Risk if not fixed. The disclosure needs another local account with write access to the operator directory. That happens with umask 000 or 002 and a shared group, a chmod 777 by mistake, or an OMNIGRAPH_HOME in a shared location. With write access, that account can probably also add a config.yaml that changes a server's url, and the next --server <name> call would then send the keyed token to that URL. I did not test this. On a single-user machine with umask 022 and a home directory that is not open, the impact is only that other users can see file names and times.

   Lance upstream

   This defect does not involve Lance. operator.rs imports no Lance crate (operator.rs:16-24), and the operator directory is CLI state, not a dataset. OmniGraph pins Lance =11.0.0 (Cargo.toml:52). The latest stable release is v13.0.0 and the latest prerelease is v14.0.0-beta.19. The only related upstream item I found is lance#8838 / lance#8839. That change makes local dataset files follow the umask (0666 base) instead of 0600. It is in v12.0.0 and later, and not in 11.0.0. It changes dataset file modes on a future Lance upgrade, not this directory. It neither fixes nor affects #1024. I found no other matching open or merged-but-unreleased Lance issues or PRs (searched: permissions, umask, chmod, file mode, create_dir_all).

   Related work

   • RFC PR #1017 (draft, for #980) puts session records in /tmp/omnigraph-<uid>/sessions/ with "the owner and mode checks managed/auth/coordination.rs already applies". Its omnigraph credential command reads this same 0600 credentials file. This is a third caller for the shared primitive. It is better to add the primitive before that work starts.
   • #1022 / PR #1030 (where the token is sent) and #1023 / PR #1029 (--as on commit writes) are separate defects. fix(cli)!: send a server token only to the server it is keyed to #1030 also changes operator.rs, but its changes are not in lines 443-536, so this fix should not conflict with it.
   • Supersession: I found no other issue or PR for this defect. This issue stays open.

   Limitations

   • I tested only on Linux. I did not run macOS or Windows. The Windows behavior above comes from reading the code.
   • E2 and G use a planted empty file and a known PID. An attacker without a known PID must plant many names. This makes the attack slower but still possible.
   • I did not test the config.yaml redirection.
   • I ran only the focused operator::tests:: module, not the full workspace test suite. I made no code changes upstream.