Skip to content

Commit 037881c

Browse files
authored
feat(desktop): add individual datasource tools (#875)
* feat(desktop): add individual datasource tools * chore: format datasource types and bump version * fix(desktop): secure datasource XML workflows * test(desktop): mock contained cache reads in workflows * fix(desktop): clarify datasource apply scope * test(desktop): lock datasource apply description * chore: bump version to 2.68.4 * fix(desktop): redact datasource password variants
1 parent 030fd30 commit 037881c

50 files changed

Lines changed: 3819 additions & 292 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "@tableau/mcp-server",
33
"description": "Helping agents see and understand data.",
4-
"version": "2.68.4",
4+
"version": "2.68.5",
55
"repository": {
66
"type": "git",
77
"url": "git+https://github.com/tableau/tableau-mcp.git"

src/desktop/cachePath.test.ts

Lines changed: 265 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,265 @@
1+
import {
2+
closeSync,
3+
constants,
4+
fstatSync,
5+
mkdirSync,
6+
mkdtempSync,
7+
openSync,
8+
readFileSync,
9+
realpathSync,
10+
rmSync,
11+
type Stats,
12+
statSync,
13+
symlinkSync,
14+
writeFileSync,
15+
} from 'fs';
16+
import { tmpdir } from 'os';
17+
import { join } from 'path';
18+
19+
import {
20+
type ContainedCacheReadOperations,
21+
getCacheDir,
22+
readContainedCacheTextFile,
23+
} from './cachePath.js';
24+
25+
describe('readContainedCacheTextFile', () => {
26+
const temporaryPaths: string[] = [];
27+
28+
afterEach(() => {
29+
for (const path of temporaryPaths.splice(0)) {
30+
rmSync(path, { recursive: true, force: true });
31+
}
32+
});
33+
34+
function cacheDirectory(label: string): string {
35+
const directory = mkdtempSync(join(getCacheDir(), `contained-cache-${label}-`));
36+
temporaryPaths.push(directory);
37+
return directory;
38+
}
39+
40+
function outsideDirectory(label: string): string {
41+
const directory = mkdtempSync(join(tmpdir(), `contained-cache-outside-${label}-`));
42+
temporaryPaths.push(directory);
43+
return directory;
44+
}
45+
46+
function defaultOperations(
47+
overrides: Partial<ContainedCacheReadOperations> = {},
48+
): ContainedCacheReadOperations {
49+
return {
50+
open: (path: string, flags: number) => openSync(path, flags),
51+
fstat: (fd: number) => fstatSync(fd),
52+
realpath: (path: string) => realpathSync(path),
53+
stat: (path: string) => statSync(path),
54+
read: (fd: number) => readFileSync(fd),
55+
close: (fd: number) => closeSync(fd),
56+
...overrides,
57+
} satisfies ContainedCacheReadOperations;
58+
}
59+
60+
it('rejects lexical sibling and traversal paths before opening them', () => {
61+
const open = vi.fn(() => {
62+
throw new Error('must not open');
63+
});
64+
const operations = defaultOperations({ open });
65+
const cacheDir = getCacheDir();
66+
67+
expect(readContainedCacheTextFile(`${cacheDir}-evil/file.xml`, operations)).toMatchObject({
68+
ok: false,
69+
issue: 'outside-cache',
70+
});
71+
expect(
72+
readContainedCacheTextFile(join(cacheDir, '..', 'escaped-datasource.xml'), operations),
73+
).toMatchObject({ ok: false, issue: 'outside-cache' });
74+
expect(open).not.toHaveBeenCalled();
75+
});
76+
77+
it('reads regular datasource and sidecar files inside the real cache root', () => {
78+
const directory = cacheDirectory('valid');
79+
const datasourceFile = join(directory, 'datasource.xml');
80+
const sidecarFile = `${datasourceFile}.meta.json`;
81+
writeFileSync(datasourceFile, '<datasource name="sales"/>');
82+
writeFileSync(sidecarFile, '{"instanceId":"safe"}');
83+
84+
expect(readContainedCacheTextFile(datasourceFile)).toEqual({
85+
ok: true,
86+
path: datasourceFile,
87+
text: '<datasource name="sales"/>',
88+
});
89+
expect(readContainedCacheTextFile(sidecarFile)).toEqual({
90+
ok: true,
91+
path: sidecarFile,
92+
text: '{"instanceId":"safe"}',
93+
});
94+
});
95+
96+
it('opens read-only with no-follow semantics when the platform exposes them', () => {
97+
const directory = cacheDirectory('open-flags');
98+
const file = join(directory, 'datasource.xml');
99+
writeFileSync(file, '<datasource/>');
100+
const open = vi.fn((path: string, flags: number) => openSync(path, flags));
101+
102+
expect(readContainedCacheTextFile(file, defaultOperations({ open })).ok).toBe(true);
103+
const flags = open.mock.calls[0]?.[1] ?? 0;
104+
const noFollow = typeof constants.O_NOFOLLOW === 'number' ? constants.O_NOFOLLOW : 0;
105+
expect(flags).toBe(constants.O_RDONLY | noFollow);
106+
});
107+
108+
it('rejects a final-component symlink that escapes the cache', () => {
109+
const directory = cacheDirectory('final-symlink');
110+
const outside = outsideDirectory('final-symlink');
111+
const outsideFile = join(outside, 'secret.xml');
112+
const candidate = join(directory, 'datasource.xml');
113+
writeFileSync(outsideFile, '<outside-secret/>');
114+
symlinkSync(outsideFile, candidate);
115+
116+
expect(readContainedCacheTextFile(candidate)).toMatchObject({
117+
ok: false,
118+
issue: 'unsafe-file',
119+
});
120+
});
121+
122+
it('rejects an intermediate-directory symlink that escapes the cache', () => {
123+
const directory = cacheDirectory('intermediate-symlink');
124+
const outside = outsideDirectory('intermediate-symlink');
125+
const outsideFile = join(outside, 'datasource.xml');
126+
writeFileSync(outsideFile, '<outside-secret/>');
127+
symlinkSync(outside, join(directory, 'linked'));
128+
129+
expect(readContainedCacheTextFile(join(directory, 'linked', 'datasource.xml'))).toMatchObject({
130+
ok: false,
131+
issue: 'unsafe-file',
132+
});
133+
});
134+
135+
it.each(['final', 'intermediate'] as const)(
136+
'rejects a %s sidecar symlink escape without reading external contents',
137+
(linkKind) => {
138+
const directory = cacheDirectory(`sidecar-${linkKind}`);
139+
const outside = outsideDirectory(`sidecar-${linkKind}`);
140+
const outsideSidecar = join(outside, 'datasource.xml.meta.json');
141+
writeFileSync(outsideSidecar, '{"source_sha256":"external-secret"}');
142+
const read = vi.fn((fd: number) => readFileSync(fd));
143+
const operations = defaultOperations({ read });
144+
let candidate: string;
145+
if (linkKind === 'final') {
146+
candidate = join(directory, 'datasource.xml.meta.json');
147+
symlinkSync(outsideSidecar, candidate);
148+
} else {
149+
symlinkSync(outside, join(directory, 'linked'));
150+
candidate = join(directory, 'linked', 'datasource.xml.meta.json');
151+
}
152+
153+
expect(readContainedCacheTextFile(candidate, operations)).toMatchObject({
154+
ok: false,
155+
issue: 'unsafe-file',
156+
});
157+
expect(read).not.toHaveBeenCalled();
158+
},
159+
);
160+
161+
it('rejects an opened/current identity mismatch without reading and closes the descriptor', () => {
162+
const directory = cacheDirectory('identity-mismatch');
163+
const file = join(directory, 'datasource.xml');
164+
writeFileSync(file, '<datasource/>');
165+
const read = vi.fn((fd: number) => readFileSync(fd));
166+
const close = vi.fn((fd: number) => closeSync(fd));
167+
const operations = defaultOperations({
168+
stat: (path) => withInode(statSync(path), statSync(path).ino + 1),
169+
read,
170+
close,
171+
});
172+
173+
expect(readContainedCacheTextFile(file, operations)).toMatchObject({
174+
ok: false,
175+
issue: 'unsafe-file',
176+
});
177+
expect(read).not.toHaveBeenCalled();
178+
expect(close).toHaveBeenCalledTimes(1);
179+
});
180+
181+
it('rejects a candidate whose real path changes across the current-file stat', () => {
182+
const directory = cacheDirectory('realpath-change');
183+
const file = join(directory, 'datasource.xml');
184+
const replacement = join(directory, 'replacement.xml');
185+
writeFileSync(file, '<datasource/>');
186+
writeFileSync(replacement, '<datasource replacement="true"/>');
187+
const read = vi.fn((fd: number) => readFileSync(fd));
188+
const close = vi.fn((fd: number) => closeSync(fd));
189+
let candidateRealpathCalls = 0;
190+
const operations = defaultOperations({
191+
realpath: (path) => {
192+
if (path === getCacheDir()) return realpathSync(path);
193+
candidateRealpathCalls += 1;
194+
return candidateRealpathCalls === 1 ? realpathSync(file) : realpathSync(replacement);
195+
},
196+
read,
197+
close,
198+
});
199+
200+
expect(readContainedCacheTextFile(file, operations)).toMatchObject({
201+
ok: false,
202+
issue: 'unsafe-file',
203+
});
204+
expect(read).not.toHaveBeenCalled();
205+
expect(close).toHaveBeenCalledTimes(1);
206+
});
207+
208+
it('closes the descriptor after both a successful read and a read failure', () => {
209+
const directory = cacheDirectory('close');
210+
const file = join(directory, 'datasource.xml');
211+
writeFileSync(file, '<datasource/>');
212+
const closeAfterSuccess = vi.fn((fd: number) => closeSync(fd));
213+
214+
expect(
215+
readContainedCacheTextFile(file, defaultOperations({ close: closeAfterSuccess })).ok,
216+
).toBe(true);
217+
expect(closeAfterSuccess).toHaveBeenCalledTimes(1);
218+
219+
const closeAfterFailure = vi.fn((fd: number) => closeSync(fd));
220+
const failure = readContainedCacheTextFile(
221+
file,
222+
defaultOperations({
223+
read: () => {
224+
throw new Error('read failed');
225+
},
226+
close: closeAfterFailure,
227+
}),
228+
);
229+
expect(failure).toMatchObject({ ok: false, issue: 'read-error' });
230+
expect(closeAfterFailure).toHaveBeenCalledTimes(1);
231+
});
232+
233+
it('classifies a missing file separately from unreadable and unsafe files', () => {
234+
const directory = cacheDirectory('classify');
235+
const missing = join(directory, 'missing.xml');
236+
const unreadable = join(directory, 'unreadable.xml');
237+
const nonFile = join(directory, 'directory.xml');
238+
writeFileSync(unreadable, '<datasource/>');
239+
mkdirSync(nonFile);
240+
241+
expect(readContainedCacheTextFile(missing)).toMatchObject({ ok: false, issue: 'missing' });
242+
expect(
243+
readContainedCacheTextFile(
244+
unreadable,
245+
defaultOperations({
246+
open: () => {
247+
const error = new Error('permission denied') as NodeJS.ErrnoException;
248+
error.code = 'EACCES';
249+
throw error;
250+
},
251+
}),
252+
),
253+
).toMatchObject({ ok: false, issue: 'read-error' });
254+
expect(readContainedCacheTextFile(nonFile)).toMatchObject({
255+
ok: false,
256+
issue: 'unsafe-file',
257+
});
258+
});
259+
});
260+
261+
function withInode(stats: Stats, ino: number): Stats {
262+
const copy = Object.assign(Object.create(Object.getPrototypeOf(stats)), stats) as Stats;
263+
Object.defineProperty(copy, 'ino', { value: ino, configurable: true });
264+
return copy;
265+
}

0 commit comments

Comments
 (0)