fix full-suite isolation regressions
This commit is contained in:
@@ -3,9 +3,7 @@ import type { Hooks, PluginInput } from "@opencode-ai/plugin"
|
||||
import { existsSync, realpathSync } from "fs"
|
||||
import { basename, dirname, isAbsolute, join, normalize, relative, resolve } from "path"
|
||||
|
||||
import { log } from "../../shared"
|
||||
import { handleWriteExistingFileGuardToolExecuteBefore } from "./tool-execute-before-handler"
|
||||
import { evictLeastRecentlyUsedSession, touchSession, trimSessionReadSet } from "./session-read-permissions"
|
||||
|
||||
export type GuardArgs = {
|
||||
filePath?: string
|
||||
@@ -16,7 +14,11 @@ export type GuardArgs = {
|
||||
|
||||
const MAX_TRACKED_SESSIONS = 256
|
||||
export const MAX_TRACKED_PATHS_PER_SESSION = 1024
|
||||
const BLOCK_MESSAGE = "File already exists. Use edit tool instead."
|
||||
|
||||
type WriteExistingFileGuardOptions = {
|
||||
maxTrackedSessions?: number
|
||||
maxTrackedPathsPerSession?: number
|
||||
}
|
||||
|
||||
export function asRecord(value: unknown): Record<string, unknown> | undefined {
|
||||
if (!value || typeof value !== "object" || Array.isArray(value)) {
|
||||
@@ -73,9 +75,11 @@ export function isOverwriteEnabled(value: boolean | string | undefined): boolean
|
||||
return false
|
||||
}
|
||||
|
||||
export function createWriteExistingFileGuardHook(ctx: PluginInput): Hooks {
|
||||
export function createWriteExistingFileGuardHook(ctx: PluginInput, options?: WriteExistingFileGuardOptions): Hooks {
|
||||
const readPermissionsBySession = new Map<string, Set<string>>()
|
||||
const sessionLastAccess = new Map<string, number>()
|
||||
const maxTrackedSessions = options?.maxTrackedSessions ?? MAX_TRACKED_SESSIONS
|
||||
const maxTrackedPathsPerSession = options?.maxTrackedPathsPerSession ?? MAX_TRACKED_PATHS_PER_SESSION
|
||||
let canonicalSessionRoot: string | undefined
|
||||
|
||||
function getCanonicalSessionRoot(): string {
|
||||
@@ -95,7 +99,8 @@ export function createWriteExistingFileGuardHook(ctx: PluginInput): Hooks {
|
||||
readPermissionsBySession,
|
||||
sessionLastAccess,
|
||||
getCanonicalSessionRoot,
|
||||
maxTrackedSessions: MAX_TRACKED_SESSIONS,
|
||||
maxTrackedSessions,
|
||||
maxTrackedPathsPerSession,
|
||||
})
|
||||
},
|
||||
event: async ({ event }: { event: { type: string; properties?: unknown } }) => {
|
||||
|
||||
@@ -3,7 +3,6 @@ import { existsSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync
|
||||
import { tmpdir } from "node:os"
|
||||
import { dirname, join, resolve } from "node:path"
|
||||
|
||||
import { MAX_TRACKED_PATHS_PER_SESSION } from "./hook"
|
||||
import { createWriteExistingFileGuardHook } from "./index"
|
||||
|
||||
const BLOCK_MESSAGE = "File already exists. Use edit tool instead."
|
||||
@@ -56,7 +55,7 @@ describe("createWriteExistingFileGuardHook", () => {
|
||||
}
|
||||
|
||||
const emitSessionDeleted = async (sessionID: string): Promise<void> => {
|
||||
await hook.event?.({ event: { type: "session.deleted", properties: { info: { id: sessionID } } } })
|
||||
await hook.event?.({ event: { type: "session.deleted", properties: { info: { id: sessionID } } } } as never)
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
@@ -432,6 +431,11 @@ describe("createWriteExistingFileGuardHook", () => {
|
||||
|
||||
test("#given session reads beyond path cap #when writing oldest and newest #then only newest is authorized", async () => {
|
||||
const sessionID = "ses_path_cap"
|
||||
const maxTrackedPathsPerSession = 4
|
||||
hook = createWriteExistingFileGuardHook(
|
||||
{ directory: tempDir } as never,
|
||||
{ maxTrackedPathsPerSession },
|
||||
)
|
||||
const oldestFile = createFile("path-cap/0.txt")
|
||||
let newestFile = oldestFile
|
||||
|
||||
@@ -441,7 +445,7 @@ describe("createWriteExistingFileGuardHook", () => {
|
||||
outputArgs: { filePath: oldestFile },
|
||||
})
|
||||
|
||||
for (let index = 1; index <= MAX_TRACKED_PATHS_PER_SESSION; index += 1) {
|
||||
for (let index = 1; index <= maxTrackedPathsPerSession; index += 1) {
|
||||
newestFile = createFile(`path-cap/${index}.txt`)
|
||||
await invoke({
|
||||
tool: "read",
|
||||
|
||||
@@ -5,37 +5,35 @@ import { join } from "node:path"
|
||||
|
||||
const realFs = await import("node:fs")
|
||||
|
||||
const existsSyncMock = mock(realFs.existsSync)
|
||||
const realpathNativeMock = mock(realFs.realpathSync.native)
|
||||
|
||||
mock.module("fs", () => ({
|
||||
...realFs,
|
||||
existsSync: existsSyncMock,
|
||||
realpathSync: {
|
||||
...realFs.realpathSync,
|
||||
native: realpathNativeMock,
|
||||
},
|
||||
}))
|
||||
|
||||
const { createWriteExistingFileGuardHook } = await import("./index")
|
||||
|
||||
describe("createWriteExistingFileGuardHook", () => {
|
||||
let tempDir = ""
|
||||
let existsSyncMock: ReturnType<typeof mock<typeof realFs.existsSync>>
|
||||
let realpathNativeMock: ReturnType<typeof mock<typeof realFs.realpathSync.native>>
|
||||
|
||||
beforeEach(() => {
|
||||
// given
|
||||
tempDir = mkdtempSync(join(tmpdir(), "write-existing-file-guard-lazy-"))
|
||||
mkdirSync(tempDir, { recursive: true })
|
||||
existsSyncMock.mockClear()
|
||||
realpathNativeMock.mockClear()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
mock.restore()
|
||||
rmSync(tempDir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
test("#given hook factory #when created #then defers fs canonical path calls until first tool invocation", async () => {
|
||||
// given
|
||||
existsSyncMock = mock(realFs.existsSync)
|
||||
realpathNativeMock = mock(realFs.realpathSync.native)
|
||||
mock.module("fs", () => ({
|
||||
...realFs,
|
||||
existsSync: existsSyncMock,
|
||||
realpathSync: {
|
||||
...realFs.realpathSync,
|
||||
native: realpathNativeMock,
|
||||
},
|
||||
}))
|
||||
const { createWriteExistingFileGuardHook } = await import(`./hook?test=${crypto.randomUUID()}`)
|
||||
const existingFile = join(tempDir, "existing.txt")
|
||||
writeFileSync(existingFile, "content")
|
||||
|
||||
|
||||
@@ -44,6 +44,7 @@ function registerReadPermission(params: {
|
||||
readPermissionsBySession: Map<string, Set<string>>
|
||||
sessionLastAccess: Map<string, number>
|
||||
maxTrackedSessions: number
|
||||
maxTrackedPathsPerSession: number
|
||||
}): void {
|
||||
const readSet = ensureSessionReadSet(params)
|
||||
if (readSet.has(params.canonicalPath)) {
|
||||
@@ -51,7 +52,7 @@ function registerReadPermission(params: {
|
||||
}
|
||||
|
||||
readSet.add(params.canonicalPath)
|
||||
trimSessionReadSet(readSet, MAX_TRACKED_PATHS_PER_SESSION)
|
||||
trimSessionReadSet(readSet, params.maxTrackedPathsPerSession)
|
||||
}
|
||||
|
||||
function consumeReadPermission(params: {
|
||||
@@ -92,8 +93,18 @@ export async function handleWriteExistingFileGuardToolExecuteBefore(params: {
|
||||
sessionLastAccess: Map<string, number>
|
||||
getCanonicalSessionRoot: () => string
|
||||
maxTrackedSessions: number
|
||||
maxTrackedPathsPerSession?: number
|
||||
}): Promise<void> {
|
||||
const { ctx, input, output, readPermissionsBySession, sessionLastAccess, getCanonicalSessionRoot, maxTrackedSessions } = params
|
||||
const {
|
||||
ctx,
|
||||
input,
|
||||
output,
|
||||
readPermissionsBySession,
|
||||
sessionLastAccess,
|
||||
getCanonicalSessionRoot,
|
||||
maxTrackedSessions,
|
||||
maxTrackedPathsPerSession = MAX_TRACKED_PATHS_PER_SESSION,
|
||||
} = params
|
||||
const toolName = input.tool?.toLowerCase()
|
||||
if (toolName !== "write" && toolName !== "read") {
|
||||
return
|
||||
@@ -124,6 +135,7 @@ export async function handleWriteExistingFileGuardToolExecuteBefore(params: {
|
||||
readPermissionsBySession,
|
||||
sessionLastAccess,
|
||||
maxTrackedSessions,
|
||||
maxTrackedPathsPerSession,
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user