Skip to content

[wrangler] Write auth config file with mode 0600 and re-chmod on save#14170

Merged
NuroDev merged 1 commit into
mainfrom
pbd/auth-config-file-mode-0600
Jun 3, 2026
Merged

[wrangler] Write auth config file with mode 0600 and re-chmod on save#14170
NuroDev merged 1 commit into
mainfrom
pbd/auth-config-file-mode-0600

Conversation

@petebacondarwin

@petebacondarwin petebacondarwin commented Jun 3, 2026 •
edited by devin-ai-integration Bot
Loading

Copy link
Copy Markdown
Contributor

Tighten on-disk permissions of the Wrangler OAuth credentials file to 0600 and re-chmod on every save so other local users on shared hosts cannot read the stored tokens.

This is the auth-config-file slice of the broader REVIEW-17452 security hardening — extracted here as a stand-alone defence-in-depth change against the user-credentials file. Other parts of the original review apply only to the WebSocket-callback OAuth flow that is not yet on main and will land separately.

What changes

  • writeAuthConfigFile in @cloudflare/workers-auth now passes mode: 0o600 to writeFileSync so newly-created files are tight from the start, and follows the write with an explicit chmodSync(path, 0o600) so files written by older Wrangler versions (or with a looser process umask) get tightened on the next refresh / login.
  • The change is a no-op on Windows, which does not honour POSIX mode bits — the tests assert the mode only on non-Windows platforms; the hardening itself runs unconditionally.

Why both mode and chmodSync

Node's writeFileSync({ mode }) option is only honoured when the file is being created — if the file already exists, the option is silently ignored. The explicit chmodSync covers the upgrade path where an old default.toml already exists with 0o644.

  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: this is a defensive permission change on a credentials file that is already an implementation detail of wrangler login; there is no user-visible API surface or configuration knob.
Open in Devin Review

@petebacondarwin
[wrangler] Write auth config file with mode 0600 and re-chmod on save
af852cc 
Tighten the on-disk permissions of the OAuth credentials file written
by 'wrangler login' so other local users on shared hosts can no longer
read the stored tokens. Existing files with looser permissions from
older Wrangler versions are tightened on the next refresh / login.

The 'mode: 0o600' option on writeFileSync only applies when the file
is created; an explicit chmodSync after the write covers the
already-existing-file case too.
@changeset-bot

changeset-bot Bot commented Jun 3, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: af852cc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
wrangler Patch
@cloudflare/workers-auth Patch
@cloudflare/vite-plugin Patch
@cloudflare/vitest-pool-workers Patch
@cloudflare/wrangler-bundler Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-project-automation github-project-automation Bot moved this to Untriaged in workers-sdk Jun 3, 2026
@workers-devprod workers-devprod requested review from a team and emily-shen and removed request for a team June 3, 2026 16:23
@workers-devprod

workers-devprod commented Jun 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

@ask-bonk

ask-bonk Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

The change centralizes through writeAuthConfigFile, which is the single write path. The implementation is correct and well-tested.

Let me consider the TOCTOU concern more concretely. For a pre-existing file with 0644 ownership belonging to the user, writeFileSync truncates and rewrites it preserving 0644, then chmodSync tightens. There's a tiny window where new tokens are at 0644. However, the prior content was also at 0644 (same sensitive tokens), so this doesn't introduce a new exposure — and it's defence-in-depth on a local file. Not worth flagging as actionable.

The code is correct, follows conventions (uses node:fs prefix, has a changeset, includes tests, skips the assertion on Windows appropriately), and the comment explains the "why". No logic bugs, security regressions, or API issues.

LGTM

github run

devin-ai-integration[bot]
devin-ai-integration Bot reviewed Jun 3, 2026
View reviewed changes

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

✅ All changesets look good

@pkg-pr-new

pkg-pr-new Bot commented Jun 3, 2026

Copy link
Copy Markdown
create-cloudflare

npm i https://pkg.pr.new/cloudflare/workers-sdk/create-cloudflare@14170

@cloudflare/deploy-helpers

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/deploy-helpers@14170

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/kv-asset-handler@14170

miniflare

npm i https://pkg.pr.new/cloudflare/workers-sdk/miniflare@14170

@cloudflare/pages-shared

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/pages-shared@14170

@cloudflare/unenv-preset

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/unenv-preset@14170

@cloudflare/vite-plugin

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/vite-plugin@14170

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/vitest-pool-workers@14170

@cloudflare/workers-auth

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/workers-auth@14170

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/workers-editor-shared@14170

@cloudflare/workers-utils

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/workers-utils@14170

wrangler

npm i https://pkg.pr.new/cloudflare/workers-sdk/wrangler@14170

@cloudflare/wrangler-bundler

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/wrangler-bundler@14170

commit: af852cc

@NuroDev NuroDev requested review from NuroDev and removed request for emily-shen June 3, 2026 16:39
@NuroDev NuroDev enabled auto-merge (squash) June 3, 2026 16:52
workers-devprod
workers-devprod approved these changes Jun 3, 2026
View reviewed changes

@workers-devprod workers-devprod left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codeowners reviews satisfied

@github-project-automation github-project-automation Bot moved this from Untriaged to Approved in workers-sdk Jun 3, 2026
@NuroDev NuroDev merged commit ea12b58 into main Jun 3, 2026
68 of 70 checks passed
@NuroDev NuroDev deleted the pbd/auth-config-file-mode-0600 branch June 3, 2026 17:02
@github-project-automation github-project-automation Bot moved this from Approved to Done in workers-sdk Jun 3, 2026
@workers-devprod workers-devprod mentioned this pull request Jun 3, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@devin-ai-integration devin-ai-integration[bot] devin-ai-integration[bot] left review comments
@NuroDev NuroDev NuroDev approved these changes
@workers-devprod workers-devprod workers-devprod approved these changes

Assignees

No one assigned

Labels

None yet

Projects

workers-sdk
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@petebacondarwin @workers-devprod @NuroDev