Skip to content

Conversation

@aronchick
Copy link

@aronchick aronchick commented Feb 7, 2026

Problem

Config files containing API tokens are created with default permissions (often 0644), making them readable by other users on shared systems.

Solution

  1. Security: Set restrictive file permissions

    • Config files: 0600 (owner read/write only)
    • Config directories: 0700 (owner only)
    • Explicit chmod call for existing files
  2. Maintainability: Extract resolveConfigPath() helper

    • Reduces duplicate 5-line blocks (4x → 1x)
    • Same legacy fallback logic in one place

Changes

  • packages/clawdhub/src/config.ts: Add chmod import, extract helper, secure writeGlobalConfig

Testing

clawhub login
ls -la ~/.config/clawhub/
# Should show: -rw------- config.json (0600)

Greptile Overview

Greptile Summary

This PR updates the CLI global config handling in packages/clawdhub/src/config.ts by (1) centralizing the legacy clawdhubclawhub fallback logic into resolveConfigPath(), and (2) attempting to harden config persistence by creating the config directory with 0o700, writing the config file with 0o600, and chmod-ing the file on non-Windows platforms after write.

Overall, it aligns config path resolution across macOS/XDG/Windows/default locations and improves token file protection, but the directory-permission hardening is not fully enforced for pre-existing directories.

Confidence Score: 4/5

  • This PR is mostly safe to merge, but directory permission hardening is incomplete for existing config directories.
  • The file-permission changes are straightforward and scoped, and path-resolution refactor is equivalent to the prior logic. The main concern is that directory permissions are only set on creation (and can be affected by umask), so the PR’s stated security guarantees aren’t met when the directory already exists with broader permissions.
  • packages/clawdhub/src/config.ts

(4/5) You can add custom instructions or style guidelines for the agent here!

Security:
- Config files now created with 0600 permissions (owner read/write only)
- Config directories created with 0700 permissions
- Protects API tokens from other users on shared systems

Maintainability:
- Extract resolveConfigPath() helper to reduce code duplication
- Same legacy fallback logic (clawhub -> clawdhub) now in one place
@vercel
Copy link
Contributor

vercel bot commented Feb 7, 2026

@aronchick is attempting to deploy a commit to the Amantus Machina Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +59 to +61
// Create directory with restricted permissions (owner only)
await mkdir(dir, { recursive: true, mode: 0o700 })

Copy link

Choose a reason for hiding this comment

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

Directory perms not enforced

The comment/PR description implies config directories will be 0700, but mkdir(dir, { recursive: true, mode: 0o700 }) only affects directories created in this call (and can be further reduced by umask). If ~/.config/clawhub (or the macOS/APPDATA equivalent) already exists with broader perms (e.g. 0755), this code won’t tighten it, leaving the directory world-readable/executable even after writing the token file.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/clawdhub/src/config.ts
Line: 59:61

Comment:
**Directory perms not enforced**

The comment/PR description implies config directories will be `0700`, but `mkdir(dir, { recursive: true, mode: 0o700 })` only affects directories created in this call (and can be further reduced by `umask`). If `~/.config/clawhub` (or the macOS/APPDATA equivalent) already exists with broader perms (e.g. `0755`), this code won’t tighten it, leaving the directory world-readable/executable even after writing the token file.

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant