write_secret() reports "write failed" when the token was actually saved

[laywill]

write_secret() chains four operations with && and cleans up on failure. The final chmod runs after the rename, so if it fails the function reports an error while the credential is already on disk, and the cleanup targets a path that no longer exists.

// include/exec.php:69-80
function write_secret($path, $value) {
  $dir = dirname($path);
  if (!is_dir($dir) && !@mkdir($dir, 0755, true)) return false;
  $tmp = @tempnam($dir, '.crf-secret-');
  if ($tmp === false) return false;
  $ok = @chmod($tmp, 0600)
    && file_put_contents($tmp, $value, LOCK_EX) !== false
    && @rename($tmp, $path)
    && @chmod($path, 0600);
  if (!$ok) @unlink($tmp);
  return $ok;
}

The false-negative save

If @chmod($path, 0600) fails, $ok is false. But $tmp was already renamed to $path, so @unlink($tmp) is a no-op and the secret is stored. The caller then reports failure:

// include/exec.php:245
echo json_encode(['ok' => $ok, 'action' => 'set-token', 'error' => $ok ? null : 'write failed']);

The user sees "write failed", re-pastes the token or assumes nothing was saved, while a working token sits on disk. Same pattern at exec.php:272.

No security impact: tempnam() creates the file 0600 and rename preserves the mode, so the file at $path is 0600 either way. It's a misreported outcome.

chmod in the chain is a hard gate on the flash filesystem

$CFGDIR is /boot/config/plugins/ci-runner-farm (exec.php:45) — the USB flash, typically vfat. If chmod(2) is refused there, the first @chmod($tmp, 0600) short-circuits the whole chain and no token can ever be saved, with 'write failed' as the only diagnostic.

Not verified on hardware. My Unraid box is my production NAS, so I haven't exercised the credential-write failure paths on it — and since v1.9.0 ships and works, chmod presumably does succeed on /boot. Treat this as worth confirming rather than as a claim. Either way the chmod calls are better decoupled from the success determination, since the file is already 0600 from tempnam.

The UI does promise the mode to the user:

// RunnerFarmSettings.page:63
'Stored at /boot/config/plugins/ci-runner-farm/token (chmod 600)…'

tempnam() can escape the target directory

$tmp = @tempnam($dir, '.crf-secret-');

PHP documents that tempnam() falls back to the system temp directory when it cannot create the file in $dir — the same read-only-flash scenario the file's own comments anticipate at exec.php:168-170. The plaintext secret would then land in /tmp. The cross-device rename() fails afterwards and @unlink cleans up, so this is transient rather than persistent, but it's avoidable.

Suggested fix

function write_secret($path, $value) {
  $dir = dirname($path);
  if (!is_dir($dir) && !@mkdir($dir, 0755, true)) return false;
  $tmp = @tempnam($dir, '.crf-secret-');
  if ($tmp === false) return false;
  // tempnam() falls back to the system temp dir if $dir is not writable; never
  // write a plaintext secret outside the target directory.
  if (realpath(dirname($tmp)) !== realpath($dir)) { @unlink($tmp); return false; }

  // tempnam() already creates the file 0600 and rename() preserves the mode, so
  // the chmod calls are belt-and-braces and must not gate the result.
  @chmod($tmp, 0600);
  if (file_put_contents($tmp, $value, LOCK_EX) === false) { @unlink($tmp); return false; }
  if (!@rename($tmp, $path)) { @unlink($tmp); return false; }
  @chmod($path, 0600);
  return true;
}

I can open a PR for this. Confirmation that chmod works on Unraid's /boot mount would settle how the flags should be handled.

[coderabbitai]

🔗 Related PRs

#42 - feat(auth): add PAT scope presets [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!