Should internal-link validation reject symlinks that resolve outside the skill directory?

[yuehua1213]

I was reviewing the internal-link boundary in structure/links.go at commit 7480766908955b0ff5b232a7d2e60217732de5cb.

The current check cleans the joined path and verifies that its string prefix remains inside the skill directory, then calls os.Stat. A path such as references/guide.md therefore looks internal syntactically, but could still be a symlink whose target resolves outside the skill directory.

Is that intentionally allowed, or should “internal link” mean the final resolved target also stays inside the skill package?

If it should be rejected, would a focused regression test in the existing structure test suite be the right way to establish the expected behavior? I’m also interested in whether you would be comfortable with the resulting before/after CI evidence being referenced publicly after a fix is accepted.

[dacharyc]

Thanks for the report. You're right that the internal-link check validated containment syntactically and then used os.Stat, which follows symlinks. This is fixed in #90: link targets are now resolved with filepath.EvalSymlinks and must remain inside the skill package, and the same containment rule applies to file reads throughout the validator. The fix will be in the next release, along with a changelog entry describing the hardening.

On your last question: this repository is public, so its issues, commits, and CI runs can be referenced like any other public material. I don't review or endorse external writeups, so I'd ask that anything you publish stand on the public record as it is, and describe the behavior accurately, including that it required running the validator against a malicious skill package and has been fixed.