fix(rules-core): isolate package + block symlink escape from rule sources
- Drop the back-import of src/shared/logger so @oh-my-opencode/rules-core stays free of host-adapter dependencies (ROADMAP package layering invariant). Expose setSisyphusRuleDeprecationLogger(logger) for hosts to inject their own logger; default is a noop. - Wire the host injection in src/hooks/rules-injector/rule-file-finder.ts as a module-level side effect so existing behavior is preserved. - Add realpath boundary check to findRuleFilesRecursive and validFileRealPath. Project rule scanners now refuse entries whose realpath escapes the rule-source root, closing the symlink-escape vector where a malicious repo could point .github/copilot-instructions.md (or any .omo/rules/* entry) at ~/.ssh/id_rsa and have the rule injector pull the secret into model context.
This commit is contained in:
@@ -5,12 +5,22 @@ import { GLOBAL_DISTANCE, OPENCODE_USER_RULE_DIRS, PROJECT_RULE_FILES, PROJECT_R
|
|||||||
import { sortCandidates } from "./ordering";
|
import { sortCandidates } from "./ordering";
|
||||||
import { findRuleFilesRecursive, safeRealpathSync } from "./scanner";
|
import { findRuleFilesRecursive, safeRealpathSync } from "./scanner";
|
||||||
import type { DirectoryScanEntry, FindRuleFilesOptions, RuleFileCandidate, RuleScanCache, RuleSource } from "./types";
|
import type { DirectoryScanEntry, FindRuleFilesOptions, RuleFileCandidate, RuleScanCache, RuleSource } from "./types";
|
||||||
import { log } from "../../../src/shared/logger";
|
|
||||||
|
export type SisyphusRuleDeprecationLogger = (
|
||||||
|
message: string,
|
||||||
|
meta: { event: string; path: string },
|
||||||
|
) => void;
|
||||||
|
|
||||||
|
const noopSisyphusRuleDeprecationLogger: SisyphusRuleDeprecationLogger = () => {};
|
||||||
|
|
||||||
const SISYPHUS_DEPRECATION_MESSAGE = "[rules] .sisyphus/rules is deprecated and will be removed in v4.3.0; migrate to .omo/rules";
|
const SISYPHUS_DEPRECATION_MESSAGE = "[rules] .sisyphus/rules is deprecated and will be removed in v4.3.0; migrate to .omo/rules";
|
||||||
const SISYPHUS_LEGACY_RULE_SOURCES: ReadonlySet<RuleSource> = new Set([".sisyphus/rules", "~/.sisyphus/rules"]);
|
const SISYPHUS_LEGACY_RULE_SOURCES: ReadonlySet<RuleSource> = new Set([".sisyphus/rules", "~/.sisyphus/rules"]);
|
||||||
const warnedSisyphusRuleDirectories = new Set<string>();
|
const warnedSisyphusRuleDirectories = new Set<string>();
|
||||||
let logSisyphusRuleDeprecation: typeof log = log;
|
let logSisyphusRuleDeprecation: SisyphusRuleDeprecationLogger = noopSisyphusRuleDeprecationLogger;
|
||||||
|
|
||||||
|
export function setSisyphusRuleDeprecationLogger(logger: SisyphusRuleDeprecationLogger): void {
|
||||||
|
logSisyphusRuleDeprecation = logger;
|
||||||
|
}
|
||||||
|
|
||||||
export function findRuleFiles(
|
export function findRuleFiles(
|
||||||
projectRoot: string | null,
|
projectRoot: string | null,
|
||||||
@@ -92,9 +102,10 @@ function addProjectSingleFileCandidates(
|
|||||||
candidates: RuleFileCandidate[],
|
candidates: RuleFileCandidate[],
|
||||||
seenRealPaths: Set<string>,
|
seenRealPaths: Set<string>,
|
||||||
): void {
|
): void {
|
||||||
|
const projectRootRealPath = safeRealpathSync(projectRoot);
|
||||||
for (const ruleFile of PROJECT_RULE_FILES) {
|
for (const ruleFile of PROJECT_RULE_FILES) {
|
||||||
const filePath = join(projectRoot, ruleFile);
|
const filePath = join(projectRoot, ruleFile);
|
||||||
const realPath = validFileRealPath(filePath);
|
const realPath = validFileRealPath(filePath, projectRootRealPath);
|
||||||
if (realPath === null || seenRealPaths.has(realPath)) continue;
|
if (realPath === null || seenRealPaths.has(realPath)) continue;
|
||||||
seenRealPaths.add(realPath);
|
seenRealPaths.add(realPath);
|
||||||
candidates.push({
|
candidates.push({
|
||||||
@@ -135,11 +146,11 @@ function addUserRuleCandidates(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function scanDirectoryWithCache(dir: string, cache: RuleScanCache | undefined): readonly DirectoryScanEntry[] {
|
function scanDirectoryWithCache(dir: string, cache: RuleScanCache | undefined, boundaryRealPath?: string): readonly DirectoryScanEntry[] {
|
||||||
const cached = cache?.getDirScan(dir);
|
const cached = cache?.getDirScan(dir);
|
||||||
if (cached) return cached;
|
if (cached) return cached;
|
||||||
const entries: DirectoryScanEntry[] = [];
|
const entries: DirectoryScanEntry[] = [];
|
||||||
findRuleFilesRecursive(dir, entries);
|
findRuleFilesRecursive(dir, entries, new Set<string>(), boundaryRealPath);
|
||||||
cache?.setDirScan(dir, entries);
|
cache?.setDirScan(dir, entries);
|
||||||
return entries;
|
return entries;
|
||||||
}
|
}
|
||||||
@@ -155,20 +166,22 @@ function warnSisyphusRuleDeprecation(source: RuleSource, path: string): void {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
export function _setSisyphusRuleDeprecationLoggerForTesting(logger: typeof log): void {
|
export function _setSisyphusRuleDeprecationLoggerForTesting(logger: SisyphusRuleDeprecationLogger): void {
|
||||||
logSisyphusRuleDeprecation = logger;
|
logSisyphusRuleDeprecation = logger;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function _resetSisyphusRuleDeprecationWarningStateForTesting(): void {
|
export function _resetSisyphusRuleDeprecationWarningStateForTesting(): void {
|
||||||
warnedSisyphusRuleDirectories.clear();
|
warnedSisyphusRuleDirectories.clear();
|
||||||
logSisyphusRuleDeprecation = log;
|
logSisyphusRuleDeprecation = noopSisyphusRuleDeprecationLogger;
|
||||||
}
|
}
|
||||||
|
|
||||||
function validFileRealPath(filePath: string): string | null {
|
function validFileRealPath(filePath: string, boundaryRealPath?: string): string | null {
|
||||||
if (!existsSync(filePath)) return null;
|
if (!existsSync(filePath)) return null;
|
||||||
try {
|
try {
|
||||||
if (!statSync(filePath).isFile()) return null;
|
if (!statSync(filePath).isFile()) return null;
|
||||||
return safeRealpathSync(filePath);
|
const realPath = safeRealpathSync(filePath);
|
||||||
|
if (boundaryRealPath !== undefined && !isSameOrChildPath(realPath, boundaryRealPath)) return null;
|
||||||
|
return realPath;
|
||||||
} catch {
|
} catch {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
export { createAgentsMdCache, createRuleScanCache } from "./cache";
|
export { createAgentsMdCache, createRuleScanCache } from "./cache";
|
||||||
export { findAgentsMdUp, type FindAgentsMdUpInput } from "./agents-md";
|
export { findAgentsMdUp, type FindAgentsMdUpInput } from "./agents-md";
|
||||||
export { findRuleFiles } from "./finder";
|
export { findRuleFiles, setSisyphusRuleDeprecationLogger, type SisyphusRuleDeprecationLogger } from "./finder";
|
||||||
export { parseRuleFrontmatter } from "./parser";
|
export { parseRuleFrontmatter } from "./parser";
|
||||||
export { shouldApplyRule, createContentHash, isDuplicateByContentHash, isDuplicateByRealPath, resetMatcherCache, getMatcherCacheStats } from "./matcher";
|
export { shouldApplyRule, createContentHash, isDuplicateByContentHash, isDuplicateByRealPath, resetMatcherCache, getMatcherCacheStats } from "./matcher";
|
||||||
export { findProjectRoot, clearProjectRootCache } from "./project-root";
|
export { findProjectRoot, clearProjectRootCache } from "./project-root";
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { existsSync, readdirSync, realpathSync } from "node:fs";
|
import { existsSync, readdirSync, realpathSync, type Dirent } from "node:fs";
|
||||||
import { join } from "node:path";
|
import { isAbsolute, join, relative } from "node:path";
|
||||||
import { EXCLUDED_DIRS, GITHUB_INSTRUCTIONS_PATTERN, RULE_EXTENSIONS } from "./constants";
|
import { EXCLUDED_DIRS, GITHUB_INSTRUCTIONS_PATTERN, RULE_EXTENSIONS } from "./constants";
|
||||||
import type { DirectoryScanEntry } from "./types";
|
import type { DirectoryScanEntry } from "./types";
|
||||||
|
|
||||||
@@ -20,12 +20,24 @@ export function safeRealpathSync(filePath: string): string {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
export function findRuleFilesRecursive(dir: string, results: DirectoryScanEntry[], visited = new Set<string>()): void {
|
function isPathWithinRoot(candidate: string, root: string): boolean {
|
||||||
|
const rel = relative(root, candidate);
|
||||||
|
return rel === "" || (!rel.startsWith("..") && !isAbsolute(rel));
|
||||||
|
}
|
||||||
|
|
||||||
|
export function findRuleFilesRecursive(
|
||||||
|
dir: string,
|
||||||
|
results: DirectoryScanEntry[],
|
||||||
|
visited = new Set<string>(),
|
||||||
|
boundaryRoot?: string,
|
||||||
|
): void {
|
||||||
if (!existsSync(dir)) return;
|
if (!existsSync(dir)) return;
|
||||||
const realDir = safeRealpathSync(dir);
|
const realDir = safeRealpathSync(dir);
|
||||||
|
const effectiveBoundary = boundaryRoot ?? realDir;
|
||||||
|
if (!isPathWithinRoot(realDir, effectiveBoundary)) return;
|
||||||
if (visited.has(realDir)) return;
|
if (visited.has(realDir)) return;
|
||||||
visited.add(realDir);
|
visited.add(realDir);
|
||||||
let entries;
|
let entries: Dirent<string>[] = [];
|
||||||
try {
|
try {
|
||||||
entries = readdirSync(dir, { withFileTypes: true, encoding: "utf8" }).sort((left, right) => left.name.localeCompare(right.name));
|
entries = readdirSync(dir, { withFileTypes: true, encoding: "utf8" }).sort((left, right) => left.name.localeCompare(right.name));
|
||||||
} catch {
|
} catch {
|
||||||
@@ -34,11 +46,13 @@ export function findRuleFilesRecursive(dir: string, results: DirectoryScanEntry[
|
|||||||
for (const entry of entries) {
|
for (const entry of entries) {
|
||||||
const fullPath = join(dir, entry.name);
|
const fullPath = join(dir, entry.name);
|
||||||
if (entry.isDirectory()) {
|
if (entry.isDirectory()) {
|
||||||
if (!EXCLUDED_DIRS.has(entry.name)) findRuleFilesRecursive(fullPath, results, visited);
|
if (!EXCLUDED_DIRS.has(entry.name)) findRuleFilesRecursive(fullPath, results, visited, effectiveBoundary);
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if (entry.isFile() && isRuleFile(entry.name, dir)) {
|
if (entry.isFile() && isRuleFile(entry.name, dir)) {
|
||||||
results.push({ path: fullPath, realPath: safeRealpathSync(fullPath), relativePath: entry.name });
|
const realPath = safeRealpathSync(fullPath);
|
||||||
|
if (!isPathWithinRoot(realPath, effectiveBoundary)) continue;
|
||||||
|
results.push({ path: fullPath, realPath, relativePath: entry.name });
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,2 +1,7 @@
|
|||||||
|
import { setSisyphusRuleDeprecationLogger } from "@oh-my-opencode/rules-core";
|
||||||
|
import { log } from "../../shared/logger";
|
||||||
|
|
||||||
|
setSisyphusRuleDeprecationLogger(log);
|
||||||
|
|
||||||
export { findRuleFiles } from "@oh-my-opencode/rules-core";
|
export { findRuleFiles } from "@oh-my-opencode/rules-core";
|
||||||
export type { FindRuleFilesOptions } from "@oh-my-opencode/rules-core";
|
export type { FindRuleFilesOptions } from "@oh-my-opencode/rules-core";
|
||||||
|
|||||||
Reference in New Issue
Block a user