Auto-generate and persist TOKEN_SECRET on first run (#280) - #281

readOrWritePersistedSecret has a race if multiple JSS processes start at the same time (e.g., pm2/cluster). Two processes can both observe ENOENT, generate different secrets, and the later writer wins, leaving the first process using a non-persisted secret and causing token verification failures across processes. Consider making the write atomic (e.g., create the file with an exclusive flag like wx/O_EXCL, and on EEXIST re-read the file).

The code comments/docs indicate the secret dir/file are created with 0700/0600, but mkdirSync(..., { mode }) and writeFileSync(..., { mode }) only apply on creation and won't tighten permissions if the directory/file already exists (and mode on mkdirSync can be ignored when the path exists). If the goal is to enforce these permissions, consider explicitly chmodSyncing path.dirname(filePath) and filePath after ensuring they exist (POSIX only).

This production error path hard-codes ~/.jss/. in the remediation message, but secretPath is injectable (and even for the default path, os.homedir() may not correspond to the literal ~ shown). Consider referencing the actual directory derived from secretPath (e.g., path.dirname(secretPath)) so the guidance matches the path used in the preceding error message.

readOrWritePersistedSecret() unconditionally calls mkdirSync/chmod on the parent dir before attempting to read an existing secret. This can make startup fail (and in production, exit) on read-only filesystems or when the dir is not writable, even if token.secret already exists and is readable. Consider trying to read/return the existing file first, and only creating/chmod’ing the directory when the file is missing/empty; also treat chmod failures as best-effort so they don’t block using an already-present secret.

The “safe against multiple processes starting in parallel” claim isn’t fully upheld: writeFileSync(..., { flag: 'wx' }) creates the file before it is fully written, so another process that hits EEXIST can read an empty/partial file and then overwrite it in the fallback path, leading to different secrets across processes. To make convergence robust, consider an atomic write strategy (e.g., write to a temp file in the same directory and renameSync into place, or retry reads until non-empty instead of overwriting).

After calling exit(1), the function returns undefined “for tests”. If a caller injects an exit stub that doesn’t terminate (or this is used programmatically), downstream code can continue with an undefined secret and fail later in less obvious ways. Consider throwing an error after invoking exit, or returning a valid (but clearly marked) value to prevent accidental continuation when exit is overridden.

The comment says the fast path returns an existing secret "without trying to mkdir or tighten the containing directory", but the code immediately calls chmodBestEffort() on both the directory and file. Please update the comment to match the behavior (or move the chmod calls out if the intent is truly read-only/no-metadata-touch in this path).

The production error message hard-codes "is not writable", but the failure from readOrWritePersistedSecret(secretPath) could also be a read failure (e.g., EACCES reading an existing file) or a rename/readback failure. Consider rewording to something accurate like "could not be read or created" (and optionally include e.code) so operators aren’t misled about the actual fault.

This test comment mentions an exclusive-create ("wx") / EEXIST race, but readOrWritePersistedSecret() currently uses a temp file + rename strategy rather than exclusive creation. Updating the comment to describe the actual concurrency behavior would make the test easier to maintain.