fix(core): authorize relative external paths (#37689)
This commit is contained in:
parent
4e85a37590
commit
c310ef82f4
4 changed files with 52 additions and 17 deletions
|
|
@ -12,8 +12,8 @@ export const Kind = Schema.Literals(["file", "directory"])
|
|||
export type Kind = typeof Kind.Type
|
||||
|
||||
/**
|
||||
* Mutation paths do not accept project references. Relative paths must stay
|
||||
* inside the active Location. Absolute paths outside it require separate
|
||||
* Mutation paths do not accept project references. Relative paths resolve
|
||||
* from the active Location. Paths outside it require separate
|
||||
* `external_directory` approval.
|
||||
*/
|
||||
export const ResolveInput = Schema.Struct({
|
||||
|
|
@ -25,7 +25,7 @@ export type ResolveInput = typeof ResolveInput.Type
|
|||
|
||||
export class PathError extends Schema.TaggedErrorClass<PathError>()("LocationMutation.PathError", {
|
||||
path: Schema.String,
|
||||
reason: Schema.Literals(["relative_escape", "location_escape", "non_directory_ancestor"]),
|
||||
reason: Schema.Literals(["location_escape", "non_directory_ancestor"]),
|
||||
}) {}
|
||||
|
||||
export interface ExternalDirectoryAuthorization {
|
||||
|
|
@ -53,9 +53,9 @@ export interface Target {
|
|||
|
||||
export interface Interface {
|
||||
/**
|
||||
* Resolve a path and derive its permission resources. Relative paths must
|
||||
* stay inside the Location. Absolute paths outside it require separate
|
||||
* `external_directory` approval. This does not approve the mutation.
|
||||
* Resolve a path and derive its permission resources. Relative paths resolve
|
||||
* from the Location. Paths outside it require separate `external_directory`
|
||||
* approval. This does not approve the mutation.
|
||||
*/
|
||||
readonly resolve: (input: ResolveInput) => Effect.Effect<Target, PathError | FSUtil.Error>
|
||||
}
|
||||
|
|
@ -120,10 +120,8 @@ const layer = Layer.effect(
|
|||
})
|
||||
|
||||
const resolve = Effect.fn("LocationMutation.resolve")(function* (input: ResolveInput) {
|
||||
const relative = !path.isAbsolute(input.path)
|
||||
const absolute = path.resolve(location.directory, input.path)
|
||||
const lexicallyInternal = FSUtil.contains(location.directory, absolute)
|
||||
if (relative && !lexicallyInternal) return yield* new PathError({ path: input.path, reason: "relative_escape" })
|
||||
|
||||
const resolved = yield* resolvePath(absolute)
|
||||
if (lexicallyInternal && !FSUtil.contains(locationRoot, resolved.canonical)) {
|
||||
|
|
@ -146,10 +144,7 @@ const layer = Layer.effect(
|
|||
directory: externalDirectory,
|
||||
resource: externalResource,
|
||||
save: slash(
|
||||
path.join(
|
||||
(yield* Project.root(fs, AbsolutePath.make(externalDirectory))) ?? externalDirectory,
|
||||
"*",
|
||||
),
|
||||
path.join((yield* Project.root(fs, AbsolutePath.make(externalDirectory))) ?? externalDirectory, "*"),
|
||||
),
|
||||
}
|
||||
: undefined,
|
||||
|
|
|
|||
|
|
@ -60,11 +60,19 @@ describe("LocationMutation", () => {
|
|||
),
|
||||
)
|
||||
|
||||
it.live("rejects a relative lexical escape instead of promoting it to external authority", () =>
|
||||
it.live("requires external-directory authorization for a relative lexical escape", () =>
|
||||
withTmp((directory) =>
|
||||
Effect.gen(function* () {
|
||||
const error = yield* Effect.flip((yield* LocationMutation.Service).resolve({ path: "../outside.txt" }))
|
||||
expect(error).toMatchObject({ _tag: "LocationMutation.PathError", reason: "relative_escape" })
|
||||
const target = yield* (yield* LocationMutation.Service).resolve({ path: "../outside.txt" })
|
||||
const root = yield* Effect.promise(() => fs.realpath(path.dirname(directory)))
|
||||
expect(target).toMatchObject({
|
||||
canonical: path.join(root, "outside.txt"),
|
||||
resource: path.join(root, "outside.txt").replaceAll("\\", "/"),
|
||||
})
|
||||
expect(target.externalDirectory).toMatchObject({
|
||||
directory: root,
|
||||
resource: path.join(root, "*").replaceAll("\\", "/"),
|
||||
})
|
||||
}).pipe(provide(directory)),
|
||||
),
|
||||
)
|
||||
|
|
|
|||
|
|
@ -277,6 +277,38 @@ describe("PatchTool", () => {
|
|||
),
|
||||
)
|
||||
|
||||
it.live("approves a relative external target before reading update content", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
|
||||
([active, outside]) => {
|
||||
reset()
|
||||
const target = path.join(outside.path, "external.txt")
|
||||
const relative = path.relative(active.path, target)
|
||||
return Effect.promise(() => fs.writeFile(target, "before\n")).pipe(
|
||||
Effect.andThen(
|
||||
withTool(active.path, (registry) =>
|
||||
Effect.gen(function* () {
|
||||
expect(
|
||||
yield* executeTool(
|
||||
registry,
|
||||
call(`*** Begin Patch\n*** Update File: ${relative}\n@@\n-before\n+after\n*** End Patch`),
|
||||
),
|
||||
).toMatchObject({ type: "text" })
|
||||
expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"])
|
||||
expect(readsBeforeEditApproval).toBe(0)
|
||||
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\n")
|
||||
}),
|
||||
),
|
||||
),
|
||||
)
|
||||
},
|
||||
([active, outside]) =>
|
||||
Effect.promise(() =>
|
||||
Promise.all([active[Symbol.asyncDispose](), outside[Symbol.asyncDispose]()]).then(() => undefined),
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
it.live("approves one external directory scope for multiple files under the same parent", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
|
||||
|
|
|
|||
|
|
@ -385,8 +385,8 @@ Affected schema:
|
|||
|
||||
Change:
|
||||
|
||||
- Resolve relative mutation paths within the active Location.
|
||||
- Accept absolute internal paths and require explicit `external_directory` approval before leaf approval for external absolute paths.
|
||||
- Resolve relative mutation paths from the active Location.
|
||||
- Require explicit `external_directory` approval before leaf approval for external paths.
|
||||
- Keep named references read-oriented and reject them for mutation.
|
||||
- Revalidate path authority immediately before write mechanics.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue