Skip to content

Warn when config file is world-readable#16

Merged
myabc merged 1 commit into
cbl-improvementsfrom
fix/config-world-readable
Jun 15, 2026
Merged

Warn when config file is world-readable#16
myabc merged 1 commit into
cbl-improvementsfrom
fix/config-world-readable

Conversation

@myabc

@myabc myabc commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Note

This PR is based off #15

Ticket

n/a

What are you trying to accomplish?

The config file stores API tokens and is written with mode 0600, but nothing detected a file that had been loosened (e.g. an old 0644 file predating the 0600 write, or one chmod-ed by hand). Add a permission check that runs on every command: when the file is accessible by group or other users, print a warning telling the user to chmod 600 it.

Screenshots

image

What approach did you choose and why?

The warning goes to stderr via a new printer.Warning so it never corrupts machine-readable output on stdout. Permission bits are not meaningful on Windows, so the check is skipped there.

Merge checklist

  • Added/updated tests
  • Tested macOS 26.5.1

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a safety warning when the CLI’s config file (which stores API tokens) is readable/writable by group/other users, and routes that warning to stderr to avoid corrupting machine-readable stdout output.

Changes:

  • Extend the printer.Printer interface with Eprintln and add printer.Warning(...) to emit non-fatal diagnostics on stderr.
  • Add configuration.InsecureConfigPermissions() (skipped on Windows) and use it from the root command to warn on insecure config modes.
  • Add tests for the new warning output and config permission detection.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
components/printer/printer.go Extends printer interface to support stderr output.
components/printer/console_printer.go Implements Eprintln writing to os.Stderr.
components/printer/testing_printer.go Implements Eprintln for test printer.
components/printer/common.go Adds Warning(...) helper that prints to stderr.
components/printer/warning_test.go Adds unit test for warning output format/content.
components/configuration/profiles.go Adds config path helper + permission check API.
components/configuration/filemode_test.go Adds tests for insecure-permissions detection.
cmd/root.go Runs the permission check on commands and emits warning.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread components/configuration/profiles.go Outdated
Comment thread cmd/root.go

@cbliard cbliard left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sounds good 👍

Please consider the two comments from copilot. Adding "" is a good suggestion. Updating the docs to remove absolute is a bit nipicking but is valid, so it should be done.

The config file stores API tokens and is written with mode 0600, but
nothing detected a file that had been loosened (e.g. an old 0644 file
predating the 0600 write, or one chmod-ed by hand). Add a permission
check that runs on every command: when the file is accessible by group
or other users, print a warning telling the user to chmod 600 it.

The warning goes to stderr via a new printer.Warning so it never
corrupts machine-readable output on stdout. Permission bits are not
meaningful on Windows, so the check is skipped there.
@myabc myabc force-pushed the fix/config-world-readable branch from 4b30277 to eca5a9f Compare June 15, 2026 11:25
@myabc myabc merged commit 5418b34 into cbl-improvements Jun 15, 2026
@myabc myabc deleted the fix/config-world-readable branch June 15, 2026 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Development

Successfully merging this pull request may close these issues.

3 participants