diff --git a/scripts/source-file-size-baseline.json b/scripts/source-file-size-baseline.json index f5086c82f3..a5fbd4ea78 100644 --- a/scripts/source-file-size-baseline.json +++ b/scripts/source-file-size-baseline.json @@ -31,7 +31,7 @@ "src/mods/package-installer.test.ts": 1248, "src/mods/package-installer.ts": 1201, "src/mods/package-registry.ts": 1033, - "src/permissions/checker.ts": 1009, + "src/permissions/checker.ts": 1007, "src/permissions/read-only-shell.test.ts": 1303, "src/permissions/read-only-shell.ts": 2009, "src/providers/chatgpt-usage-service.ts": 1112, diff --git a/src/agent/memory-git-hooks.ts b/src/agent/memory-git-hooks.ts index eb98fb9d4a..7c1b24c99b 100644 --- a/src/agent/memory-git-hooks.ts +++ b/src/agent/memory-git-hooks.ts @@ -32,6 +32,10 @@ AGENT_EDITABLE_KEYS="description" PROTECTED_KEYS="read_only" ALL_KNOWN_KEYS="description read_only limit" errors="" +allow_read_only_change=false +if [ "$LETTA_APPROVED_READ_ONLY_CHANGE" = "1" ]; then + allow_read_only_change=true +fi # Skills must always be directories: skills//SKILL.md # Reject legacy flat skill files (both current and legacy repo layouts). @@ -48,6 +52,19 @@ get_fm_value() { echo "$content" | tail -n +2 | head -n $((closing_line - 1)) | grep "^$key:" | cut -d: -f2- | sed 's/^ *//;s/ *$//' } +# Deleting or renaming a read-only file must be rejected too. Deleted paths do +# not appear in the ACM validation loop below, so inspect their HEAD content +# separately. +if [ "$allow_read_only_change" = "false" ]; then + for file in $(git diff --cached --no-renames --name-only --diff-filter=D | grep -E '^(memory/)?(system|reference)/.*\\.md$' || true); do + head_content=$(git show "HEAD:$file" 2>/dev/null || true) + head_ro=$(get_fm_value "$head_content" "read_only") + if [ "$head_ro" = "true" ]; then + errors="$errors\n $file: file is read_only and cannot be deleted or renamed" + fi + done +fi + # Match .md files under system/ or reference/ (with optional memory/ prefix). # Skip skill SKILL.md files — they use a different frontmatter format. for file in $(git diff --cached --name-only --diff-filter=ACM | grep -E '^(memory/)?(system|reference)/.*\\.md$'); do @@ -69,7 +86,7 @@ for file in $(git diff --cached --name-only --diff-filter=ACM | grep -E '^(memor # Check read_only protection against HEAD version head_content=$(git show "HEAD:$file" 2>/dev/null || true) - if [ -n "$head_content" ]; then + if [ "$allow_read_only_change" = "false" ] && [ -n "$head_content" ]; then head_ro=$(get_fm_value "$head_content" "read_only") if [ "$head_ro" = "true" ]; then errors="$errors\\n $file: file is read_only and cannot be modified" @@ -110,6 +127,7 @@ for file in $(git diff --cached --name-only --diff-filter=ACM | grep -E '^(memor # Check if agent is trying to modify a protected key for k in $PROTECTED_KEYS; do if [ "$key" = "$k" ]; then + [ "$allow_read_only_change" = "true" ] && continue # Compare against HEAD — if value changed (or key was added), reject if [ -n "$head_content" ]; then head_val=$(get_fm_value "$head_content" "$key") @@ -143,7 +161,7 @@ for file in $(git diff --cached --name-only --diff-filter=ACM | grep -E '^(memor fi # Check if protected keys were removed (existed in HEAD but not in staged) - if [ -n "$head_content" ]; then + if [ "$allow_read_only_change" = "false" ] && [ -n "$head_content" ]; then for k in $PROTECTED_KEYS; do head_val=$(get_fm_value "$head_content" "$k") if [ -n "$head_val" ]; then diff --git a/src/agent/memory-git.precommit.test.ts b/src/agent/memory-git.precommit.test.ts index 260a66a0c9..1f0bb0790f 100644 --- a/src/agent/memory-git.precommit.test.ts +++ b/src/agent/memory-git.precommit.test.ts @@ -141,6 +141,36 @@ describe("pre-commit hook: field validation", () => { }); describe("pre-commit hook: read_only protection", () => { + test("rejects deleting or renaming a read_only file", () => { + const hookPath = join(tempDir, ".git", "hooks", "pre-commit"); + rmSync(hookPath); + writeAndStage( + "system/locked.md", + "---\ndescription: Locked\nread_only: true\n---\n\nContent.\n", + ); + tryCommit(); + writeFileSync(hookPath, PRE_COMMIT_HOOK_SCRIPT, { mode: 0o755 }); + + git("rm system/locked.md"); + expect(tryCommit().success).toBe(false); + git("reset --hard HEAD"); + git("mv system/locked.md system/renamed.md"); + expect(tryCommit().success).toBe(false); + }); + + test("allows an approved read_only change", () => { + writeAndStage( + "system/locked.md", + "---\ndescription: Locked\nread_only: true\n---\n", + ); + expect(() => + execSync('git commit -m "approved"', { + cwd: tempDir, + env: { ...GIT_ENV, LETTA_APPROVED_READ_ONLY_CHANGE: "1" }, + }), + ).not.toThrow(); + }); + test("rejects modifying a read_only file", () => { // First commit: create a read_only file (bypass hook for setup) const hookPath = join(tempDir, ".git", "hooks", "pre-commit"); diff --git a/src/cli/subcommands/memory-read-only.spec.ts b/src/cli/subcommands/memory-read-only.spec.ts new file mode 100644 index 0000000000..8938362804 --- /dev/null +++ b/src/cli/subcommands/memory-read-only.spec.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "bun:\u0074est"; +import { checkPermission } from "@/permissions/checker"; +import type { PermissionRules } from "@/permissions/types"; +import { updateReadOnlyFrontmatter } from "./memory-read-only"; + +const permissions: PermissionRules = { + allow: [], + deny: [], + ask: [], + alwaysAsk: [], + additionalDirectories: [], +}; + +describe("memory read-only", () => { + it("adds and toggles read_only without changing the body", () => { + const original = "---\ndescription: Keep\n---\n\nBody\n"; + const enabled = updateReadOnlyFrontmatter(original, true); + expect(enabled).toBe( + "---\ndescription: Keep\nread_only: true\n---\n\nBody\n", + ); + expect(updateReadOnlyFrontmatter(enabled ?? "", false)).toContain( + "read_only: false\n---\n\nBody\n", + ); + expect( + updateReadOnlyFrontmatter("---\r\ndescription: CRLF\r\n---\r\n", true), + ).toContain("read_only: true\r\n---"); + }); + + it("always requires approval", () => { + for (const command of [ + "letta memory read-only system/persona.md true", + `sh -c "letta memfs read-only reference/policy.md false"`, + ]) { + expect(checkPermission("Bash", { command }, permissions).decision).toBe( + "alwaysAsk", + ); + } + }); +}); diff --git a/src/cli/subcommands/memory-read-only.ts b/src/cli/subcommands/memory-read-only.ts new file mode 100644 index 0000000000..4d0434303f --- /dev/null +++ b/src/cli/subcommands/memory-read-only.ts @@ -0,0 +1,108 @@ +import { existsSync, readFileSync, realpathSync, writeFileSync } from "node:fs"; +import { isAbsolute, relative, resolve } from "node:path"; +import { + assertMemoryRepoCleanForWrite, + commitMemoryWrite, +} from "@/agent/memory-git"; +import { isLocalBackendEnvEnabled } from "@/backend/local/paths"; + +export function updateReadOnlyFrontmatter( + content: string, + value: boolean, +): string | null { + const frontmatter = /^---\r?\n[\s\S]*?\r?\n---/.exec(content); + if (!frontmatter) throw new Error("Memory file is missing frontmatter."); + + const field = /^read_only\s*:\s*(.*?)\s*$/m.exec(frontmatter[0]); + if (field && field[1] !== "true" && field[1] !== "false") { + throw new Error("Memory file read_only must be true or false."); + } + if (field?.[1] === String(value)) return null; + if (field) { + return content.replace( + /^read_only\s*:\s*(?:true|false)\s*$/m, + `read_only: ${value}`, + ); + } + + const newline = frontmatter[0].includes("\r\n") ? "\r\n" : "\n"; + const closing = frontmatter.index + frontmatter[0].length - 3; + return `${content.slice(0, closing)}read_only: ${value}${newline}${content.slice(closing)}`; +} + +function resolveMemoryFile(memoryDir: string, input: string) { + if (isAbsolute(input)) throw new Error("Memory path must be relative."); + const absolutePath = resolve(memoryDir, input); + const relativePath = relative(memoryDir, absolutePath).replace(/\\/g, "/"); + if (!/^(system|reference)\/.+\.md$/.test(relativePath)) { + throw new Error( + "Memory path must be a .md file under system/ or reference/.", + ); + } + if (!existsSync(absolutePath)) + throw new Error(`Memory file not found: ${input}`); + if ( + relative(realpathSync(memoryDir), realpathSync(absolutePath)).startsWith( + "..", + ) + ) { + throw new Error("Memory file resolves outside the memory directory."); + } + return { absolutePath, relativePath }; +} + +export async function runMemoryReadOnlyAction(args: { + agentId: string; + memoryDir: string; + path?: string; + value?: string; + extraPositionals: string[]; +}): Promise { + if (!args.path || !args.value || args.extraPositionals.length) { + console.error("Usage: letta memory read-only "); + return 1; + } + if (args.value !== "true" && args.value !== "false") { + console.error("read-only value must be true or false."); + return 1; + } + + await assertMemoryRepoCleanForWrite(args.memoryDir); + const file = resolveMemoryFile(args.memoryDir, args.path); + const original = readFileSync(file.absolutePath, "utf8"); + const desired = args.value === "true"; + const updated = updateReadOnlyFrontmatter(original, desired); + if (updated === null) return 0; + + writeFileSync(file.absolutePath, updated, "utf8"); + const previousApproval = process.env.LETTA_APPROVED_READ_ONLY_CHANGE; + process.env.LETTA_APPROVED_READ_ONLY_CHANGE = "1"; + try { + const result = await commitMemoryWrite({ + memoryDir: args.memoryDir, + pathspecs: [file.relativePath], + reason: `${desired ? "Mark" : "Unmark"} ${file.relativePath} as read-only`, + author: { + agentId: args.agentId, + authorName: args.agentId, + authorEmail: `${args.agentId}@letta.com`, + }, + syncMode: isLocalBackendEnvEnabled() ? "local" : "remote", + }); + if (!result.committed) { + writeFileSync(file.absolutePath, original, "utf8"); + return 1; + } + console.log( + JSON.stringify({ ...result, path: file.relativePath, readOnly: desired }), + ); + return 0; + } catch (error) { + writeFileSync(file.absolutePath, original, "utf8"); + throw error; + } finally { + if (previousApproval === undefined) + delete process.env.LETTA_APPROVED_READ_ONLY_CHANGE; + else process.env.LETTA_APPROVED_READ_ONLY_CHANGE = previousApproval; + } +} diff --git a/src/cli/subcommands/memory.ts b/src/cli/subcommands/memory.ts index 34154cd0bc..8f54ca00bb 100644 --- a/src/cli/subcommands/memory.ts +++ b/src/cli/subcommands/memory.ts @@ -5,6 +5,7 @@ import { parseArgs } from "node:util"; import { getScopedMemoryFilesystemRoot } from "@/agent/memory-filesystem"; import { getMemoryGitStatus, isGitRepo, pullMemory } from "@/agent/memory-git"; import { isLocalBackendEnvEnabled } from "@/backend/local/paths"; +import { runMemoryReadOnlyAction } from "./memory-read-only"; import { runMemoryTokensAction } from "./memory-tokens"; function printUsage(): void { @@ -18,6 +19,7 @@ Usage: letta memory restore --from --force [--agent ] letta memory export --agent --out letta memory pull [--agent ] + letta memory read-only [--agent ] letta memory tokens [--memory-dir ] [--agent ] [--top ] [--format text|json] [--quiet] @@ -34,6 +36,7 @@ Examples: letta memory backup --agent agent-123 letta memory export --agent agent-123 --out /tmp/letta-memory-agent-123 letta memory tokens + letta memory read-only system/persona.md true --agent agent-123 letta memory tokens --memory-dir ~/.letta/agents/agent-123/memory --format json `.trim(), ); @@ -132,7 +135,8 @@ export async function runMemorySubcommand(argv: string[]): Promise { return 1; } - const [action] = parsed.positionals; + const [action, firstPositional, secondPositional, ...extraPositionals] = + parsed.positionals; if (parsed.values.help || !action || action === "help") { printUsage(); @@ -161,6 +165,16 @@ export async function runMemorySubcommand(argv: string[]): Promise { } try { + if (action === "read-only") { + return runMemoryReadOnlyAction({ + agentId, + memoryDir: getMemoryRoot(agentId), + path: firstPositional, + value: secondPositional, + extraPositionals, + }); + } + if (action === "status") { if (!isGitRepo(agentId)) { console.log( diff --git a/src/permissions/checker.ts b/src/permissions/checker.ts index 790f6c8fd4..f61df29d34 100644 --- a/src/permissions/checker.ts +++ b/src/permissions/checker.ts @@ -19,6 +19,7 @@ import type { PermissionModeState } from "@/tools/permission-mode-state"; import { canonicalToolName, isShellToolName } from "./canonical"; import { cliPermissions } from "./cli-permissions-instance"; import { evaluateCrossAgentGuard, extractFilePath } from "./cross-agent-guard"; +import { envFlagEnabled, getMandatoryApproval } from "./mandatory-approval"; import { type MatcherOptions, matchesBashPattern, @@ -109,12 +110,6 @@ interface ModPermissionCheckOptions { toolCallId?: string | null; } -function envFlagEnabled(name: string): boolean { - const value = process.env[name]; - if (!value) return false; - return value === "1" || value.toLowerCase() === "true"; -} - function isPermissionsV2Enabled(): boolean { const value = process.env.LETTA_PERMISSIONS_V2; if (!value) return true; @@ -362,6 +357,9 @@ function checkPermissionForEngine( } } + const mandatoryApproval = getMandatoryApproval(canonicalTool, toolArgs); + if (mandatoryApproval) return { result: mandatoryApproval, trace }; + if (sessionRules.alwaysAsk) { for (const pattern of sessionRules.alwaysAsk) { const matched = matchesPattern( diff --git a/src/permissions/mandatory-approval.ts b/src/permissions/mandatory-approval.ts new file mode 100644 index 0000000000..cc3d35c2cd --- /dev/null +++ b/src/permissions/mandatory-approval.ts @@ -0,0 +1,37 @@ +import type { PermissionCheckResult } from "./types"; + +type ToolArgs = Record; + +export function envFlagEnabled(name: string): boolean { + const value = process.env[name]; + if (!value) return false; + return value === "1" || value.toLowerCase() === "true"; +} + +function extractShellCommand(toolArgs: ToolArgs): string | null { + const command = + typeof toolArgs.cmd === "string" ? toolArgs.cmd : toolArgs.command; + if (typeof command === "string") return command; + return Array.isArray(command) ? command.join(" ") : null; +} + +export function getMandatoryApproval( + canonicalTool: string, + toolArgs: ToolArgs, +): PermissionCheckResult | null { + if (canonicalTool !== "Bash") return null; + const command = extractShellCommand(toolArgs); + if ( + !command || + /(?:^|[\s;&|'"])(?:[^\s;&|]*\/)?letta(?:\.js)?['"]?\s+(?:memory|memfs)\s+read-only(?:\s|$)/.exec( + command, + ) === null + ) { + return null; + } + return { + decision: "alwaysAsk", + matchedRule: "Bash(letta memory read-only:*)", + reason: "Changing memory read-only status requires explicit user approval", + }; +}