Skip to content

Commit eec91fd

Browse files
sahrizviclaude
andcommitted
fix(workspace): a project with no git does not publish from /
Ralph's one open item on the re-review, traced independently by CodeRabbit and Codex. `Project.fromDirectory` sets the worktree to the sentinel `/` for a project with no git; `workdir(api)` returned it unchanged, so the TUI's containment boundary was `/` and any discovered skill on the machine passed. The TUI now falls back to the session directory, as the CLI already did — and `assertProjectSkill` refuses a filesystem root as a boundary outright, so the next caller that forgets cannot reopen this. Also, cubic's optional one: parent traversal is tested exactly (`..` or `../…`), so a directory literally named `..foo` under the root is inside. Verified: 668 pass across the workspace, plugin, fork-guard and skill suites, typecheck clean. Mutation-checked: accepting a root of `/`, and refusing `..foo`, each fail a test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Q51zFUmPg1WwtS5CrGJE6
1 parent 266e123 commit eec91fd

3 files changed

Lines changed: 41 additions & 5 deletions

File tree

‎packages/opencode/src/altimate/workspace/skill-publish.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -789,8 +789,15 @@ function updateConflict(err: ConflictError, skillName: string): Error {
789789
return err
790790
}
791791

792-
/** The skill's real directory, once it has passed. */
793-
function assertProjectSkill(projectRoot: string, skillDirectory: string): string {
792+
/** The skill's real directory, once it has passed. Exported for its test:
793+
* the boundary rule has to hold for every caller, and a root of `/` — the
794+
* sentinel a project with no git carries — is not a boundary at all. */
795+
export function assertProjectSkill(projectRoot: string, skillDirectory: string): string {
796+
// A filesystem root would contain everything. Both callers substitute the
797+
// session directory for the `/` sentinel; this refuses it in case one
798+
// forgets, since the failure mode is publishing anything on the machine.
799+
if (path.resolve(projectRoot) === path.parse(path.resolve(projectRoot)).root)
800+
throw new NotProjectSkillError(skillDirectory)
794801
const lexical = path.resolve(skillDirectory)
795802
let real: string
796803
try {
@@ -817,7 +824,10 @@ function assertProjectSkill(projectRoot: string, skillDirectory: string): string
817824
root = path.resolve(projectRoot)
818825
}
819826
const rel = path.relative(root, real)
820-
if (rel.startsWith("..") || path.isAbsolute(rel)) throw new NotProjectSkillError(skillDirectory)
827+
// Parent traversal exactly, not any name that begins with two dots: a
828+
// skill directory literally named `..foo` is inside the project.
829+
if (rel === ".." || rel.startsWith(".." + path.sep) || path.isAbsolute(rel))
830+
throw new NotProjectSkillError(skillDirectory)
821831
return real
822832
}
823833

‎packages/opencode/src/plugin/tui/altimate/skill-ops.tsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -573,8 +573,10 @@ function openActionPicker(api: TuiPluginApi, info: SkillInfo | undefined, skillN
573573
// `workdir` resolves to, and the two differ in a worktree subdirectory.
574574
const projectDirectory = api.state.path.directory || workdir(api)
575575
// The boundary a skill must lie within: the worktree, since discovery
576-
// walks up to it. `workdir` already resolves that.
577-
const projectRoot = workdir(api)
576+
// walks up to it — EXCEPT for a project with no git, where the worktree
577+
// is the sentinel `/` and `workdir` returns it unchanged. A root of `/`
578+
// would accept any skill on the machine. Same fallback as the CLI.
579+
const projectRoot = api.state.path.worktree === "/" ? projectDirectory : workdir(api)
578580
const managed = !isBuiltin && isManagedSkill(projectDirectory, path.dirname(info!.location))
579581

580582
const actions: TuiDialogSelectOption<string>[] = (

‎packages/opencode/test/altimate/workspace/skill-publish.test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ const {
4949
NotLinkedError,
5050
SkillNameConflictError,
5151
SymlinkError,
52+
assertProjectSkill,
5253
collectBundle,
5354
describePublish,
5455
explainPublishError,
@@ -677,6 +678,29 @@ describe("what counts as a project skill", () => {
677678
expect(report.action).toBe("created")
678679
})
679680

681+
test("a root of `/` is no boundary: the session directory must be the fallback", () => {
682+
// A project with no git carries the sentinel worktree `/`. Passed through
683+
// as the boundary, it contains every skill on the machine — a parent
684+
// directory's `.opencode/skills/x`, an absolute `skills.paths` entry.
685+
// Ralph traced it on the TUI, where `workdir` returned `/` unchanged;
686+
// the shared path refuses it too, for the next caller that forgets.
687+
const elsewhere = mkdtempSync(path.join(SANDBOX, "elsewhere-"))
688+
writeFileSync(path.join(elsewhere, "SKILL.md"), "---\nname: x\n---\n")
689+
expect(() => assertProjectSkill("/", elsewhere)).toThrow(NotProjectSkillError)
690+
// The same skill against the session directory: outside it, refused;
691+
// inside it, allowed.
692+
expect(() => assertProjectSkill(project, elsewhere)).toThrow(NotProjectSkillError)
693+
expect(assertProjectSkill(project, skillDir)).toBe(realpathSync(skillDir))
694+
})
695+
696+
test("a directory named with two leading dots is still inside the project", () => {
697+
// Directly under the root, so `path.relative` is `..foo` itself — the
698+
// one spelling a `startsWith("..")` check mistakes for traversal.
699+
const odd = path.join(project, "..foo")
700+
mkdirSync(odd, { recursive: true })
701+
expect(assertProjectSkill(project, odd)).toBe(realpathSync(odd))
702+
})
703+
680704
test("a skill outside the project is refused", async () => {
681705
// A personal skill under the home directory is the user's, not this
682706
// project's, and publishing would share it with the whole workspace.

0 commit comments

Comments
 (0)