Skip to content

Commit d6de799

Browse files
kevin1chunclaude
andcommitted
remove plaintext session fallback, keychain-only tokens (v0.6.1)
Remove ~/.robinhood-for-agents/session.json plaintext fallback from token-store. Tokens are now stored exclusively in the OS keychain via Bun.secrets. No tokens are written to disk. Update ClawHub skill metadata to declare Chrome dependency and credential storage mechanism. Rewrite domain files as client-API-first (no MCP references in main flow). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b891215 commit d6de799

File tree

6 files changed

+24
-104
lines changed

6 files changed

+24
-104
lines changed

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ await rh.restoreSession();
5959
```
6060
- All methods are `async` (native `fetch` under the hood)
6161
- Multi-account is first-class: every account-scoped method accepts `accountNumber`
62-
- Session cached in OS keychain via `Bun.secrets` (macOS Keychain Services); plaintext fallback for CI
62+
- Session cached in OS keychain via `Bun.secrets` (macOS Keychain Services) — no plaintext fallback, no tokens on disk
6363
- Token refresh via `refresh_token` + `device_token` when access token expires
6464
- Proper exceptions: `AuthenticationError`, `APIError`
6565
- **Do NOT use `phoenix.robinhood.com`** — it rejects TLS. Use `api.robinhood.com` endpoints only.

__tests__/client/auth.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { AuthenticationError } from "../../src/client/errors.js";
44
// Mock token-store
55
vi.mock("../../src/client/token-store.js", () => ({
66
loadTokens: vi.fn().mockResolvedValue(null),
7-
saveTokens: vi.fn().mockResolvedValue("/tmp/session.enc"),
7+
saveTokens: vi.fn().mockResolvedValue("keychain"),
88
deleteTokens: vi.fn().mockResolvedValue(undefined),
99
}));
1010

__tests__/client/token-store.test.ts

Lines changed: 10 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,8 @@
11
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
22

3-
// Mock node:fs before importing the module
4-
vi.mock("node:fs", () => ({
5-
existsSync: vi.fn(),
6-
mkdirSync: vi.fn(),
7-
writeFileSync: vi.fn(),
8-
renameSync: vi.fn(),
9-
unlinkSync: vi.fn(),
10-
}));
11-
12-
import { existsSync, mkdirSync, unlinkSync, writeFileSync } from "node:fs";
133
import type { Mock } from "vitest";
144
import { deleteTokens, loadTokens, saveTokens } from "../../src/client/token-store.js";
155

16-
const mockExistsSync = existsSync as Mock;
17-
const mockWriteFileSync = writeFileSync as Mock;
18-
const mockUnlinkSync = unlinkSync as Mock;
19-
206
const sampleTokens = {
217
access_token: "tok123",
228
refresh_token: "ref456",
@@ -52,7 +38,7 @@ describe("token-store", () => {
5238
vi.restoreAllMocks();
5339
});
5440

55-
describe("saveTokens (keychain available)", () => {
41+
describe("saveTokens", () => {
5642
it("stores tokens in Bun.secrets", async () => {
5743
const result = await saveTokens(sampleTokens);
5844

@@ -71,27 +57,9 @@ describe("token-store", () => {
7157
expect(parsed.saved_at).toBeTypeOf("number");
7258
});
7359

74-
it("does not write to filesystem when keychain available", async () => {
75-
await saveTokens(sampleTokens);
76-
expect(mockWriteFileSync).not.toHaveBeenCalled();
77-
});
78-
});
79-
80-
describe("saveTokens (keychain unavailable)", () => {
81-
it("falls back to plaintext JSON file", async () => {
82-
mockSecrets.get.mockRejectedValueOnce(new Error("unavailable"));
83-
const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
84-
85-
await saveTokens(sampleTokens);
86-
87-
expect(mkdirSync).toHaveBeenCalled();
88-
expect(mockWriteFileSync).toHaveBeenCalled();
89-
90-
const writtenData = mockWriteFileSync.mock.calls[0]?.[1];
91-
const parsed = JSON.parse(writtenData);
92-
expect(parsed.access_token).toBe("tok123");
93-
94-
expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining("Bun.secrets unavailable"));
60+
it("throws when Bun.secrets is unavailable", async () => {
61+
mockSecrets.set.mockRejectedValueOnce(new Error("unavailable"));
62+
await expect(saveTokens(sampleTokens)).rejects.toThrow("unavailable");
9563
});
9664
});
9765

@@ -106,44 +74,42 @@ describe("token-store", () => {
10674
});
10775

10876
it("returns null when no tokens stored", async () => {
109-
mockExistsSync.mockReturnValue(false);
11077
const result = await loadTokens();
11178
expect(result).toBeNull();
11279
});
11380

11481
it("returns null for invalid JSON in keychain", async () => {
11582
mockSecretsStore.set("robinhood-for-agents:session-tokens", "not json");
116-
mockExistsSync.mockReturnValue(false);
117-
11883
const result = await loadTokens();
11984
expect(result).toBeNull();
12085
});
12186

12287
it("returns null for JSON without access_token", async () => {
12388
mockSecretsStore.set("robinhood-for-agents:session-tokens", JSON.stringify({ foo: "bar" }));
124-
mockExistsSync.mockReturnValue(false);
89+
const result = await loadTokens();
90+
expect(result).toBeNull();
91+
});
12592

93+
it("returns null when Bun.secrets throws", async () => {
94+
mockSecrets.get.mockRejectedValueOnce(new Error("keychain locked"));
12695
const result = await loadTokens();
12796
expect(result).toBeNull();
12897
});
12998
});
13099

131100
describe("deleteTokens", () => {
132-
it("deletes from keychain and cleans up fallback file", async () => {
101+
it("deletes from keychain", async () => {
133102
mockSecretsStore.set("robinhood-for-agents:session-tokens", "data");
134-
mockExistsSync.mockReturnValue(true);
135103

136104
await deleteTokens();
137105

138106
expect(mockSecrets.delete).toHaveBeenCalledWith({
139107
service: "robinhood-for-agents",
140108
name: "session-tokens",
141109
});
142-
expect(mockUnlinkSync).toHaveBeenCalled();
143110
});
144111

145112
it("does not throw when nothing to delete", async () => {
146-
mockExistsSync.mockReturnValue(false);
147113
await expect(deleteTokens()).resolves.toBeUndefined();
148114
});
149115
});

skills/robinhood-for-agents/SKILL.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ install:
88
package: robinhood-for-agents
99
bins: [robinhood-for-agents]
1010
requires:
11-
bins: [bun]
11+
bins: [bun, google-chrome]
12+
metadata: {"credentials":"OAuth tokens stored in OS keychain via Bun.secrets (macOS Keychain Services, Linux libsecret, Windows Credential Manager). No tokens on disk. Browser login captures tokens via Playwright intercepting network traffic — no DOM interaction. Tokens expire ~24h.","chrome":"Required only for initial login (bunx robinhood-for-agents login). Not needed for subsequent API calls."}
1213
---
1314

1415
# robinhood-for-agents

skills/robinhood-for-agents/setup.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ Confirm to the user that authentication is complete.
3737
## Security Warning
3838
After successful login, **always** remind the user:
3939

40-
> **The session file at `~/.robinhood-for-agents/session.enc` contains encrypted Robinhood OAuth tokens. The encryption key is stored in the OS keychain (AES-256-GCM via node:crypto). Anyone with access to this machine can decrypt them. Tokens expire after ~24 hours. Never copy these files to untrusted locations.**
40+
> **Robinhood OAuth tokens are stored in the OS keychain (macOS Keychain Services, Linux libsecret, Windows Credential Manager) via Bun.secrets. No tokens are written to disk. Tokens expire after ~24 hours. Anyone with access to this machine's keychain can read them.**
4141
4242
## Notes
4343
- No credentials (username/password) pass through the tool layer — login happens on the real Robinhood website

src/client/token-store.ts

Lines changed: 9 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,12 @@
44
* Tokens are stored as JSON directly in the OS keychain
55
* (macOS Keychain Services, Linux libsecret, Windows Credential Manager).
66
*
7-
* Falls back to plaintext JSON at ~/.robinhood-for-agents/session.json
8-
* if Bun.secrets is unavailable (e.g. in CI or minimal environments).
7+
* Bun.secrets is required — no plaintext fallback.
98
*/
109

11-
import { existsSync, mkdirSync, renameSync, unlinkSync, writeFileSync } from "node:fs";
12-
import { homedir } from "node:os";
13-
import { join } from "node:path";
14-
1510
const KEYRING_SERVICE = "robinhood-for-agents";
1611
const KEYRING_NAME = "session-tokens";
1712

18-
const TOKEN_DIR = join(homedir(), ".robinhood-for-agents");
19-
const FALLBACK_FILE = join(TOKEN_DIR, "session.json");
20-
2113
export interface TokenData {
2214
access_token: string;
2315
refresh_token: string;
@@ -27,18 +19,6 @@ export interface TokenData {
2719
saved_at: number;
2820
}
2921

30-
async function secretsAvailable(): Promise<boolean> {
31-
try {
32-
if (typeof Bun !== "undefined" && Bun.secrets) {
33-
await Bun.secrets.get(KEYRING_SERVICE, KEYRING_NAME);
34-
return true;
35-
}
36-
return false;
37-
} catch {
38-
return false;
39-
}
40-
}
41-
4222
function isTokenData(data: unknown): data is TokenData {
4323
if (typeof data !== "object" || data === null) return false;
4424
const obj = data as Record<string, unknown>;
@@ -54,44 +34,21 @@ function isTokenData(data: unknown): data is TokenData {
5434
export async function saveTokens(tokens: Omit<TokenData, "saved_at">): Promise<string> {
5535
const data: TokenData = { ...tokens, saved_at: Date.now() / 1000 };
5636
const json = JSON.stringify(data);
57-
58-
if (await secretsAvailable()) {
59-
await Bun.secrets.set(KEYRING_SERVICE, KEYRING_NAME, json);
60-
return "keychain";
61-
}
62-
63-
// Fallback: plaintext JSON file (atomic write with correct permissions from creation)
64-
console.warn("[robinhood-for-agents] Bun.secrets unavailable — saving tokens as plaintext JSON");
65-
mkdirSync(TOKEN_DIR, { recursive: true, mode: 0o700 });
66-
const tmpFile = `${FALLBACK_FILE}.tmp`;
67-
writeFileSync(tmpFile, json, { mode: 0o600 });
68-
renameSync(tmpFile, FALLBACK_FILE);
69-
return FALLBACK_FILE;
37+
await Bun.secrets.set(KEYRING_SERVICE, KEYRING_NAME, json);
38+
return "keychain";
7039
}
7140

7241
export async function loadTokens(): Promise<TokenData | null> {
7342
try {
74-
if (await secretsAvailable()) {
75-
const json = await Bun.secrets.get(KEYRING_SERVICE, KEYRING_NAME);
76-
if (json) {
77-
const data: unknown = JSON.parse(json);
78-
if (isTokenData(data)) return data;
79-
}
43+
const json = await Bun.secrets.get(KEYRING_SERVICE, KEYRING_NAME);
44+
if (json) {
45+
const data: unknown = JSON.parse(json);
46+
if (isTokenData(data)) return data;
8047
}
8148
} catch {
82-
// Fall through to file fallback
83-
}
84-
85-
// Fallback: plaintext JSON file
86-
if (!existsSync(FALLBACK_FILE)) return null;
87-
try {
88-
const raw = Bun.file(FALLBACK_FILE);
89-
const data: unknown = await raw.json();
90-
if (isTokenData(data)) return data;
91-
return null;
92-
} catch {
93-
return null;
49+
// Bun.secrets unavailable or keychain access denied
9450
}
51+
return null;
9552
}
9653

9754
export async function deleteTokens(): Promise<void> {
@@ -100,8 +57,4 @@ export async function deleteTokens(): Promise<void> {
10057
} catch {
10158
// Bun.secrets unavailable
10259
}
103-
104-
if (existsSync(FALLBACK_FILE)) {
105-
unlinkSync(FALLBACK_FILE);
106-
}
10760
}

0 commit comments

Comments
 (0)