Skip to content

Commit 051691f

Browse files
committed
fix(mcp): resolve the auth token per request so rotation needs no restart
Backport of dba-agent 5d7ffc3 (mcp/ portion only). The MCP server is a long-lived stdio subprocess that snapshotted DEEPSQL_AUTH_TOKEN once at spawn into an immutable config. Where a provisioner rotates the token on disk without respawning the process, the captured token eventually expires, every tool call silently 401s, and re-provisioning is ignored by the dead process — a restart being the only remediation. The bearer token is now resolved per request from DEEPSQL_TOKEN_FILE, cached by mtime so disk is touched only when the file actually changes, with a single re-read-and-retry on 401 when the token has since rotated. 403 is never retried (RBAC denial, not stale creds), and installs with no token file keep using the env snapshot, so editor and CLI behaviour is unchanged. Scope note: the commit's hermes/deploy/provisioner.py and provision-profile.sh changes are deliberately NOT ported — this distribution does not ship the containerised agent. Nothing here writes DEEPSQL_TOKEN_FILE, and setup-agent.sh mints a token with no expiry (POST /auth/mcp-tokens with no expiresAt), so the rotation path is currently inert for self-host. It is ported for parity and for anyone running their own provisioner; the env-snapshot fallback means it costs nothing when unused. Also documents why agent.provisioner-url defaults to a deepsql-agent container that this stack does not run: provisioning is gated on agent.provision-secret, which is unset by default, so the unreachable URL is never contacted and setup-agent.sh writes the u-<user> profile locally instead. Behaviour unchanged; the default value is untouched. Tests: 259/259 (up from 252 — 7 ported cases covering token-file reads, the mtime cache, the 401 retry, 403 not retrying, and createConfigFromEnv wiring).
1 parent ddbea0b commit 051691f

3 files changed

Lines changed: 269 additions & 26 deletions

File tree

backend/src/main/java/com/dbaagent/service/AgentBridgeService.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,21 @@ public AgentBridgeService(McpTokenService mcpTokenService) {
6161
@Value("${agent.provision-enabled:true}")
6262
private boolean provisionEnabled;
6363

64-
/** The agent container's internal provisioning endpoint (compose network). */
64+
/**
65+
* The agent provisioner endpoint.
66+
*
67+
* <p>The default names a {@code deepsql-agent} container this distribution does
68+
* <em>not</em> ship: the self-host stack is four containers (postgres, valkey,
69+
* backend, frontend) and Hermes runs on the host via
70+
* {@code scripts/self-host/setup-agent.sh}. Nothing resolves that hostname here,
71+
* so the default is unreachable by design and kept only for deployments running
72+
* their own containerised provisioner.
73+
*
74+
* <p>That is harmless because provisioning is gated on {@code provisionSecret}
75+
* below: unset — the default — no request is ever sent to this URL, and
76+
* {@code setup-agent.sh} writes the {@code u-<user>} profile locally instead.
77+
* Set both values only if you run your own provisioner.
78+
*/
6579
@Value("${agent.provisioner-url:http://deepsql-agent:8788/provision}")
6680
private String provisionerUrl;
6781

mcp/deepsql-phase1-lib.js

Lines changed: 97 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
const fs = require("fs");
2+
13
const ALLOWED_READ_ONLY_KEYWORDS = new Set([
24
"SELECT",
35
"WITH",
@@ -1038,14 +1040,60 @@ function clampInteger(value, min, max, fallback) {
10381040
return Math.min(max, Math.max(min, parsed));
10391041
}
10401042

1043+
// Live bearer-token resolution. The MCP server is a long-lived stdio
1044+
// subprocess; inside the DeepSQL Agent container the provisioner rotates the
1045+
// token on disk (DEEPSQL_TOKEN_FILE) WITHOUT respawning us. Reading the token
1046+
// per request — cached by mtime so we only touch disk when the file actually
1047+
// changes — lets a rotated token take effect with no container restart. Editor
1048+
// and CLI installs set no token file and keep using the env snapshot
1049+
// (config.authToken), so their behaviour is unchanged.
1050+
let _tokenFileCache = null; // { path, mtimeMs, token }
1051+
1052+
function readTokenFile(tokenFile) {
1053+
const stat = fs.statSync(tokenFile);
1054+
if (
1055+
_tokenFileCache &&
1056+
_tokenFileCache.path === tokenFile &&
1057+
_tokenFileCache.mtimeMs === stat.mtimeMs
1058+
) {
1059+
return _tokenFileCache.token;
1060+
}
1061+
const token = fs.readFileSync(tokenFile, "utf8").trim();
1062+
_tokenFileCache = { path: tokenFile, mtimeMs: stat.mtimeMs, token };
1063+
return token;
1064+
}
1065+
1066+
function getAuthToken(config) {
1067+
if (config && config.tokenFile) {
1068+
try {
1069+
const token = readTokenFile(config.tokenFile);
1070+
if (token) {
1071+
return token;
1072+
}
1073+
} catch {
1074+
// Any stat/read error → fall back to the env token snapshot so we never
1075+
// hard-fail just because the file is momentarily missing mid-rewrite.
1076+
}
1077+
}
1078+
return (config && config.authToken) || "";
1079+
}
1080+
1081+
// Force the next getAuthToken() to re-read from disk regardless of mtime. Used
1082+
// by the 401 self-heal path, where the provisioner may have just rewritten the
1083+
// token file (same second → identical mtime granularity on some filesystems).
1084+
function invalidateTokenCache() {
1085+
_tokenFileCache = null;
1086+
}
1087+
10411088
function buildHeaders(config, extraHeaders = {}) {
10421089
const headers = {
10431090
Accept: "application/json",
10441091
...extraHeaders,
10451092
};
10461093

1047-
if (config.authToken) {
1048-
headers.Authorization = `Bearer ${config.authToken}`;
1094+
const authToken = getAuthToken(config);
1095+
if (authToken) {
1096+
headers.Authorization = `Bearer ${authToken}`;
10491097
}
10501098

10511099
// Origin-tracking headers so the backend audit row can distinguish
@@ -1070,13 +1118,13 @@ function resolveApiUrl(baseUrl, path) {
10701118
return new URL(normalizedPath, baseUrl).toString();
10711119
}
10721120

1073-
async function callDeepSqlApi(config, path, { method = "GET", json, headers } = {}) {
1121+
async function performFetch(config, path, { method = "GET", json, headers } = {}) {
10741122
const controller = new AbortController();
10751123
const timeout = setTimeout(() => controller.abort(), config.timeoutMs);
10761124
const url = resolveApiUrl(config.baseUrl, path);
10771125

10781126
try {
1079-
const response = await fetch(url, {
1127+
return await fetch(url, {
10801128
method,
10811129
headers: buildHeaders(
10821130
config,
@@ -1090,26 +1138,6 @@ async function callDeepSqlApi(config, path, { method = "GET", json, headers } =
10901138
body: json == null ? undefined : JSON.stringify(json),
10911139
signal: controller.signal,
10921140
});
1093-
1094-
const rawBody = await response.text();
1095-
let payload = null;
1096-
if (rawBody) {
1097-
try {
1098-
payload = JSON.parse(rawBody);
1099-
} catch {
1100-
payload = rawBody;
1101-
}
1102-
}
1103-
1104-
if (!response.ok) {
1105-
const message =
1106-
(payload && typeof payload === "object" && payload.message) ||
1107-
response.statusText ||
1108-
"DeepSQL API request failed";
1109-
throw new DeepSqlApiError(message, response.status, payload);
1110-
}
1111-
1112-
return payload;
11131141
} catch (error) {
11141142
if (error.name === "AbortError") {
11151143
throw new DeepSqlApiError(
@@ -1124,6 +1152,44 @@ async function callDeepSqlApi(config, path, { method = "GET", json, headers } =
11241152
}
11251153
}
11261154

1155+
async function callDeepSqlApi(config, path, options = {}) {
1156+
const tokenBefore = getAuthToken(config);
1157+
let response = await performFetch(config, path, options);
1158+
1159+
// 401 self-heal: our long-lived subprocess may be holding a token the agent
1160+
// provisioner has since rotated on disk. Re-read the token file (bypassing
1161+
// the mtime cache) and retry exactly once if it actually changed. We
1162+
// deliberately do NOT retry 403 — that's an RBAC denial, not stale creds —
1163+
// and only retry when a tokenFile is configured (editor/CLI installs aren't).
1164+
if (response.status === 401 && config && config.tokenFile) {
1165+
invalidateTokenCache();
1166+
const tokenAfter = getAuthToken(config);
1167+
if (tokenAfter && tokenAfter !== tokenBefore) {
1168+
response = await performFetch(config, path, options);
1169+
}
1170+
}
1171+
1172+
const rawBody = await response.text();
1173+
let payload = null;
1174+
if (rawBody) {
1175+
try {
1176+
payload = JSON.parse(rawBody);
1177+
} catch {
1178+
payload = rawBody;
1179+
}
1180+
}
1181+
1182+
if (!response.ok) {
1183+
const message =
1184+
(payload && typeof payload === "object" && payload.message) ||
1185+
response.statusText ||
1186+
"DeepSQL API request failed";
1187+
throw new DeepSqlApiError(message, response.status, payload);
1188+
}
1189+
1190+
return payload;
1191+
}
1192+
11271193
function summarizeConnections(connections) {
11281194
const lines = connections.map((connection) => {
11291195
const name = connection.connectionName || connection.name || connection.id;
@@ -1677,7 +1743,7 @@ const CONNECTIONS_CACHE_TTL_MS = 30000;
16771743
let _connectionsCache = null; // { key, ts, payload }
16781744

16791745
async function fetchConnectionsCached(config) {
1680-
const key = `${(config && config.baseUrl) || ""}|${(config && config.authToken) || ""}`;
1746+
const key = `${(config && config.baseUrl) || ""}|${getAuthToken(config)}`;
16811747
if (_connectionsCache && _connectionsCache.key === key
16821748
&& Date.now() - _connectionsCache.ts < CONNECTIONS_CACHE_TTL_MS) {
16831749
return _connectionsCache.payload;
@@ -2273,6 +2339,10 @@ function createConfigFromEnv(env = process.env) {
22732339
return {
22742340
baseUrl,
22752341
authToken: env.DEEPSQL_AUTH_TOKEN || "",
2342+
// Optional path to a file holding just the bearer token. When set (the
2343+
// agent-container case), getAuthToken reads it live per request so a
2344+
// provisioner-rotated token takes effect without respawning this process.
2345+
tokenFile: env.DEEPSQL_TOKEN_FILE || null,
22762346
timeoutMs: clampInteger(env.DEEPSQL_MCP_TIMEOUT_MS, 1000, 600000, 120000),
22772347
defaultUserId: env.DEEPSQL_MCP_USER_ID || "mcp-phase1",
22782348
defaultProjectId: env.DEEPSQL_MCP_PROJECT_ID || "mcp-phase1",
@@ -2296,7 +2366,9 @@ module.exports = {
22962366
containsForbiddenKeyword,
22972367
createConfigFromEnv,
22982368
firstKeyword,
2369+
getAuthToken,
22992370
handleToolCall,
2371+
invalidateTokenCache,
23002372
normalizeSqlForInspection,
23012373
resolveApiUrl,
23022374
splitStatements,

mcp/deepsql-phase1-lib.test.js

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
const test = require("node:test");
22
const assert = require("node:assert/strict");
3+
const fs = require("node:fs");
4+
const os = require("node:os");
5+
const path = require("node:path");
36

47
const {
58
resolveApiUrl,
@@ -10,8 +13,47 @@ const {
1013
validateReadOnlySql,
1114
TOOL_DEFINITIONS,
1215
handleToolCall,
16+
callDeepSqlApi,
17+
createConfigFromEnv,
18+
getAuthToken,
19+
invalidateTokenCache,
1320
} = require("./deepsql-phase1-lib");
1421

22+
function tmpTokenFile(contents) {
23+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "dsql-tok-"));
24+
const p = path.join(dir, "deepsql.token");
25+
fs.writeFileSync(p, contents);
26+
return p;
27+
}
28+
29+
// Install a global.fetch stub that records the Authorization header on each
30+
// request and returns queued responses. Returns { authHeaders, restore }.
31+
// Each response entry is { status, ok, body, onCall? } — onCall fires before
32+
// the response is returned, letting a test rotate the token file mid-flight.
33+
function installFetchStub(responses) {
34+
const authHeaders = [];
35+
const original = global.fetch;
36+
let i = 0;
37+
global.fetch = async (_url, init) => {
38+
authHeaders.push(init && init.headers ? init.headers.Authorization : undefined);
39+
const spec = responses[Math.min(i, responses.length - 1)];
40+
i += 1;
41+
if (spec && typeof spec.onCall === "function") {
42+
spec.onCall();
43+
}
44+
const status = spec?.status ?? 200;
45+
return {
46+
ok: spec?.ok ?? (status >= 200 && status < 300),
47+
status,
48+
statusText: spec?.statusText ?? "OK",
49+
async text() {
50+
return JSON.stringify(spec?.body ?? {});
51+
},
52+
};
53+
};
54+
return { authHeaders, restore: () => { global.fetch = original; } };
55+
}
56+
1557
test("resolveApiUrl keeps the /api prefix for absolute-looking tool paths", () => {
1658
const result = resolveApiUrl("http://localhost:8080/api/", "/connections");
1759
assert.equal(result, "http://localhost:8080/api/connections");
@@ -870,3 +912,118 @@ test("Phase A tools that require connectionId reject empty input cleanly (no net
870912
assert.equal(calls.length, 0, `${name} must not hit the network on validation failure`);
871913
}
872914
});
915+
916+
// ─── Live token resolution + 401 self-heal ─────────────────────────────────
917+
// The MCP server is a long-lived subprocess; in the agent container the token
918+
// is rotated on disk without respawning us. These tests cover getAuthToken's
919+
// mtime-cached live read, buildHeaders/callDeepSqlApi picking up a rotated
920+
// token, and the one-shot 401 self-heal retry.
921+
922+
test("createConfigFromEnv wires DEEPSQL_TOKEN_FILE and keeps the env token fallback", () => {
923+
const withFile = createConfigFromEnv({ DEEPSQL_TOKEN_FILE: "/x/y.token", DEEPSQL_AUTH_TOKEN: "z" });
924+
assert.equal(withFile.tokenFile, "/x/y.token");
925+
assert.equal(withFile.authToken, "z");
926+
const without = createConfigFromEnv({});
927+
assert.equal(without.tokenFile, null);
928+
});
929+
930+
test("getAuthToken caches by mtime and re-reads only when the file changes", () => {
931+
invalidateTokenCache();
932+
const p = tmpTokenFile("tok1\n");
933+
const cfg = { tokenFile: p, authToken: "envtok" };
934+
const realRead = fs.readFileSync;
935+
let reads = 0;
936+
fs.readFileSync = (...a) => { reads += 1; return realRead(...a); };
937+
try {
938+
assert.equal(getAuthToken(cfg), "tok1");
939+
assert.equal(getAuthToken(cfg), "tok1");
940+
assert.equal(reads, 1, "unchanged mtime → served from cache, no second read");
941+
const later = new Date(Date.now() + 10000);
942+
fs.writeFileSync(p, "tok2\n");
943+
fs.utimesSync(p, later, later);
944+
assert.equal(getAuthToken(cfg), "tok2");
945+
assert.equal(reads, 2, "mtime advanced → re-read from disk");
946+
} finally {
947+
fs.readFileSync = realRead;
948+
}
949+
});
950+
951+
test("getAuthToken falls back to the env token when no file / unreadable file", () => {
952+
invalidateTokenCache();
953+
assert.equal(getAuthToken({ authToken: "envtok" }), "envtok");
954+
assert.equal(getAuthToken({ tokenFile: "/no/such/deepsql.token", authToken: "envtok" }), "envtok");
955+
assert.equal(getAuthToken({ tokenFile: "/no/such/deepsql.token" }), "");
956+
});
957+
958+
test("callDeepSqlApi sends the live token and reflects a rotation mid-process", async () => {
959+
invalidateTokenCache();
960+
const p = tmpTokenFile("tokA\n");
961+
const cfg = { baseUrl: "http://test/api/", tokenFile: p, timeoutMs: 5000 };
962+
const stub = installFetchStub([{ body: {} }, { body: {} }]);
963+
try {
964+
await callDeepSqlApi(cfg, "/connections");
965+
const later = new Date(Date.now() + 10000);
966+
fs.writeFileSync(p, "tokB\n");
967+
fs.utimesSync(p, later, later);
968+
await callDeepSqlApi(cfg, "/connections");
969+
} finally {
970+
stub.restore();
971+
}
972+
assert.equal(stub.authHeaders[0], "Bearer tokA");
973+
assert.equal(stub.authHeaders[1], "Bearer tokB", "rotated token used without restart");
974+
});
975+
976+
test("callDeepSqlApi self-heals a 401 by re-reading a rotated token and retrying once", async () => {
977+
invalidateTokenCache();
978+
const p = tmpTokenFile("stale\n");
979+
const cfg = { baseUrl: "http://test/api/", tokenFile: p, timeoutMs: 5000 };
980+
const stub = installFetchStub([
981+
{
982+
status: 401,
983+
ok: false,
984+
statusText: "Unauthorized",
985+
body: { message: "unauthorized" },
986+
// Provisioner rotates the token concurrently with the failing call.
987+
onCall: () => {
988+
const later = new Date(Date.now() + 10000);
989+
fs.writeFileSync(p, "fresh\n");
990+
fs.utimesSync(p, later, later);
991+
},
992+
},
993+
{ body: { ok: true } },
994+
]);
995+
try {
996+
const result = await callDeepSqlApi(cfg, "/connections");
997+
assert.deepEqual(result, { ok: true });
998+
} finally {
999+
stub.restore();
1000+
}
1001+
assert.equal(stub.authHeaders.length, 2, "exactly one retry");
1002+
assert.equal(stub.authHeaders[0], "Bearer stale");
1003+
assert.equal(stub.authHeaders[1], "Bearer fresh");
1004+
});
1005+
1006+
test("callDeepSqlApi does not retry a 401 when no token file is configured", async () => {
1007+
invalidateTokenCache();
1008+
const cfg = { baseUrl: "http://test/api/", authToken: "envtok", timeoutMs: 5000 };
1009+
const stub = installFetchStub([{ status: 401, ok: false, statusText: "Unauthorized", body: { message: "nope" } }]);
1010+
try {
1011+
await assert.rejects(() => callDeepSqlApi(cfg, "/connections"), /nope/);
1012+
} finally {
1013+
stub.restore();
1014+
}
1015+
assert.equal(stub.authHeaders.length, 1, "env-only install must not retry");
1016+
});
1017+
1018+
test("callDeepSqlApi does not retry a 401 when the token file is unchanged", async () => {
1019+
invalidateTokenCache();
1020+
const p = tmpTokenFile("same\n");
1021+
const cfg = { baseUrl: "http://test/api/", tokenFile: p, timeoutMs: 5000 };
1022+
const stub = installFetchStub([{ status: 401, ok: false, statusText: "Unauthorized", body: { message: "nope" } }]);
1023+
try {
1024+
await assert.rejects(() => callDeepSqlApi(cfg, "/connections"), /nope/);
1025+
} finally {
1026+
stub.restore();
1027+
}
1028+
assert.equal(stub.authHeaders.length, 1, "unchanged token → no pointless retry");
1029+
});

0 commit comments

Comments
 (0)