From 8bbe6b33dd79d1e785ed167fc0d63381e8fe4aac Mon Sep 17 00:00:00 2001 From: jichao wang Date: Sat, 20 Jun 2026 03:56:49 +0100 Subject: [PATCH 1/2] fix(skill): resolve references/ wikilinks whose stem contains a dot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `validate_skill` appended the implicit `.md` to a `[[references/...]]` link only when `Path(link).suffix` was empty. `Path.suffix` returns everything after the last dot, so a reference whose stem contains a dot — e.g. `[[references/api.v2]]` or `[[references/v1.2-guide]]` — has a truthy suffix (`.v2` / `.2-guide`). The `.md` was therefore never appended and the validator looked for an extension-less `references/api.v2`, emitting a false "doesn't exist" error even though `references/api.v2.md` is present and correctly linked. (`WIKILINK_RE` allows `.` in the target, so such links are legal and reach this branch.) Append `.md` based on a literal `.md` check instead of `Path.suffix`, so a dotted stem is suffixed correctly while an explicit `...md` link is left alone. Genuinely missing references still error. Adds a regression test for the dotted-stem case (the existing `test_wikilink_without_md_suffix_resolves` only covered a dot-free stem). --- openkb/skill/validator.py | 9 +++++---- tests/test_skill_validator.py | 13 +++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/openkb/skill/validator.py b/openkb/skill/validator.py index f40014d6a..b70444ae0 100644 --- a/openkb/skill/validator.py +++ b/openkb/skill/validator.py @@ -207,10 +207,11 @@ def validate_skill(skill_dir: Path, *, strict: bool = False) -> ValidationResult # references/ wikilink resolution wikilinks = WIKILINK_RE.findall(text) for link in wikilinks: - # link may or may not include .md suffix - target = refs_dir / link - if not target.suffix: - target = target.with_suffix(".md") + # The link may already include the .md suffix; append it otherwise. + # Test the literal ".md" rather than Path.suffix — a dotted stem like + # "api.v2" has a truthy suffix (".v2"), so Path.suffix would skip the + # ".md" and then look for a non-existent extension-less file. + target = refs_dir / (link if link.lower().endswith(".md") else f"{link}.md") if not target.exists(): result.errors.append( f"SKILL.md references [[references/{link}]] but " diff --git a/tests/test_skill_validator.py b/tests/test_skill_validator.py index 653cb64c0..6321072b6 100644 --- a/tests/test_skill_validator.py +++ b/tests/test_skill_validator.py @@ -259,6 +259,19 @@ def test_wikilink_without_md_suffix_resolves(tmp_path): assert result.passed, result.errors +def test_wikilink_dotted_stem_without_md_suffix_resolves(tmp_path): + # A reference name whose stem contains a dot (e.g. "api.v2") must still get + # the implicit ".md". Path.suffix would treat ".v2" as the suffix and skip + # appending ".md", falsely reporting the existing api.v2.md as missing. + sd = _write_skill( + tmp_path, "ref-dotted-stem", + body="See [[references/api.v2]] for details.\n", + refs={"api.v2.md": "# api v2\n"}, + ) + result = validate_skill(sd) + assert result.passed, result.errors + + # --------------------------------------------------------------------------- # scripts/ imports — strict mode only # --------------------------------------------------------------------------- From 883b22783848cfb2564c2c75732c37c86e4d0997 Mon Sep 17 00:00:00 2001 From: jichao wang Date: Sun, 21 Jun 2026 15:26:52 +0100 Subject: [PATCH 2/2] skill: reject references/ wikilinks that escape the references directory Review feedback: WIKILINK_RE allows '.' and '/', so '[[references/../SKILL]]' resolved outside references/ and was accepted whenever the resolved file existed (and would crash the not-found message's relative_to(skill_dir)). Reject any link whose resolved target is not under references/. --- openkb/skill/validator.py | 9 +++++++++ tests/test_skill_validator.py | 13 +++++++++++++ 2 files changed, 22 insertions(+) diff --git a/openkb/skill/validator.py b/openkb/skill/validator.py index b70444ae0..37ac71862 100644 --- a/openkb/skill/validator.py +++ b/openkb/skill/validator.py @@ -212,6 +212,15 @@ def validate_skill(skill_dir: Path, *, strict: bool = False) -> ValidationResult # "api.v2" has a truthy suffix (".v2"), so Path.suffix would skip the # ".md" and then look for a non-existent extension-less file. target = refs_dir / (link if link.lower().endswith(".md") else f"{link}.md") + # A reference must resolve to a file *under* references/ — reject any + # that escape it (e.g. "[[references/../SKILL]]"), which would also + # break the relative_to(skill_dir) call in the not-found message below. + if not target.resolve().is_relative_to(refs_dir.resolve()): + result.errors.append( + f"SKILL.md references [[references/{link}]] which resolves " + f"outside the references/ directory." + ) + continue if not target.exists(): result.errors.append( f"SKILL.md references [[references/{link}]] but " diff --git a/tests/test_skill_validator.py b/tests/test_skill_validator.py index 6321072b6..6010ebac8 100644 --- a/tests/test_skill_validator.py +++ b/tests/test_skill_validator.py @@ -272,6 +272,19 @@ def test_wikilink_dotted_stem_without_md_suffix_resolves(tmp_path): assert result.passed, result.errors +def test_wikilink_escaping_references_dir_is_rejected(tmp_path): + # A link that resolves outside references/ (e.g. "../SKILL") must error — + # not be accepted just because the resolved target (here SKILL.md) exists. + sd = _write_skill( + tmp_path, "ref-escape", + body="See [[references/../SKILL]] for details.\n", + refs={"topic.md": "# topic\n"}, + ) + result = validate_skill(sd) + assert not result.passed + assert any("outside the references/" in e for e in result.errors) + + # --------------------------------------------------------------------------- # scripts/ imports — strict mode only # ---------------------------------------------------------------------------