Skip to content

Commit 02a3d08

Browse files
voidstackloopclaude
andcommitted
Add OS-keyring API key storage, file permission hardening, and pip packaging fixes
Security: buffdata auth set/remove/status stores provider API keys in the OS-native credential store (Windows Credential Manager / macOS Keychain / Linux Secret Service) via keyring, now a base dependency. The default secret resolver checks the environment first and falls back to the keyring automatically, so a stored key just works with zero other configuration. Checkpoints, report.json, and the audit database now get owner-only (0600) file permissions on write. Packaging: fixed two real dependency bugs a genuinely fresh-venv install caught -- orjson and xxhash were imported directly but never declared, so `pip install buffdata` would crash on import (orjson) or on the default exact-dedup path (xxhash) for every new user. Also dropped a typer[all] extra that no longer exists upstream, and added PyPI metadata (classifiers, keywords, project URLs). Verified end-to-end against a real fresh install from live PyPI (buffdata 0.3.0). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 1ef691e commit 02a3d08

14 files changed

Lines changed: 539 additions & 9 deletions

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,8 @@ contract. BuffData never sends a failed request to a different provider automati
249249
- Local LLM servers (Ollama, LM Studio, vLLM, llama.cpp) or any other OpenAI-compatible
250250
endpoint, on this machine or another one on your network -- with an optional
251251
structural guarantee (`--network-policy local`) that calls can never leave it.
252+
- API keys stored in your OS's native credential store (`buffdata auth set`), not a
253+
plaintext `.env` file -- see [docs/security.md](docs/security.md).
252254
- Local PII redaction before any dataset content reaches a provider.
253255
- Schema-preserving JSON/JSONL/NDJSON, CSV/TSV, Parquet, Arrow/Feather/IPC,
254256
YAML, plain-text, and Hugging Face on-disk dataset processing.

buffdata/cli/main.py

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import asyncio
2+
import os
23
from pathlib import Path
34
from typing import List, Optional
45
import typer
@@ -1238,3 +1239,102 @@ def sbom_cmd(
12381239
console.print(
12391240
f"[bold green]Wrote SBOM[/bold green] ({len(sbom['components'])} components) to {output}"
12401241
)
1242+
1243+
1244+
auth_app = typer.Typer(
1245+
help="Store or remove provider API keys in the OS-native credential store (Windows "
1246+
"Credential Manager / macOS Keychain / Linux Secret Service), instead of a plaintext "
1247+
"file or a shell history entry."
1248+
)
1249+
app.add_typer(auth_app, name="auth")
1250+
1251+
# Not exhaustive -- any name works, since the OS keyring just stores whatever key you give
1252+
# it -- but this is every secret name a stock BuffData install actually looks for, so
1253+
# `buffdata auth status` has something concrete to check.
1254+
_KNOWN_SECRET_NAMES = [
1255+
"GEMINI_API_KEY", "GOOGLE_API_KEY", "OPENAI_API_KEY", "ANTHROPIC_API_KEY",
1256+
"AZURE_OPENAI_API_KEY", "OPENAI_COMPATIBLE_API_KEY",
1257+
"OLLAMA_API_KEY", "LMSTUDIO_API_KEY", "VLLM_API_KEY", "LLAMACPP_API_KEY",
1258+
]
1259+
1260+
1261+
def _keyring_service_name() -> str:
1262+
return os.getenv("BUFFDATA_KEYRING_SERVICE", "buffdata")
1263+
1264+
1265+
@auth_app.command("set")
1266+
def auth_set_cmd(
1267+
key_name: str = typer.Argument(
1268+
...,
1269+
help=f"Secret name to store, e.g. one of: {', '.join(_KNOWN_SECRET_NAMES)}",
1270+
),
1271+
):
1272+
"""Store a provider API key in the OS keyring -- prompted with hidden input, never
1273+
echoed to the terminal, never written to any file or shell history. Every buffdata
1274+
command that needs it picks it up automatically afterward with no other
1275+
configuration: the default secret backend checks the OS keyring whenever the
1276+
matching environment variable isn't already set (see buffdata/engine/secrets.py).
1277+
"""
1278+
try:
1279+
import keyring
1280+
except ImportError:
1281+
console.print("[bold red]Install keyring (pip install keyring) to use `buffdata auth`.[/bold red]")
1282+
raise typer.Exit(1)
1283+
1284+
value = typer.prompt(f"Value for {key_name}", hide_input=True, confirmation_prompt=True)
1285+
if not value:
1286+
console.print("[yellow]Empty value -- nothing stored.[/yellow]")
1287+
raise typer.Exit(1)
1288+
try:
1289+
keyring.set_password(_keyring_service_name(), key_name, value)
1290+
except Exception as exc:
1291+
console.print(f"[bold red]Could not store {key_name} in the OS keyring: {exc}[/bold red]")
1292+
raise typer.Exit(1)
1293+
console.print(
1294+
f"[bold green]Stored {key_name} in the OS keyring.[/bold green] No .env file or "
1295+
"BUFFDATA_SECRET_BACKEND change needed -- it's used automatically from here on."
1296+
)
1297+
1298+
1299+
@auth_app.command("remove")
1300+
def auth_remove_cmd(
1301+
key_name: str = typer.Argument(..., help="Secret name to remove, as previously passed to `buffdata auth set`"),
1302+
):
1303+
"""Remove a key previously stored via `buffdata auth set`."""
1304+
try:
1305+
import keyring
1306+
from keyring.errors import PasswordDeleteError
1307+
except ImportError:
1308+
console.print("[bold red]Install keyring (pip install keyring) to use `buffdata auth`.[/bold red]")
1309+
raise typer.Exit(1)
1310+
1311+
try:
1312+
keyring.delete_password(_keyring_service_name(), key_name)
1313+
console.print(f"[bold green]Removed {key_name} from the OS keyring.[/bold green]")
1314+
except PasswordDeleteError:
1315+
console.print(f"[yellow]{key_name} was not set in the OS keyring.[/yellow]")
1316+
1317+
1318+
@auth_app.command("status")
1319+
def auth_status_cmd():
1320+
"""Show which known provider secrets currently resolve, and from where (environment
1321+
variable vs. OS keyring) -- the values themselves are never displayed, here or
1322+
anywhere else in buffdata.
1323+
"""
1324+
table = Table(title="Secret resolution status", border_style="cyan")
1325+
table.add_column("Name", style="bold")
1326+
table.add_column("Resolves from")
1327+
for name in _KNOWN_SECRET_NAMES:
1328+
if os.getenv(name):
1329+
source = "[green]environment[/green]"
1330+
else:
1331+
found = None
1332+
try:
1333+
import keyring
1334+
1335+
found = keyring.get_password(_keyring_service_name(), name)
1336+
except Exception:
1337+
pass
1338+
source = "[cyan]OS keyring[/cyan]" if found else "[dim]not set[/dim]"
1339+
table.add_row(name, source)
1340+
console.print(table)

buffdata/engine/checkpoint.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from pathlib import Path
33
from typing import Dict, List, Optional, Set, Union
44
from buffdata.models.schemas import DatasetItem
5+
from buffdata.security.permissions import restrict_to_owner
56

67

78
class CheckpointManager:
@@ -36,6 +37,7 @@ def save_item(self, item: DatasetItem):
3637
data["_buffdata_id"] = str(item.id)
3738
with open(self.path, "a", encoding="utf-8") as f:
3839
f.write(json.dumps(data, ensure_ascii=False) + "\n")
40+
restrict_to_owner(self.path)
3941
self.processed_ids.add(str(item.id))
4042

4143
def load_all(self) -> List[DatasetItem]:

buffdata/engine/pipeline.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from buffdata.engine.validator import DatasetValidator
1717
from buffdata.models.formats import read_dataset, write_dataset_atomic
1818
from buffdata.observability import ObservabilityRegistry, disabled_registry
19+
from buffdata.security.permissions import restrict_to_owner
1920
from buffdata.models.schemas import (
2021
ClassificationMode,
2122
DatasetItem,
@@ -83,6 +84,10 @@ def _atomic_json(path: Path, data: dict[str, Any]) -> None:
8384
path.parent.mkdir(parents=True, exist_ok=True)
8485
temporary = path.with_name(f".{path.name}.tmp")
8586
temporary.write_bytes(orjson.dumps(data, option=orjson.OPT_INDENT_2))
87+
# Owner-only permissions before the rename, not after: os.replace preserves the
88+
# source inode's mode on POSIX, so this is the only chmod needed, and it means
89+
# the file is never briefly world-readable at its final path.
90+
restrict_to_owner(temporary)
8691
os.replace(temporary, path)
8792

8893
async def _save_checkpoint(

buffdata/engine/secrets.py

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
Every LLM client in engine/client.py resolves its API key through whatever
44
SecretResolver is configured, instead of calling os.getenv directly. The
55
default (EnvSecretResolver) makes this a no-op for every existing setup --
6-
nothing changes until BUFFDATA_SECRET_BACKEND is set to something else. This
7-
exists so an enterprise deployment can point BuffData at Vault, AWS Secrets
8-
Manager, GCP Secret Manager, or Azure Key Vault without any code changes above
9-
this layer -- only environment configuration.
6+
nothing changes until BUFFDATA_SECRET_BACKEND is set to something else, except
7+
that it now also checks the OS keyring (see KeyringSecretResolver) when an
8+
environment variable isn't set, so a value stored with `buffdata auth set`
9+
just works. Beyond the default, an enterprise deployment can point BuffData at
10+
Vault, AWS Secrets Manager, GCP Secret Manager, or Azure Key Vault without any
11+
code changes above this layer -- only environment configuration.
1012
"""
1113

1214
from __future__ import annotations
@@ -21,14 +23,54 @@ class SecretResolver(Protocol):
2123
def get(self, key: str) -> Optional[str]: ...
2224

2325

26+
class KeyringSecretResolver:
27+
"""Reads secrets from the OS-native credential store -- Windows Credential Manager,
28+
macOS Keychain, or Linux Secret Service/KWallet -- via the `keyring` package. The one
29+
backend that needs no server, no cloud account, and no infrastructure to already exist
30+
to be useful: exactly the gap for a `pip install`ed CLI running on someone's own
31+
machine, where Vault/AWS/GCP/Azure secret managers all assume infrastructure that
32+
plainly isn't there. Populated with `buffdata auth set <NAME>`, which prompts for the
33+
value with hidden input and never writes it to any file.
34+
"""
35+
36+
SERVICE_NAME = "buffdata"
37+
38+
def __init__(self, service_name: Optional[str] = None):
39+
self.service_name = service_name or os.getenv("BUFFDATA_KEYRING_SERVICE", self.SERVICE_NAME)
40+
41+
def get(self, key: str) -> Optional[str]:
42+
try:
43+
import keyring
44+
except ImportError:
45+
return None
46+
try:
47+
return keyring.get_password(self.service_name, key)
48+
except Exception:
49+
# A missing/misconfigured OS backend (headless Linux with no Secret Service,
50+
# a locked keychain, ...) must degrade to "not found" for this one lookup, not
51+
# crash every secret resolution on a machine that simply has none configured.
52+
return None
53+
54+
2455
class EnvSecretResolver:
2556
"""Default resolver: environment variables (including .env, already loaded via
26-
engine/client.py's load_dotenv()). Matches BuffData's behavior before any secret-backend
27-
integration existed -- this is what every client fell back to already.
57+
engine/client.py's load_dotenv()), falling back to the OS keyring
58+
(KeyringSecretResolver) for any key not found in the environment. This is the only
59+
resolver that falls back to anything -- every other backend below is explicit and
60+
deliberately does not -- because the fallback exists specifically to make the
61+
unconfigured default case work with zero setup: `buffdata auth set GEMINI_API_KEY`
62+
followed immediately by any command that needs it, no BUFFDATA_SECRET_BACKEND change
63+
required. An environment variable, when set, always wins over the keyring.
2864
"""
2965

66+
def __init__(self):
67+
self._keyring = KeyringSecretResolver()
68+
3069
def get(self, key: str) -> Optional[str]:
31-
return os.getenv(key)
70+
value = os.getenv(key)
71+
if value:
72+
return value
73+
return self._keyring.get(key)
3274

3375

3476
class VaultSecretResolver:
@@ -178,6 +220,7 @@ def get(self, key: str) -> Optional[str]:
178220

179221
_RESOLVERS = {
180222
"env": EnvSecretResolver,
223+
"keyring": KeyringSecretResolver,
181224
"vault": VaultSecretResolver,
182225
"aws_secrets_manager": AWSSecretsManagerResolver,
183226
"gcp_secret_manager": GCPSecretManagerResolver,

buffdata/governance/audit_store.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030

3131
from pydantic import BaseModel, Field
3232

33+
from buffdata.security.permissions import restrict_to_owner
34+
3335

3436
class AuditRecord(BaseModel):
3537
run_id: str = Field(default_factory=lambda: str(uuid.uuid4()))
@@ -78,6 +80,7 @@ class SQLiteAuditStore:
7880
def __init__(self, db_path: Union[str, Path] = "buffdata_audit.db"):
7981
self.db_path = Path(db_path)
8082
self._init_schema()
83+
restrict_to_owner(self.db_path)
8184

8285
def _connect(self) -> sqlite3.Connection:
8386
conn = sqlite3.connect(self.db_path)

buffdata/security/permissions.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
"""Restrictive file permissions for locally-written files that can carry sensitive
2+
content -- pipeline checkpoints (which can briefly hold pre-PII-redaction text, since
3+
the "pii" stage runs after "validate"'s own checkpoint is saved -- see
4+
buffdata/engine/pipeline.py's STAGES), report.json (record counts, provider/model,
5+
dataset-revealing file paths), and the audit database. Owner-only (0600) on POSIX,
6+
where "everyone else on this machine can read your files by default" is the real
7+
default; a best-effort no-op everywhere else, since os.chmod doesn't express meaningful
8+
ACLs on Windows and NTFS's own per-user-profile isolation already covers the common
9+
case there.
10+
"""
11+
12+
from __future__ import annotations
13+
14+
import os
15+
import stat
16+
from pathlib import Path
17+
from typing import Union
18+
19+
20+
def restrict_to_owner(path: Union[str, Path]) -> None:
21+
"""Set owner-read-write-only (0600) permissions on `path`. Silently does nothing if
22+
the path doesn't exist, or if the platform/filesystem doesn't support chmod (e.g. a
23+
FAT-formatted mount) -- this is defense in depth, not a guarantee the rest of the
24+
pipeline should ever depend on for correctness, so a failure here must never fail
25+
the write it's protecting.
26+
"""
27+
try:
28+
os.chmod(path, stat.S_IRUSR | stat.S_IWUSR)
29+
except OSError:
30+
pass

docs/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ Written for three readers:
2121
| [architecture.md](architecture.md) | The 7-stage pipeline, the provider-neutral client contract, data flow, where each guarantee (strict/local network policy, accuracy contracts) actually lives in the code |
2222
| [cli-reference.md](cli-reference.md) | Every command, grouped by what it's for, with real examples |
2323
| [configuration.md](configuration.md) | Every `PipelineConfig` field: type, default, what it controls, and the YAML equivalent |
24+
| [security.md](security.md) | API keys in the OS keyring instead of a plaintext file (`buffdata auth`), and owner-only file permissions on checkpoints/reports/the audit DB |
2425
| [providers.md](providers.md) | Cloud providers, local LLM servers (Ollama/LM Studio/vLLM/llama.cpp), secret backends (Vault/AWS/GCP/Azure), cloud dataset storage (S3/GCS/ADLS) |
2526
| [governance.md](governance.md) | Access policy (RBAC), OIDC/SSO bearer-token verification, the durable audit log, Data Contracts, SBOM generation |
2627
| [observability.md](observability.md) | OpenTelemetry tracing and Prometheus metrics per pipeline run |

docs/security.md

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
# Security
2+
3+
Two independent protections, both on by default with zero configuration: where API
4+
keys live, and who on the local machine can read the files BuffData writes.
5+
6+
## API keys: the OS keyring, not a plaintext file
7+
8+
```bash
9+
buffdata auth set GEMINI_API_KEY
10+
# Value for GEMINI_API_KEY: [hidden input]
11+
# Repeat for confirmation: [hidden input]
12+
# Stored GEMINI_API_KEY in the OS keyring.
13+
```
14+
15+
Stores the key in the platform-native credential store -- Windows Credential Manager,
16+
macOS Keychain, or Linux Secret Service/KWallet, via the `keyring` package (a base
17+
dependency, not an extra: this is the default story for every `pip install buffdata`
18+
user, not an enterprise add-on). The prompt uses hidden input with a confirmation
19+
step, and the value is never written to any file, never echoed to the terminal, and
20+
never lands in shell history the way `export GEMINI_API_KEY=sk-...` would.
21+
22+
Every command picks it up automatically afterward -- no `.env` file, no
23+
`BUFFDATA_SECRET_BACKEND` change. `EnvSecretResolver`, the default backend
24+
([`buffdata/engine/secrets.py`](../buffdata/engine/secrets.py)), checks the
25+
environment variable first (so CI/scripted use with real env vars is completely
26+
unaffected) and falls back to the OS keyring only when that's unset. It's the one
27+
backend that behaves this way; every other backend (`vault`,
28+
`aws_secrets_manager`, `gcp_secret_manager`, `azure_key_vault` -- see
29+
[providers.md](providers.md#secret-backends)) is explicit and doesn't fall back to
30+
anything, since those are deliberate infrastructure choices, not a smoothing default.
31+
32+
```bash
33+
buffdata auth status # which known secrets resolve, and from where -- values never shown
34+
buffdata auth remove GEMINI_API_KEY
35+
```
36+
37+
```
38+
Secret resolution status
39+
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
40+
┃ Name ┃ Resolves from ┃
41+
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
42+
│ GEMINI_API_KEY │ not set │
43+
│ GOOGLE_API_KEY │ not set │
44+
│ OPENAI_API_KEY │ environment │
45+
│ ANTHROPIC_API_KEY │ not set │
46+
│ AZURE_OPENAI_API_KEY │ not set │
47+
│ OPENAI_COMPATIBLE_API_KEY │ not set │
48+
│ OLLAMA_API_KEY │ not set │
49+
│ LMSTUDIO_API_KEY │ not set │
50+
│ VLLM_API_KEY │ not set │
51+
│ LLAMACPP_API_KEY │ not set │
52+
└───────────────────────────┴───────────────┘
53+
```
54+
55+
Set `BUFFDATA_KEYRING_SERVICE` to use a different keyring namespace than the default
56+
`buffdata` (multiple installs/profiles on one machine, for instance).
57+
58+
Verified for real in
59+
[`tests/test_auth_keyring.py`](../tests/test_auth_keyring.py) -- genuine
60+
`keyring.set_password`/`get_password`/`delete_password` round trips (via a real,
61+
file-backed `keyrings.alt` backend in the test environment, since no OS-native
62+
credential store is available in headless CI; the platform-native ones are what
63+
production actually uses), the full `buffdata auth set/remove/status` CLI flow, and an
64+
end-to-end test confirming a key stored via `auth set` is what `create_llm_client`
65+
actually resolves and uses -- no gap between "stored" and "used."
66+
67+
## Files at rest: owner-only permissions
68+
69+
[`buffdata/security/permissions.py`](../buffdata/security/permissions.py)'s
70+
`restrict_to_owner()` sets `0600` (owner read/write only) on every locally-written file
71+
that can carry sensitive content, immediately after writing it:
72+
73+
- **Pipeline checkpoints** (`.{output}.checkpoint.json`) -- can briefly hold
74+
pre-PII-redaction text, since the checkpoint after the `validate` stage is written
75+
before the `pii` stage runs (see [architecture.md](architecture.md)).
76+
- **`report.json`** -- record counts, provider/model, dataset-revealing file paths.
77+
- **The audit database** (`buffdata_audit.db` by default) -- every recorded run across
78+
a team, queryable by anyone who can read the file.
79+
80+
A no-op on Windows (`os.chmod` doesn't express meaningful ACLs there, and NTFS's
81+
per-user-profile isolation already covers the common single-user-machine case) and
82+
never fails the write it's protecting -- this is defense in depth for a shared POSIX
83+
machine, not something the rest of the pipeline depends on for correctness.
84+
85+
## What this doesn't cover
86+
87+
- **Encryption at rest** for these same files was deliberately not built: without a
88+
passphrase or key-management story for a `pip install`ed CLI to draw from, an
89+
encryption key would have to be hardcoded (pointless) or become yet another secret
90+
the user has to manage (worse than the file-permission protection it would replace).
91+
File permissions are the standard mitigation for this exact scenario --
92+
`~/.ssh/id_rsa` and `~/.aws/credentials` use the same approach, not encryption.
93+
- **The final output dataset** (`optimized.jsonl` and its `.rejected.jsonl`) is *not*
94+
permission-restricted -- it's the deliverable you explicitly asked BuffData to
95+
produce for downstream use (training, sharing, `buffdata push` to a Hub), and
96+
defaulting it to owner-only would just be friction for the common case of a build
97+
pipeline or teammate reading it under a different local user.
98+
- Redacting secrets from third-party SDK exception messages (if `openai`/`anthropic`/
99+
`google-genai` ever included a raw key in an error string) is outside BuffData's
100+
control -- checked during this work and none of buffdata's own code paths do this
101+
(no client's `__repr__`, exception message, or anything written to `report.json`/the
102+
audit DB ever includes `self.api_key`), but a third-party library's own error
103+
formatting isn't something this codebase can guarantee.

0 commit comments

Comments
 (0)