Add permission check before write/delete operations This avoids

accidentally writing to a read-only file and only finding out about it
after a sync.

Also removes redundant space primitive implementations that weren't used
This commit is contained in:
Zef Hemel
2025-11-19 14:57:10 +01:00
parent ce25cc9d9e
commit b0f6b9a33d
4 changed files with 30 additions and 208 deletions
+5 -22
View File
@@ -37,7 +37,6 @@ import type {
} from "@silverbulletmd/silverbullet/type/event";
import type { StyleObject } from "../plugs/index/style.ts";
import { jitter, throttle } from "@silverbulletmd/silverbullet/lib/async";
import { PlugSpacePrimitives } from "./spaces/plug_space_primitives.ts";
import { EventedSpacePrimitives } from "./spaces/evented_space_primitives.ts";
import { HttpSpacePrimitives } from "./spaces/http_space_primitives.ts";
import {
@@ -54,12 +53,10 @@ import {
import { ClientSystem } from "./client_system.ts";
import { createEditorState, isValidEditor } from "./codemirror/editor_state.ts";
import { MainUI } from "./editor_ui.tsx";
import type { SpacePrimitives } from "./spaces/space_primitives.ts";
import { DataStore } from "./data/datastore.ts";
import { IndexedDBKvPrimitives } from "./data/indexeddb_kv_primitives.ts";
import { DataStoreMQ } from "./data/mq.datastore.ts";
import { ReadOnlySpacePrimitives } from "./spaces/ro_space_primitives.ts";
import { LimitedMap } from "@silverbulletmd/silverbullet/lib/limited_map";
import { fsEndpoint } from "./spaces/constants.ts";
import { diffAndPrepareChanges } from "./codemirror/cm_util.ts";
@@ -72,7 +69,7 @@ import type {
PageMeta,
} from "@silverbulletmd/silverbullet/type/index";
import { parseMarkdown } from "./markdown_parser/parser.ts";
import { CheckPathSpacePrimitives } from "./spaces/checked_space_primitives.ts";
import { CheckedSpacePrimitives } from "./spaces/checked_space_primitives.ts";
import {
notFoundError,
offlineError,
@@ -109,7 +106,6 @@ export class Client {
space!: Space;
clientSystem!: ClientSystem;
plugSpaceRemotePrimitives!: PlugSpacePrimitives;
eventedSpacePrimitives!: EventedSpacePrimitives;
httpSpacePrimitives!: HttpSpacePrimitives;
@@ -315,24 +311,11 @@ export class Client {
},
);
let remoteSpacePrimitives: SpacePrimitives = new CheckPathSpacePrimitives(
this.httpSpacePrimitives,
);
if (this.bootConfig.readOnly) {
remoteSpacePrimitives = new ReadOnlySpacePrimitives(
this.httpSpacePrimitives,
);
}
this.plugSpaceRemotePrimitives = new PlugSpacePrimitives(
remoteSpacePrimitives,
this.clientSystem.namespaceHook,
this.bootConfig.readOnly ? undefined : "client",
);
this.eventedSpacePrimitives = new EventedSpacePrimitives(
this.httpSpacePrimitives,
new CheckedSpacePrimitives(
this.httpSpacePrimitives,
this.bootConfig.readOnly,
),
this.eventHook,
this.ds,
);
+25 -8
View File
@@ -2,9 +2,15 @@ import type { FileMeta } from "@silverbulletmd/silverbullet/type/index";
import type { SpacePrimitives } from "./space_primitives.ts";
import { isValidPath } from "@silverbulletmd/silverbullet/lib/ref";
export class CheckPathSpacePrimitives implements SpacePrimitives {
/**
* Adds checks for two things:
* 1. Allowed path names
* 2. Permissions
*/
export class CheckedSpacePrimitives implements SpacePrimitives {
constructor(
private wrapped: SpacePrimitives,
private readOnly: boolean,
) {
}
@@ -28,23 +34,23 @@ export class CheckPathSpacePrimitives implements SpacePrimitives {
return this.wrapped.getFileMeta(path, observing);
}
writeFile(
async writeFile(
path: string,
data: Uint8Array,
meta?: FileMeta,
): Promise<FileMeta> {
if (!this.isWritable(path)) {
throw new Error("Couldn't write file, path is invalid");
if (!await this.isWritable(path)) {
throw new Error("Couldn't write file, path is not writable");
}
return this.wrapped.writeFile(path, data, meta);
}
deleteFile(path: string): Promise<void> {
async deleteFile(path: string): Promise<void> {
// We allow deletion of paths we can't write to. This is for the case when
// the user has an invalidly named file in their space and they need to
// remove it/rename it
if (!this.isReadable(path)) {
throw new Error("Couldn't delete file, path isn't writable");
if (!await this.isWritable(path)) {
throw new Error("Couldn't delete file, path is not writable");
}
return this.wrapped.deleteFile(path);
}
@@ -53,7 +59,18 @@ export class CheckPathSpacePrimitives implements SpacePrimitives {
return !path.startsWith(".");
}
private isWritable(path: string): boolean {
private async isWritable(path: string): Promise<boolean> {
if (this.readOnly) {
return false;
}
try {
const fileMeta = await this.getFileMeta(path);
if (fileMeta.perm === "ro") {
return false;
}
} catch {
// Assumption, not found, that's ok
}
return this.isReadable(path) && isValidPath(path);
}
}
-134
View File
@@ -1,134 +0,0 @@
import type { SpacePrimitives } from "./space_primitives.ts";
import type { NamespaceOperation } from "@silverbulletmd/silverbullet/type/namespace";
import type { PlugNamespaceHook } from "../plugos/hooks/plug_namespace.ts";
import type { FileMeta } from "@silverbulletmd/silverbullet/type/index";
export class PlugSpacePrimitives implements SpacePrimitives {
constructor(
private wrapped: SpacePrimitives,
private hook: PlugNamespaceHook,
private env?: string,
) {
}
// Used e.g. by the sync engine to see if it should sync a certain path (likely not the case when we have a plug space override)
public isLikelyHandled(path: string): boolean {
for (
const { pattern } of this.hook.spaceFunctions
) {
if (path.match(pattern)) {
return true;
}
}
return false;
}
performOperation(
type: NamespaceOperation,
path: string,
...args: any[]
): Promise<any> | false {
for (
const { operation, pattern, plug, name, env } of this.hook.spaceFunctions
) {
// console.log(
// "Going to match agains pattern",
// operation,
// pattern,
// path,
// this.env,
// env,
// );
if (
operation === type && path.match(pattern) &&
// Both envs are set, and they don't match
(!this.env || !env || env === this.env)
) {
return plug.invoke(name, [path, ...args]);
}
}
return false;
}
async fetchFileList(): Promise<FileMeta[]> {
const allFiles: FileMeta[] = [];
const alreadySeenFiles = new Set<string>();
for (const { plug, name, operation } of this.hook.spaceFunctions) {
if (operation === "listFiles") {
try {
for (const pm of await plug.invoke(name, [])) {
allFiles.push(pm);
// console.log("Adding file from plug space", pm.name);
alreadySeenFiles.add(pm.name);
}
} catch (e: any) {
if (!e.message.includes("not available")) {
// Don't report "not available in" environments errors
console.error("Error listing files", e);
}
}
}
}
const files = await this.wrapped.fetchFileList();
for (const pm of files) {
// We'll use the files coming from the wrapped space only as a fallback
if (alreadySeenFiles.has(pm.name)) {
continue;
}
allFiles.push(pm);
}
return allFiles;
}
async readFile(
path: string,
): Promise<{ data: Uint8Array; meta: FileMeta }> {
const result: { data: Uint8Array; meta: FileMeta } | false = await this
.performOperation(
"readFile",
path,
);
if (result) {
return result;
}
return this.wrapped.readFile(path);
}
getFileMeta(path: string, observing?: boolean): Promise<FileMeta> {
const result = this.performOperation("getFileMeta", path, observing);
if (result) {
return result;
}
return this.wrapped.getFileMeta(path, observing);
}
writeFile(
path: string,
data: Uint8Array,
meta?: FileMeta,
): Promise<FileMeta> {
const result = this.performOperation(
"writeFile",
path,
data,
meta,
);
if (result) {
return result;
}
return this.wrapped.writeFile(
path,
data,
meta,
);
}
deleteFile(path: string): Promise<void> {
const result = this.performOperation("deleteFile", path);
if (result) {
return result;
}
return this.wrapped.deleteFile(path);
}
}
-44
View File
@@ -1,44 +0,0 @@
import type { SpacePrimitives } from "./space_primitives.ts";
import type { FileMeta } from "../../plug-api/types/index.ts";
export class ReadOnlySpacePrimitives implements SpacePrimitives {
wrapped: SpacePrimitives;
constructor(wrapped: SpacePrimitives) {
this.wrapped = wrapped;
}
async fetchFileList(): Promise<FileMeta[]> {
return (await this.wrapped.fetchFileList()).map((f: FileMeta) => ({
...f,
perm: "ro",
}));
}
async readFile(path: string): Promise<{ meta: FileMeta; data: Uint8Array }> {
const { meta, data } = await this.wrapped.readFile(path);
return {
meta: {
...meta,
perm: "ro",
},
data,
};
}
async getFileMeta(path: string, observing?: boolean): Promise<FileMeta> {
const meta = await this.wrapped.getFileMeta(path, observing);
return {
...meta,
perm: "ro",
};
}
writeFile(): Promise<FileMeta> {
throw new Error("Read only space, not allowed to write");
}
deleteFile(): Promise<void> {
throw new Error("Read only space, not allowed to delete");
}
}