Skip to content

Fix/warn credential permission failures#124

Merged
jpoehnelt merged 1 commit intogoogleworkspace:mainfrom
jeftekhari:fix/warn-credential-permission-failures
Mar 5, 2026
Merged

Fix/warn credential permission failures#124
jpoehnelt merged 1 commit intogoogleworkspace:mainfrom
jeftekhari:fix/warn-credential-permission-failures

Conversation

@jeftekhari
Copy link
Contributor

Description

Replaced silent let _ = on set_permissions calls in save_encrypted() with eprintln! warnings so users are aware if their credential files end up with insecure permissions. Also log keyring access failures instead of silently falling through to file storage.

This follows the existing pattern already used in get_or_create_key() (lines 92-98) where permission failures are properly warned about.

Changes:

  • save_encrypted: Warn if directory permissions (0o700) fail to set
  • save_encrypted: Warn if credentials file permissions (0o600) fail to set
  • get_or_create_key: Log keyring errors instead of silently ignoring them

Checklist:

  • My code follows the AGENTS.md guidelines (no generated google-* crates).
  • I have run cargo fmt --all to format the code perfectly.
  • I have run cargo clippy -- -D warnings and resolved all warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have provided a Changeset file (e.g. via pnpx changeset) to document my changes.

@changeset-bot
Copy link

changeset-bot bot commented Mar 5, 2026

🦋 Changeset detected

Latest commit: ad69e9c

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

This PR includes changesets to release 1 package
Name Type
@googleworkspace/cli 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

@gemini-code-assist
Copy link
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@jpoehnelt
Copy link
Member

Please fix the conflicts.

@codecov
Copy link

codecov bot commented Mar 5, 2026

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.18%. Comparing base (c6095dd) to head (ad69e9c).
⚠️ Report is 25 commits behind head on main.

Files with missing lines Patch % Lines
src/credential_store.rs 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #124      +/-   ##
==========================================
- Coverage   56.19%   56.18%   -0.01%     
==========================================
  Files          38       38              
  Lines       13860    13862       +2     
==========================================
  Hits         7788     7788              
- Misses       6072     6074       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Replaced silent `let _ =` on set_permissions calls in save_encrypted
with eprintln! warnings so users are aware if their credential files
end up with insecure permissions. Also log keyring access failures
instead of silently falling through to file storage.
@jeftekhari jeftekhari force-pushed the fix/warn-credential-permission-failures branch from aa7f28b to ad69e9c Compare March 5, 2026 18:33
@jpoehnelt jpoehnelt added area: auth cla: yes This human has signed the Contributor License Agreement. complexity: low Small, straightforward change labels Mar 5, 2026
@jpoehnelt
Copy link
Member

tested locally

@jpoehnelt jpoehnelt merged commit 132c3b1 into googleworkspace:main Mar 5, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: auth cla: yes This human has signed the Contributor License Agreement. complexity: low Small, straightforward change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants