Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Change global config perm #286

Merged

Conversation

xtuc
Copy link
Member

@xtuc xtuc commented Jul 1, 2019

Avoid the global user config to be system readable, restrict any access
to the current user and only in read only (600).

Merge #239 first.

src/commands/config.rs Outdated Show resolved Hide resolved
@xtuc xtuc changed the title feat: change global config perm [WIP] change global config perm Jul 1, 2019
src/commands/config.rs Outdated Show resolved Hide resolved
@xtuc xtuc force-pushed the feat-change-global-config-perms branch from 8e6fea7 to 22c2ba5 Compare July 1, 2019 15:47
@ashleygwilliams ashleygwilliams added this to the 1.1.0 milestone Jul 1, 2019
@xtuc xtuc force-pushed the feat-change-global-config-perms branch 3 times, most recently from 2dfae76 to 529d445 Compare July 2, 2019 12:22
@xtuc xtuc changed the title [WIP] change global config perm Change global config perm Jul 2, 2019

// check dir permissions (but not on windows)
if !cfg!(target_os = "windows") {
let mut command = Command::new("stat");
Copy link
Member Author

Choose a reason for hiding this comment

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

let metadata = config_file.clone().metadata().unwrap();
let permissions = metadata.permissions();
assert_eq!(permissions.mode(), 0o600);

The API returned odd modes, that's why I used stat.

@xtuc xtuc changed the base branch from master to refactor-config-stdin July 2, 2019 12:24
@xtuc xtuc added feature Feature requests and suggestions changelog - feature and removed status - work in progress labels Jul 2, 2019
@xtuc xtuc force-pushed the refactor-config-stdin branch from b313e47 to d25d65b Compare July 2, 2019 12:31
@EverlastingBugstopper
Copy link
Contributor

Could you explain a bit more what this is doing? Having a hard time grokking from title + comment. Does this have to do with how wrangler is installed?

@xtuc
Copy link
Member Author

xtuc commented Jul 2, 2019

sent on gchat.

);

// check dir permissions (but not on windows)
if !cfg!(target_os = "windows") {
Copy link

Choose a reason for hiding this comment

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

What do we do on Windows?

Copy link
Member Author

@xtuc xtuc Jul 2, 2019

Choose a reason for hiding this comment

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

I don't know, cc @steveklabnik @ashleygwilliams do you have an idea?

@xtuc xtuc force-pushed the refactor-config-stdin branch 7 times, most recently from 20d9e75 to cc7782a Compare July 3, 2019 10:45
@xtuc xtuc force-pushed the refactor-config-stdin branch from cc7782a to efb6953 Compare July 10, 2019 10:07
@xtuc xtuc force-pushed the feat-change-global-config-perms branch from 529d445 to 6b566d6 Compare July 10, 2019 10:32
@xtuc xtuc force-pushed the refactor-config-stdin branch from efb6953 to cd52965 Compare July 10, 2019 10:34
@xtuc xtuc force-pushed the feat-change-global-config-perms branch from 6b566d6 to 057da57 Compare July 10, 2019 10:35
@ashleygwilliams ashleygwilliams added the regression Something is broken, but works in previous releases label Jul 15, 2019
@xtuc xtuc force-pushed the refactor-config-stdin branch 2 times, most recently from f0f130d to 0054bf9 Compare July 16, 2019 13:04
This changes the way `wrangler config` works, previously both the email
and the api_key were passed as arguments which would be captured by the
terminal history.

When launching the command it will prompt for an email, once stdin
receive a line it will prompt for the api_key. Both lines are values and
will be stored in the config.
@xtuc xtuc force-pushed the feat-change-global-config-perms branch from 057da57 to 7740667 Compare July 16, 2019 13:08
Avoid the global user config to be system readable, restrict any access
to the current user (600).

Extends the global_config API to allow to specify a configuration
directory to create the configuration file, it's needed for testing.
@xtuc xtuc force-pushed the feat-change-global-config-perms branch from 7740667 to 6f9af42 Compare July 16, 2019 13:09
@xtuc xtuc requested a review from a team July 17, 2019 10:11
Copy link
Contributor

@EverlastingBugstopper EverlastingBugstopper left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@ashleygwilliams ashleygwilliams left a comment

Choose a reason for hiding this comment

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

can we add some tests for this please?

@ashleygwilliams ashleygwilliams merged commit 5576513 into refactor-config-stdin Jul 29, 2019
@delete-merged-branch delete-merged-branch bot deleted the feat-change-global-config-perms branch July 29, 2019 14:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature Feature requests and suggestions regression Something is broken, but works in previous releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants