feat(auth): use strict OS keychain integration on macOS and Windows (#631)

* fix(auth): implement platform-aware keyring storage

* chore: regenerate skills [skip ci]

* fix(auth): address PR comments on error handling and TOCTOU vulnerabilities

* fix(test): isolate config directory to prevent race conditions during cargo test

* refactor(auth): deduplicate key generation logic in credential store

* fix(test): execute credential tests serially to prevent environment pollution

* fix(auth): sanitize os keyring error strings and handle missing file fallbacks

This commit removes the inadvertently included skills, uses output::sanitize_for_terminal to protect the user from escape sequence injection via keyring errors, and ensures removal of the fallback file properly alerts the user when an io error occurs.

* chore: regenerate skills [skip ci]

---------

Co-authored-by: jpoehnelt-bot <jpoehnelt-bot@users.noreply.github.com>
Co-authored-by: googleworkspace-bot <googleworkspace-bot@users.noreply.github.com>
This commit is contained in:
Justin Poehnelt
2026-03-26 12:40:07 -06:00
committed by GitHub
parent 674d53a65a
commit 80bd150f76
6 changed files with 320 additions and 52 deletions
+8
View File
@@ -0,0 +1,8 @@
---
"@googleworkspace/cli": patch
---
feat(auth): use strict OS keychain integration on macOS and Windows
Closes #623. The CLI no longer writes a fallback `.encryption_key` text file on macOS and Windows when securely storing credentials. Instead, it strictly uses the native OS keychain (Keychain Access on macOS, Credential Manager on Windows). If an old `.encryption_key` file is found during a successful keychain login, it will be automatically deleted for security.
Linux deployments continue to use a seamless file-based fallback by default to ensure maximum compatibility with headless continuous integration (CI) runners, Docker containers, and SSH environments without desktop DBUS services.
+4
View File
@@ -750,6 +750,7 @@ mod tests {
}
#[tokio::test]
#[serial_test::serial]
async fn test_load_credentials_encrypted_file() {
// Simulate an encrypted credentials file
let json = r#"{
@@ -762,6 +763,9 @@ mod tests {
let dir = tempfile::tempdir().unwrap();
let enc_path = dir.path().join("credentials.enc");
// Isolate global config dir to prevent races with other tests
std::env::set_var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR", dir.path());
// Encrypt and write
let encrypted = crate::credential_store::encrypt(json.as_bytes()).unwrap();
std::fs::write(&enc_path, &encrypted).unwrap();
@@ -66,8 +66,8 @@ fn save_key_file_exclusive(path: &std::path::Path, b64_key: &str) -> std::io::Re
/// Persist the base64-encoded encryption key to a local file with restrictive
/// permissions (0600 file, 0700 directory). Overwrites any existing file.
/// Persist the base64-encoded encryption key to a local file with restrictive
/// permissions. Uses atomic_write to prevent TOCTOU/symlink race conditions.
/// Uses atomic_write to prevent TOCTOU/symlink race conditions.
#[allow(dead_code)]
fn save_key_file(path: &std::path::Path, b64_key: &str) -> std::io::Result<()> {
crate::fs_util::atomic_write(path, b64_key.as_bytes())
}
@@ -202,65 +202,123 @@ fn resolve_key(
// --- 1. Try keyring (only when backend = Keyring) --------------------
if backend == KeyringBackend::Keyring {
match provider.get_password() {
Ok(b64_key) => {
if let Ok(decoded) = STANDARD.decode(&b64_key) {
if decoded.len() == 32 {
let mut arr = [0u8; 32];
arr.copy_from_slice(&decoded);
// Ensure file backup stays in sync with keyring so
// credentials survive keyring loss (e.g. after OS
// upgrades, container restarts, daemon changes).
if let Err(err) = save_key_file(key_file, &b64_key) {
eprintln!(
"Warning: failed to sync keyring backup file at '{}': {err}",
key_file.display()
);
#[cfg(any(target_os = "macos", target_os = "windows"))]
{
match provider.get_password() {
Ok(b64_key) => {
if let Ok(decoded) = STANDARD.decode(&b64_key) {
if decoded.len() == 32 {
let mut arr = [0u8; 32];
arr.copy_from_slice(&decoded);
// Cleanup insecure file fallback if it still exists.
// TOCTOU race condition is a known limitation.
if let Err(e) = std::fs::remove_file(key_file) {
if e.kind() != std::io::ErrorKind::NotFound {
eprintln!(
"Warning: failed to remove legacy key file at '{}': {}",
key_file.display(),
e
);
}
}
return Ok(arr);
}
return Ok(arr);
}
// Keyring contained invalid data — fall through to generate new.
}
// Keyring contained invalid data — fall through to file.
}
Err(keyring::Error::NoEntry) => {
// Keyring is reachable but empty — check file, then generate.
if let Some(key) = read_key_file(key_file) {
// Best-effort: copy file key into keyring for future runs.
let _ = provider.set_password(&STANDARD.encode(key));
return Ok(key);
Err(keyring::Error::NoEntry) => {
// Keyring is empty — fall through to generate new.
}
// Generate a new key.
let key = generate_random_key();
let b64_key = STANDARD.encode(key);
// Best-effort: store in keyring.
let _ = provider.set_password(&b64_key);
// Atomically create the file; if another process raced us,
// use their key instead.
match save_key_file_exclusive(key_file, &b64_key) {
Ok(()) => return Ok(key),
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {
if let Some(winner) = read_key_file(key_file) {
// Sync the winner's key into the keyring so both
// backends stay consistent after the race.
let _ = provider.set_password(&STANDARD.encode(winner));
return Ok(winner);
}
// File exists but is unreadable/corrupt — overwrite.
save_key_file(key_file, &b64_key)?;
return Ok(key);
}
Err(e) => return Err(e.into()),
Err(e) => {
anyhow::bail!("OS keyring failed: {}. Set GOOGLE_WORKSPACE_CLI_KEYRING_BACKEND=file to use file storage.", sanitize_for_terminal(&e.to_string()));
}
}
Err(e) => {
eprintln!(
"Warning: keyring access failed, falling back to file storage: {}",
// Generate a new key if keyring was empty or contained invalid data.
let key = generate_random_key();
let b64_key = STANDARD.encode(key);
if let Err(e) = provider.set_password(&b64_key) {
anyhow::bail!(
"Failed to set key in OS keyring: {}",
sanitize_for_terminal(&e.to_string())
);
}
if let Err(e) = std::fs::remove_file(key_file) {
if e.kind() != std::io::ErrorKind::NotFound {
eprintln!(
"Warning: failed to remove legacy key file at '{}': {}",
key_file.display(),
e
);
}
}
return Ok(key);
}
#[cfg(not(any(target_os = "macos", target_os = "windows")))]
{
// On Linux, keyring uses a mock store by default without C DBus dependencies,
// so we continue to use the file fallback for reliability.
match provider.get_password() {
Ok(b64_key) => {
if let Ok(decoded) = STANDARD.decode(&b64_key) {
if decoded.len() == 32 {
let mut arr = [0u8; 32];
arr.copy_from_slice(&decoded);
// Ensure file backup stays in sync with keyring so
// credentials survive keyring loss (e.g. after OS
// upgrades, container restarts, daemon changes).
if let Err(err) = save_key_file(key_file, &b64_key) {
eprintln!(
"Warning: failed to sync keyring backup file at '{}': {err}",
key_file.display()
);
}
return Ok(arr);
}
}
// Keyring contained invalid data — fall through to file.
}
Err(keyring::Error::NoEntry) => {
// Keyring is reachable but empty — check file, then generate.
if let Some(key) = read_key_file(key_file) {
// Best-effort: copy file key into keyring for future runs.
let _ = provider.set_password(&STANDARD.encode(key));
return Ok(key);
}
// Generate a new key.
let key = generate_random_key();
let b64_key = STANDARD.encode(key);
// Best-effort: store in keyring.
let _ = provider.set_password(&b64_key);
// Atomically create the file; if another process raced us,
// use their key instead.
match save_key_file_exclusive(key_file, &b64_key) {
Ok(()) => return Ok(key),
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {
if let Some(winner) = read_key_file(key_file) {
// Sync the winner's key into the keyring so both
// backends stay consistent after the race.
let _ = provider.set_password(&STANDARD.encode(winner));
return Ok(winner);
}
// File exists but is unreadable/corrupt — overwrite.
save_key_file(key_file, &b64_key)?;
return Ok(key);
}
Err(e) => return Err(e.into()),
}
}
Err(e) => {
eprintln!(
"Warning: keyring access failed, falling back to file storage: {}",
sanitize_for_terminal(&e.to_string())
);
}
}
}
}
@@ -293,7 +351,11 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> {
return Ok(*key);
}
#[cfg(not(test))]
let backend = KeyringBackend::from_env();
#[cfg(test)]
let backend = KeyringBackend::File; // Force file to avoid native keychain prompts during test execution
// Item 5: log which backend was selected
eprintln!("Using keyring backend: {}", backend.as_str());
@@ -407,6 +469,8 @@ pub fn load_encrypted() -> anyhow::Result<String> {
#[cfg(test)]
mod tests {
#![allow(dead_code)]
use super::*;
use std::cell::RefCell;
@@ -513,7 +577,73 @@ mod tests {
assert_eq!(result, expected);
}
// ---- Backend::Keyring tests (macOS/Windows specific behavior) ----
#[test]
#[cfg(any(target_os = "macos", target_os = "windows"))]
fn keyring_backend_cleans_up_legacy_file_on_success() {
use base64::{engine::general_purpose::STANDARD, Engine as _};
let dir = tempfile::tempdir().unwrap();
let key_file = dir.path().join(".encryption_key");
// Create a legacy fallback file
std::fs::write(&key_file, STANDARD.encode([99u8; 32])).unwrap();
assert!(key_file.exists());
// Keyring has a valid key
let expected = [7u8; 32];
let mock = MockKeyring::with_password(&STANDARD.encode(expected));
let result = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap();
assert_eq!(result, expected);
assert!(
!key_file.exists(),
"Legacy file must be deleted upon successful keyring read"
);
}
#[test]
#[cfg(any(target_os = "macos", target_os = "windows"))]
fn keyring_backend_cleans_up_legacy_file_on_generation() {
let dir = tempfile::tempdir().unwrap();
let key_file = dir.path().join(".encryption_key");
std::fs::write(&key_file, "legacy-data").unwrap();
assert!(key_file.exists());
let mock = MockKeyring::no_entry();
let result = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap();
assert_eq!(result.len(), 32);
assert!(
!key_file.exists(),
"Legacy file must be deleted upon successful keyring generation"
);
}
#[test]
#[cfg(any(target_os = "macos", target_os = "windows"))]
fn keyring_backend_returns_error_on_platform_failure() {
let dir = tempfile::tempdir().unwrap();
let key_file = dir.path().join(".encryption_key");
let mock = MockKeyring::platform_error();
let result = resolve_key(KeyringBackend::Keyring, &mock, &key_file);
assert!(result.is_err());
assert!(result
.unwrap_err()
.to_string()
.contains("OS keyring failed"));
}
// ---- Backend::Keyring tests (Linux fallback behavior) ----
#[test]
#[cfg(not(any(target_os = "macos", target_os = "windows")))]
fn keyring_backend_creates_file_backup_when_missing() {
use base64::{engine::general_purpose::STANDARD, Engine as _};
let dir = tempfile::tempdir().unwrap();
@@ -535,6 +665,7 @@ mod tests {
}
#[test]
#[cfg(not(any(target_os = "macos", target_os = "windows")))]
fn keyring_backend_syncs_file_when_keyring_differs() {
use base64::{engine::general_purpose::STANDARD, Engine as _};
let dir = tempfile::tempdir().unwrap();
@@ -554,6 +685,7 @@ mod tests {
}
#[test]
#[cfg(not(any(target_os = "macos", target_os = "windows")))]
fn keyring_backend_no_entry_reads_file() {
let dir = tempfile::tempdir().unwrap();
let (expected, key_file) = write_test_key(dir.path());
@@ -568,6 +700,7 @@ mod tests {
}
#[test]
#[cfg(not(any(target_os = "macos", target_os = "windows")))]
fn keyring_backend_no_entry_no_file_generates_and_saves_both() {
let dir = tempfile::tempdir().unwrap();
let key_file = dir.path().join(".encryption_key");
@@ -581,6 +714,7 @@ mod tests {
}
#[test]
#[cfg(not(any(target_os = "macos", target_os = "windows")))]
fn keyring_backend_no_entry_no_file_keyring_set_fails() {
let dir = tempfile::tempdir().unwrap();
let key_file = dir.path().join(".encryption_key");
@@ -593,6 +727,7 @@ mod tests {
}
#[test]
#[cfg(not(any(target_os = "macos", target_os = "windows")))]
fn keyring_backend_platform_error_falls_back_to_file() {
let dir = tempfile::tempdir().unwrap();
let (expected, key_file) = write_test_key(dir.path());
@@ -602,6 +737,7 @@ mod tests {
}
#[test]
#[cfg(not(any(target_os = "macos", target_os = "windows")))]
fn keyring_backend_platform_error_no_file_generates() {
let dir = tempfile::tempdir().unwrap();
let key_file = dir.path().join(".encryption_key");
@@ -612,6 +748,7 @@ mod tests {
}
#[test]
#[cfg(not(any(target_os = "macos", target_os = "windows")))]
fn keyring_backend_invalid_keyring_data_uses_file() {
use base64::{engine::general_purpose::STANDARD, Engine as _};
let dir = tempfile::tempdir().unwrap();
@@ -660,6 +797,7 @@ mod tests {
// ---- Stability tests ----
#[test]
#[cfg(not(any(target_os = "macos", target_os = "windows")))]
fn key_is_stable_across_calls() {
let dir = tempfile::tempdir().unwrap();
let key_file = dir.path().join(".encryption_key");
@@ -858,6 +996,7 @@ mod tests {
// ---- Race condition tests ----
#[test]
#[cfg(not(any(target_os = "macos", target_os = "windows")))]
fn race_loser_syncs_winner_key_to_keyring() {
use base64::{engine::general_purpose::STANDARD, Engine as _};
let dir = tempfile::tempdir().unwrap();
@@ -881,6 +1020,7 @@ mod tests {
}
#[test]
#[cfg(not(any(target_os = "macos", target_os = "windows")))]
fn race_loser_corrupt_file_overwrites() {
let dir = tempfile::tempdir().unwrap();
let key_file = dir.path().join(".encryption_key");
+2
View File
@@ -26,6 +26,7 @@ Core Google Workspace API skills.
| [gws-events](../skills/gws-events/SKILL.md) | Subscribe to Google Workspace events. |
| [gws-modelarmor](../skills/gws-modelarmor/SKILL.md) | Google Model Armor: Filter user-generated content for safety. |
| [gws-workflow](../skills/gws-workflow/SKILL.md) | Google Workflow: Cross-service productivity workflows. |
| [gws-script](../skills/gws-script/SKILL.md) | Manage Google Apps Script projects. |
## Helpers
@@ -57,6 +58,7 @@ Shortcut commands for common operations.
| [gws-workflow-email-to-task](../skills/gws-workflow-email-to-task/SKILL.md) | Google Workflow: Convert a Gmail message into a Google Tasks entry. |
| [gws-workflow-weekly-digest](../skills/gws-workflow-weekly-digest/SKILL.md) | Google Workflow: Weekly summary: this week's meetings + unread email count. |
| [gws-workflow-file-announce](../skills/gws-workflow-file-announce/SKILL.md) | Google Workflow: Announce a Drive file in a Chat space. |
| [gws-script-push](../skills/gws-script-push/SKILL.md) | Google Apps Script: Upload local files to an Apps Script project. |
## Personas
+52
View File
@@ -0,0 +1,52 @@
---
name: gws-script-push
description: "Google Apps Script: Upload local files to an Apps Script project."
metadata:
version: 0.22.2
openclaw:
category: "productivity"
requires:
bins:
- gws
cliHelp: "gws script +push --help"
---
# script +push
> **PREREQUISITE:** Read `../gws-shared/SKILL.md` for auth, global flags, and security rules. If missing, run `gws generate-skills` to create it.
Upload local files to an Apps Script project
## Usage
```bash
gws script +push --script <ID>
```
## Flags
| Flag | Required | Default | Description |
|------|----------|---------|-------------|
| `--script` | ✓ | — | Script Project ID |
| `--dir` | — | — | Directory containing script files (defaults to current dir) |
## Examples
```bash
gws script +push --script SCRIPT_ID
gws script +push --script SCRIPT_ID --dir ./src
```
## Tips
- Supports .gs, .js, .html, and appsscript.json files.
- Skips hidden files and node_modules automatically.
- This replaces ALL files in the project.
> [!CAUTION]
> This is a **write** command — confirm with the user before executing.
## See Also
- [gws-shared](../gws-shared/SKILL.md) — Global flags and auth
- [gws-script](../gws-script/SKILL.md) — All manage google apps script projects commands
+62
View File
@@ -0,0 +1,62 @@
---
name: gws-script
description: "Manage Google Apps Script projects."
metadata:
version: 0.22.2
openclaw:
category: "productivity"
requires:
bins:
- gws
cliHelp: "gws script --help"
---
# script (v1)
> **PREREQUISITE:** Read `../gws-shared/SKILL.md` for auth, global flags, and security rules. If missing, run `gws generate-skills` to create it.
```bash
gws script <resource> <method> [flags]
```
## Helper Commands
| Command | Description |
|---------|-------------|
| [`+push`](../gws-script-push/SKILL.md) | Upload local files to an Apps Script project |
## API Resources
### processes
- `list` — List information about processes made by or on behalf of a user, such as process type and current status.
- `listScriptProcesses` — List information about a script's executed processes, such as process type and current status.
### projects
- `create` — Creates a new, empty script project with no script files and a base manifest file.
- `get` — Gets a script project's metadata.
- `getContent` — Gets the content of the script project, including the code source and metadata for each script file.
- `getMetrics` — Get metrics data for scripts, such as number of executions and active users.
- `updateContent` — Updates the content of the specified script project. This content is stored as the HEAD version, and is used when the script is executed as a trigger, in the script editor, in add-on preview mode, or as a web app or Apps Script API in development mode. This clears all the existing files in the project.
- `deployments` — Operations on the 'deployments' resource
- `versions` — Operations on the 'versions' resource
### scripts
- `run`
## Discovering Commands
Before calling any API method, inspect it:
```bash
# Browse resources and methods
gws script --help
# Inspect a method's required params, types, and defaults
gws schema script.<resource>.<method>
```
Use `gws schema` output to build your `--params` and `--json` flags.