feat(startup): warn if NVIM_LOG_FILE is inaccessible #38070

Problem:
If NVIM_LOG_FILE, or the default fallback, is inaccessible (e.g.
directory is owned by root), users get confused.

Solution:
Show a warning when $NVIM_LOG_FILE or $XDG_STATE_HOME are inaccessible.

Also fix a latent memory leak: `os_mkdir_recurse` returns a uv error
code (int), but it was stored as `bool`, causing `os_strerror` to
receive an invalid error code and leak memory.

See: https://docs.libuv.org/en/v1.x/errors.html#c.uv_strerror

Co-authored-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Co-authored-by: Justin M. Keyes <justinkz@gmail.com>

runtime/doc/news.txt

@@ -397,6 +397,7 @@ PLUGINS
 STARTUP
 
 • |v:argf| provides file arguments given at startup.
+• Nvim shows a warning if the log file path is inaccessible.
 
 TERMINAL
 

runtime/lua/vim/_core/defaults.lua

@@ -541,6 +541,18 @@ end
 
 --- Default autocommands. See |default-autocmds|
 do
+  -- Warn if $NVIM_LOG_FILE or $XDG_STATE_HOME are inaccessible. #38039
+  if vim.v.vim_did_enter then
+    require('vim._core.log').check_log_file()
+  else
+    vim.api.nvim_create_autocmd('VimEnter', {
+      once = true,
+      callback = function()
+        require('vim._core.log').check_log_file()
+      end,
+    })
+  end
+
   local nvim_terminal_augroup = vim.api.nvim_create_augroup('nvim.terminal', {})
   vim.api.nvim_create_autocmd('BufReadCmd', {
     pattern = 'term://*',

runtime/lua/vim/_core/log.lua

@@ -0,0 +1,30 @@
+local M = {}
+
+--- Checks that the logfile is accessible.
+function M.check_log_file()
+  if vim.fn.mode() == 'c' then -- Ex mode
+    return
+  end
+
+  local wanted = vim.fn.getenv('__NVIM_LOG_FILE_WANT')
+  if not wanted or wanted == vim.NIL then
+    return
+  end
+
+  local actual = vim.fn.getenv('NVIM_LOG_FILE')
+
+  local msg --[[@type string]]
+  if not actual or actual == vim.NIL or actual == '' then
+    msg = ('log: %q not accessible, logging disabled (stderr)'):format(wanted)
+  elseif actual ~= wanted then
+    msg = ('log: %q not accessible, logging to: %q'):format(wanted, actual)
+  else
+    return
+  end
+
+  vim.defer_fn(function()
+    vim.notify(msg, vim.log.levels.WARN)
+  end, 100)
+end
+
+return M

src/nvim/log.c

@@ -65,16 +65,22 @@ static void log_path_init(void)
 {
   size_t size = sizeof(log_file_path);
   expand_env("$" ENV_LOGFILE, log_file_path, (int)size - 1);
-  if (strequal("$" ENV_LOGFILE, log_file_path)
+  bool user_set = !strequal("$" ENV_LOGFILE, log_file_path);
+
+  if (!user_set
       || log_file_path[0] == NUL
       || os_isdir(log_file_path)
       || !log_try_create(log_file_path)) {
+    if (user_set) {  // User-provided $NVIM_LOG_FILE.
+      // Used by _core/log.lua:check_log_file to validate logfile on startup.
+      os_setenv("__NVIM_LOG_FILE_WANT", log_file_path, true);
+    }
     // Make $XDG_STATE_HOME if it does not exist.
     char *loghome = get_xdg_home(kXDGStateHome);
     char *failed_dir = NULL;
-    bool log_dir_failure = false;
+    int log_dir_failure = 0;
     if (!os_isdir(loghome)) {
-      log_dir_failure = (os_mkdir_recurse(loghome, 0700, &failed_dir, NULL) != 0);
+      log_dir_failure = os_mkdir_recurse(loghome, 0700, &failed_dir, NULL);
     }
     XFREE_CLEAR(loghome);
     // Invalid $NVIM_LOG_FILE or failed to expand; fall back to default.
@@ -83,6 +89,10 @@ static void log_path_init(void)
     xfree(defaultpath);
     // Fall back to $CWD/nvim.log
     if (len >= size || !log_try_create(log_file_path)) {
+      if (!user_set) {  // Default fallback path.
+        // Used by _core/log.lua:check_log_file to validate logfile on startup.
+        os_setenv("__NVIM_LOG_FILE_WANT", log_file_path, true);
+      }
       len = xstrlcpy(log_file_path, "nvim.log", size);
     }
     // Fall back to stderr

test/functional/core/log_spec.lua

@@ -99,4 +99,24 @@ describe('log', function()
     -- Child Nvim spawned by jobstart() prepends "c/" to parent name.
     assert_log('c/' .. tid .. '%.%d+%.%d +server_init:%d+: test log message', testlog, 100)
   end)
+
+  it('warns when $NVIM_LOG_FILE is inaccessible', function()
+    clear({
+      args_rm = { '-u' },
+      args = { '--clean' },
+      env = { NVIM_LOG_FILE = '/foo/bar' },
+    })
+    t.retry(nil, nil, function()
+      t.matches('log: "/foo/bar" not accessible, logging to', n.exec_capture('messages'))
+    end)
+
+    clear({
+      args_rm = { '-u' },
+      args = { '--clean' },
+      env = { NVIM_LOG_FILE = '/foo/bar', XDG_STATE_HOME = '/foo2/bar2' },
+    })
+    t.retry(nil, nil, function()
+      t.matches('log: "/foo/bar" not accessible, logging to', n.exec_capture('messages'))
+    end)
+  end)
 end)

test/functional/core/main_spec.lua

@@ -223,6 +223,7 @@ describe('vim._core', function()
       'vim._core.ex_cmd',
       'vim._core.exrc',
       'vim._core.help',
+      'vim._core.log',
       'vim._core.options',
       'vim._core.server',
       'vim._core.shared',