Skip to content

Commit 333c067

Browse files
committed
test(skills): add path resolution tests for symlink/junction scenarios
Test that SkillProvider and SkillsDirectoryProvider correctly handle template resource reads when the skill path goes through a symlink (POSIX) or NTFS junction (Windows). This catches the bug where skill_path_ stored as lexically_normal() diverged from weakly_canonical() in the template provider's is_within() check. Tests: - [link] Template reads through symlink/junction path - [link-dir] Directory provider through symlink/junction - [canonical-temp] Temp path with canonical differences - [escape] Path traversal rejection (../secret.txt) - [resources-mode] Resources mode through non-canonical path Key implementation note: uses require() instead of assert() to avoid side-effect-in-assert bugs in Release builds (NDEBUG strips assert).
1 parent ae6f05f commit 333c067

3 files changed

Lines changed: 354 additions & 1 deletion

File tree

CMakeLists.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,10 @@ if(FASTMCPP_BUILD_TESTS)
513513
target_link_libraries(fastmcpp_provider_skills PRIVATE fastmcpp_core)
514514
add_test(NAME fastmcpp_provider_skills COMMAND fastmcpp_provider_skills)
515515

516+
add_executable(fastmcpp_provider_skills_paths tests/providers/skills_path_resolution.cpp)
517+
target_link_libraries(fastmcpp_provider_skills_paths PRIVATE fastmcpp_core)
518+
add_test(NAME fastmcpp_provider_skills_paths COMMAND fastmcpp_provider_skills_paths)
519+
516520
add_executable(fastmcpp_provider_openapi tests/providers/openapi_provider.cpp)
517521
target_link_libraries(fastmcpp_provider_openapi PRIVATE fastmcpp_core)
518522
add_test(NAME fastmcpp_provider_openapi COMMAND fastmcpp_provider_openapi)

src/providers/skills_provider.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ std::filesystem::path home_dir()
241241

242242
SkillProvider::SkillProvider(std::filesystem::path skill_path, std::string main_file_name,
243243
SkillSupportingFiles supporting_files)
244-
: skill_path_(std::filesystem::weakly_canonical(std::filesystem::absolute(std::move(skill_path)))),
244+
: skill_path_(std::filesystem::weakly_canonical(std::filesystem::absolute(skill_path))),
245245
skill_name_(skill_path_.filename().string()), main_file_name_(std::move(main_file_name)),
246246
supporting_files_(supporting_files)
247247
{
Lines changed: 349 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,349 @@
1+
#include "fastmcpp/app.hpp"
2+
#include "fastmcpp/exceptions.hpp"
3+
#include "fastmcpp/providers/skills_provider.hpp"
4+
5+
#include <cstdlib>
6+
#include <filesystem>
7+
#include <fstream>
8+
#include <iostream>
9+
#include <string>
10+
#include <vector>
11+
12+
#ifdef _WIN32
13+
#include <windows.h>
14+
#endif
15+
16+
using namespace fastmcpp;
17+
18+
namespace
19+
{
20+
std::filesystem::path make_temp_dir(const std::string& name)
21+
{
22+
auto base = std::filesystem::temp_directory_path() / ("fastmcpp_skills_path_" + name);
23+
std::error_code ec;
24+
std::filesystem::remove_all(base, ec);
25+
std::filesystem::create_directories(base);
26+
return base;
27+
}
28+
29+
void write_text(const std::filesystem::path& path, const std::string& text)
30+
{
31+
std::filesystem::create_directories(path.parent_path());
32+
std::ofstream out(path, std::ios::binary | std::ios::trunc);
33+
out << text;
34+
}
35+
36+
std::string read_text_data(const resources::ResourceContent& content)
37+
{
38+
if (auto* text = std::get_if<std::string>(&content.data))
39+
return *text;
40+
return {};
41+
}
42+
43+
// Create a directory-level indirection (symlink or junction) from link_path
44+
// to target_path. Returns true on success. On Windows, tries symlink first
45+
// (requires developer mode/admin), then falls back to junctions (no admin).
46+
// On POSIX, uses symlinks.
47+
bool create_dir_link(const std::filesystem::path& target, const std::filesystem::path& link_path)
48+
{
49+
std::error_code ec;
50+
std::filesystem::create_directory_symlink(target, link_path, ec);
51+
if (!ec)
52+
return true;
53+
54+
#ifdef _WIN32
55+
// Fall back to NTFS junction (works without admin privileges).
56+
std::string cmd = "cmd /c mklink /J \"" + link_path.string() + "\" \"" + target.string() + "\"";
57+
cmd += " >NUL 2>&1";
58+
return std::system(cmd.c_str()) == 0;
59+
#else
60+
return false;
61+
#endif
62+
}
63+
64+
// Remove a directory link (symlink or junction) and all contents.
65+
void remove_dir_link(const std::filesystem::path& link_path)
66+
{
67+
std::error_code ec;
68+
#ifdef _WIN32
69+
// Junctions are removed with RemoveDirectoryW, not remove().
70+
RemoveDirectoryW(link_path.wstring().c_str());
71+
#endif
72+
std::filesystem::remove(link_path, ec);
73+
// Fall back to remove_all in case a regular directory was left behind.
74+
std::filesystem::remove_all(link_path, ec);
75+
}
76+
77+
// Check whether creating directory links works on this platform and
78+
// whether weakly_canonical resolves through them (which is the actual
79+
// condition that triggers the bug).
80+
bool links_change_canonical()
81+
{
82+
auto test_dir = std::filesystem::temp_directory_path() / "fastmcpp_canon_probe_real";
83+
auto test_link = std::filesystem::temp_directory_path() / "fastmcpp_canon_probe_link";
84+
std::error_code ec;
85+
std::filesystem::remove_all(test_dir, ec);
86+
remove_dir_link(test_link);
87+
std::filesystem::create_directories(test_dir);
88+
if (!create_dir_link(test_dir, test_link))
89+
{
90+
std::filesystem::remove_all(test_dir, ec);
91+
return false;
92+
}
93+
94+
// Write a file through the link and check canonical form.
95+
write_text(test_link / "probe.txt", "x");
96+
auto via_link = std::filesystem::absolute(test_link / "probe.txt").lexically_normal();
97+
auto canonical = std::filesystem::weakly_canonical(test_link / "probe.txt");
98+
bool differs = via_link != canonical;
99+
100+
remove_dir_link(test_link);
101+
std::filesystem::remove_all(test_dir, ec);
102+
return differs;
103+
}
104+
105+
void require(bool condition, const std::string& message)
106+
{
107+
if (!condition)
108+
{
109+
std::cerr << "FAIL: " << message << std::endl;
110+
std::abort();
111+
}
112+
}
113+
} // namespace
114+
115+
int main()
116+
{
117+
// ---------------------------------------------------------------
118+
// Test 1: Template resource read through a linked path.
119+
//
120+
// This is the scenario that failed on macOS CI (/tmp -> /private/tmp)
121+
// and Windows CI (8.3 short names). The SkillProvider must resolve
122+
// the skill_path_ to its canonical form so that is_within() works
123+
// when the template provider uses weakly_canonical() on child paths.
124+
//
125+
// Uses symlinks on POSIX, junctions on Windows (no admin needed).
126+
// ---------------------------------------------------------------
127+
if (links_change_canonical())
128+
{
129+
std::cerr << " [link] Running linked-path resolution tests\n";
130+
131+
const auto real_dir = make_temp_dir("link_real");
132+
const auto link_dir = real_dir.parent_path() / "fastmcpp_skills_path_link";
133+
remove_dir_link(link_dir);
134+
bool link_ok = create_dir_link(real_dir, link_dir);
135+
require(link_ok, "Failed to create directory link");
136+
137+
const auto skill = link_dir / "my-skill";
138+
write_text(skill / "SKILL.md", "# Linked Skill\nContent here.");
139+
write_text(skill / "data" / "info.txt", "linked-data");
140+
write_text(skill / "nested" / "deep" / "file.md", "deep-content");
141+
142+
// Verify the link is actually an indirection (not a regular directory).
143+
auto child_via_link = std::filesystem::absolute(link_dir / "my-skill" / "data" / "info.txt")
144+
.lexically_normal();
145+
auto child_canonical =
146+
std::filesystem::weakly_canonical(link_dir / "my-skill" / "data" / "info.txt");
147+
require(child_via_link != child_canonical,
148+
"Link did not create path indirection: " + child_via_link.string() +
149+
" == " + child_canonical.string());
150+
151+
// Construct provider using the link path (not the real path).
152+
auto provider = std::make_shared<providers::SkillProvider>(
153+
skill, "SKILL.md", providers::SkillSupportingFiles::Template);
154+
FastMCP app("link_test", "1.0.0");
155+
app.add_provider(provider);
156+
157+
// Main file should be readable.
158+
auto main_content = app.read_resource("skill://my-skill/SKILL.md");
159+
require(read_text_data(main_content).find("Linked Skill") != std::string::npos,
160+
"Main file content mismatch through link");
161+
162+
// Template-based reads through the linked root must work.
163+
// This is the exact scenario that failed with "Skill path escapes root".
164+
auto info = app.read_resource("skill://my-skill/data/info.txt");
165+
require(read_text_data(info) == "linked-data",
166+
"Template resource read failed through link");
167+
168+
auto deep = app.read_resource("skill://my-skill/nested/deep/file.md");
169+
require(read_text_data(deep) == "deep-content",
170+
"Nested template resource read failed through link");
171+
172+
// Manifest should list all files.
173+
auto manifest_content = app.read_resource("skill://my-skill/_manifest");
174+
const std::string manifest_text = read_text_data(manifest_content);
175+
require(manifest_text.find("data/info.txt") != std::string::npos,
176+
"Manifest missing data/info.txt");
177+
require(manifest_text.find("nested/deep/file.md") != std::string::npos,
178+
"Manifest missing nested/deep/file.md");
179+
180+
std::cerr << " [link] PASSED\n";
181+
182+
// ---------------------------------------------------------------
183+
// Test 2: SkillsDirectoryProvider through a linked root.
184+
//
185+
// Same scenario but with the directory-level provider that
186+
// discovers skills by scanning subdirectories.
187+
// ---------------------------------------------------------------
188+
std::cerr << " [link-dir] Running linked directory provider tests\n";
189+
190+
const auto dir_real = make_temp_dir("linkdir_real");
191+
const auto dir_link = dir_real.parent_path() / "fastmcpp_skills_path_linkdir";
192+
remove_dir_link(dir_link);
193+
link_ok = create_dir_link(dir_real, dir_link);
194+
require(link_ok, "Failed to create directory link for dir provider");
195+
196+
write_text(dir_link / "tool-a" / "SKILL.md", "# Tool A\nFirst tool.");
197+
write_text(dir_link / "tool-a" / "extra.txt", "extra-a");
198+
199+
auto dir_provider = std::make_shared<providers::SkillsDirectoryProvider>(
200+
dir_link, false, "SKILL.md", providers::SkillSupportingFiles::Template);
201+
FastMCP app_dir("link_dir_test", "1.0.0");
202+
app_dir.add_provider(dir_provider);
203+
204+
auto tool_main = app_dir.read_resource("skill://tool-a/SKILL.md");
205+
require(read_text_data(tool_main).find("Tool A") != std::string::npos,
206+
"Dir provider main file read failed through link");
207+
208+
auto extra = app_dir.read_resource("skill://tool-a/extra.txt");
209+
require(read_text_data(extra) == "extra-a",
210+
"Dir provider template resource read failed through link");
211+
212+
std::cerr << " [link-dir] PASSED\n";
213+
214+
// Cleanup.
215+
remove_dir_link(link_dir);
216+
remove_dir_link(dir_link);
217+
std::error_code ec;
218+
std::filesystem::remove_all(real_dir, ec);
219+
std::filesystem::remove_all(dir_real, ec);
220+
}
221+
else
222+
{
223+
std::cerr << " [link] SKIPPED (cannot create dir links or canonical path unchanged)\n";
224+
}
225+
226+
// ---------------------------------------------------------------
227+
// Test 3: Canonical temp path.
228+
//
229+
// Even without an explicit link, temp_directory_path() may differ
230+
// from weakly_canonical(temp_directory_path()) -- e.g. macOS /tmp
231+
// vs /private/tmp, or Windows trailing slash. Use the raw
232+
// (non-canonical) temp path to exercise the provider.
233+
// ---------------------------------------------------------------
234+
{
235+
std::cerr << " [canonical-temp] Running canonical temp path tests\n";
236+
237+
const auto raw_tmp = std::filesystem::temp_directory_path();
238+
const auto root = raw_tmp / "fastmcpp_skills_path_canonical";
239+
std::error_code ec;
240+
std::filesystem::remove_all(root, ec);
241+
const auto skill = root / "canon-skill";
242+
write_text(skill / "SKILL.md", "# Canon\nCanonical test.");
243+
write_text(skill / "sub" / "data.txt", "canon-data");
244+
245+
auto provider = std::make_shared<providers::SkillProvider>(
246+
skill, "SKILL.md", providers::SkillSupportingFiles::Template);
247+
FastMCP app("canonical_test", "1.0.0");
248+
app.add_provider(provider);
249+
250+
auto main_content = app.read_resource("skill://canon-skill/SKILL.md");
251+
require(read_text_data(main_content).find("Canon") != std::string::npos,
252+
"Canonical temp: main file content mismatch");
253+
254+
auto sub = app.read_resource("skill://canon-skill/sub/data.txt");
255+
require(read_text_data(sub) == "canon-data",
256+
"Canonical temp: template resource read failed");
257+
258+
std::cerr << " [canonical-temp] PASSED\n";
259+
260+
std::filesystem::remove_all(root, ec);
261+
}
262+
263+
// ---------------------------------------------------------------
264+
// Test 4: Path escape attempts must be rejected.
265+
//
266+
// Verify that the is_within security check blocks traversal
267+
// regardless of canonical vs non-canonical path representation.
268+
// ---------------------------------------------------------------
269+
{
270+
std::cerr << " [escape] Running path escape security tests\n";
271+
272+
const auto root = make_temp_dir("escape");
273+
const auto skill = root / "safe-skill";
274+
write_text(skill / "SKILL.md", "# Safe\nInside root.");
275+
276+
// Create a file outside the skill directory to verify it can't be read.
277+
write_text(root / "secret.txt", "should-not-be-readable");
278+
279+
auto provider = std::make_shared<providers::SkillProvider>(
280+
skill, "SKILL.md", providers::SkillSupportingFiles::Template);
281+
FastMCP app("escape_test", "1.0.0");
282+
app.add_provider(provider);
283+
284+
bool caught_escape = false;
285+
try
286+
{
287+
app.read_resource("skill://safe-skill/../secret.txt");
288+
}
289+
catch (const std::exception& e)
290+
{
291+
const std::string msg = e.what();
292+
caught_escape = msg.find("escapes root") != std::string::npos ||
293+
msg.find("not found") != std::string::npos;
294+
}
295+
require(caught_escape, "Path escape was not rejected");
296+
297+
std::cerr << " [escape] PASSED\n";
298+
299+
std::error_code ec;
300+
std::filesystem::remove_all(root, ec);
301+
}
302+
303+
// ---------------------------------------------------------------
304+
// Test 5: Resources mode through non-canonical path.
305+
//
306+
// In Resources mode, supporting files are enumerated as explicit
307+
// resources (not via template matching). Verify this also works
308+
// when the skill path requires canonicalization.
309+
// ---------------------------------------------------------------
310+
{
311+
std::cerr << " [resources-mode] Running resources mode path tests\n";
312+
313+
const auto raw_tmp = std::filesystem::temp_directory_path();
314+
const auto root = raw_tmp / "fastmcpp_skills_path_resmode";
315+
std::error_code ec;
316+
std::filesystem::remove_all(root, ec);
317+
const auto skill = root / "res-skill";
318+
write_text(skill / "SKILL.md", "# Resources\nResources mode.");
319+
write_text(skill / "assets" / "data.json", "{\"key\":\"value\"}");
320+
321+
auto provider = std::make_shared<providers::SkillProvider>(
322+
skill, "SKILL.md", providers::SkillSupportingFiles::Resources);
323+
FastMCP app("resources_mode_test", "1.0.0");
324+
app.add_provider(provider);
325+
326+
auto resources = app.list_all_resources();
327+
bool found_asset = false;
328+
for (const auto& res : resources)
329+
{
330+
if (res.uri == "skill://res-skill/assets/data.json")
331+
{
332+
found_asset = true;
333+
break;
334+
}
335+
}
336+
require(found_asset, "Resources mode: asset not found in resource list");
337+
338+
auto asset = app.read_resource("skill://res-skill/assets/data.json");
339+
require(read_text_data(asset).find("\"key\"") != std::string::npos,
340+
"Resources mode: asset content mismatch");
341+
342+
std::cerr << " [resources-mode] PASSED\n";
343+
344+
std::filesystem::remove_all(root, ec);
345+
}
346+
347+
std::cerr << "All skills path resolution tests passed.\n";
348+
return 0;
349+
}

0 commit comments

Comments
 (0)