Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions assistant/src/__tests__/checker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@ import {
import { homedir, tmpdir } from "node:os";
import { join, resolve } from "node:path";
import {
afterAll,
afterEach,
beforeAll,
beforeEach,
describe,
expect,
mock,
spyOn,
test,
} from "bun:test";

Expand Down Expand Up @@ -72,9 +74,19 @@ mock.module("../config/loader.js", () => ({
// Defaults to null so existing tests see no extra rules, matching the
// behaviour on a fresh install without a resolved guardian.
let mockGuardianPersonaPath: string | null = null;
mock.module("../prompts/persona-resolver.js", () => ({
resolveGuardianPersonaPath: () => mockGuardianPersonaPath,
}));

// Spy on the namespace import rather than using `mock.module`. Bun's
// `mock.module` is a persistent process-wide override that would clobber
// every other export (e.g. `ensureGuardianPersonaFile`,
// `isGuardianPersonaCustomized`) and break unrelated test files
// (persona-resolver.test.ts) when run in the same bun test invocation.
// `spyOn` with `mockRestore()` in afterAll restores the original
// implementation so other test files see the real exports.
import * as personaResolver from "../prompts/persona-resolver.js";
const guardianPathSpy = spyOn(
personaResolver,
"resolveGuardianPersonaPath",
).mockImplementation(() => mockGuardianPersonaPath);

import {
check,
Expand Down Expand Up @@ -149,6 +161,13 @@ function writeSkill(
);
}

// Restore the guardian persona spy at the end of this file's run so
// subsequent test files (e.g. persona-resolver.test.ts) see the real
// implementation when they import from the module namespace.
afterAll(() => {
guardianPathSpy.mockRestore();
});

describe("Permission Checker", () => {
beforeAll(async () => {
// Warm up the shell parser (loads WASM)
Expand Down
26 changes: 13 additions & 13 deletions assistant/src/__tests__/persona-resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,12 @@ describe("resolveGuardianPersonaPath", () => {

describe("ensureGuardianPersonaFile", () => {
test("writes the template when the file is missing", () => {
const slug = "sidd.md";
const filePath = join(mockWorkspaceDir, "users", slug);
const userFile = "sidd.md";
const filePath = join(mockWorkspaceDir, "users", userFile);

expect(existsSync(filePath)).toBe(false);

ensureGuardianPersonaFile(slug);
ensureGuardianPersonaFile(userFile);

expect(existsSync(filePath)).toBe(true);
const content = readFileSync(filePath, "utf-8");
Expand All @@ -133,15 +133,15 @@ describe("ensureGuardianPersonaFile", () => {
});

test("is a no-op when the file already exists (does not clobber)", () => {
const slug = "sidd.md";
const userFile = "sidd.md";
const dir = join(mockWorkspaceDir, "users");
const filePath = join(dir, slug);
const filePath = join(dir, userFile);
const existingContent = "# Existing user notes\n\n- Likes sparkling water\n";

mkdirSync(dir, { recursive: true });
writeFileSync(filePath, existingContent, "utf-8");

ensureGuardianPersonaFile(slug);
ensureGuardianPersonaFile(userFile);

const content = readFileSync(filePath, "utf-8");
expect(content).toBe(existingContent);
Expand All @@ -157,20 +157,20 @@ describe("isGuardianPersonaCustomized", () => {
});

test("returns false for the bare scaffold template (no user edits)", () => {
const slug = "sidd.md";
const filePath = join(mockWorkspaceDir, "users", slug);
const userFile = "sidd.md";
const filePath = join(mockWorkspaceDir, "users", userFile);

// ensureGuardianPersonaFile writes the canonical template — the
// exact bytes that "not customized" accepts.
ensureGuardianPersonaFile(slug);
ensureGuardianPersonaFile(userFile);

expect(isGuardianPersonaCustomized(filePath)).toBe(false);
});

test("returns false when the file contains only comment lines", () => {
const slug = "sidd.md";
const userFile = "sidd.md";
const dir = join(mockWorkspaceDir, "users");
const filePath = join(dir, slug);
const filePath = join(dir, userFile);

mkdirSync(dir, { recursive: true });
writeFileSync(
Expand All @@ -183,9 +183,9 @@ describe("isGuardianPersonaCustomized", () => {
});

test("returns true when the file has user-authored content", () => {
const slug = "sidd.md";
const userFile = "sidd.md";
const dir = join(mockWorkspaceDir, "users");
const filePath = join(dir, slug);
const filePath = join(dir, userFile);

mkdirSync(dir, { recursive: true });
writeFileSync(
Expand Down
26 changes: 19 additions & 7 deletions assistant/src/contacts/contacts-write.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import type { ChannelId } from "../channels/types.js";
import type { GuardianBinding } from "../memory/channel-verification-sessions.js";
import { clearCache as clearTrustCache } from "../permissions/trust-store.js";
import { ensureGuardianPersonaFile } from "../prompts/persona-resolver.js";
import { canonicalizeInboundIdentity } from "../util/canonicalize-identity.js";
import { getLogger } from "../util/logger.js";
Expand Down Expand Up @@ -98,8 +99,20 @@ export function createGuardianBinding(params: {
// Seed the per-user persona file so downstream readers (journaling,
// persona resolution) can rely on `users/<slug>.md` existing on disk.
// Idempotent: pre-existing customized files are preserved.
//
// Seeding is restricted to the guardian-creation path only — it must
// NOT run from inbound-message upsertContactChannel calls, since the
// `users/` directory watcher would fire on every new contact and
// evict live conversations.
if (contact.userFile) {
ensureGuardianPersonaFile(contact.userFile);
// Invalidate the trust rule cache so the dynamic guardian-persona
// auto-allow rules from `permissions/defaults.ts` are backfilled on
// the next `getRules()` call. Without this, guardians created at
// runtime (self-heal paths, first-message-seeds-guardian) wouldn't
// get their auto-allow rule until the daemon restarts, and the
// model would prompt on its first `file_edit users/<slug>.md`.
clearTrustCache();
}

const now = Date.now();
Expand Down Expand Up @@ -193,7 +206,7 @@ export function upsertContactChannel(params: {
) ?? params.externalUserId)
: null;

const contact = upsertContact({
upsertContact({
id: params.contactId,
displayName,
role: params.role,
Expand All @@ -218,12 +231,11 @@ export function upsertContactChannel(params: {
reassignConflictingChannels: !!params.contactId,
});

// Seed the per-user persona file so downstream readers (journaling,
// persona resolution) can rely on `users/<slug>.md` existing on disk.
// Idempotent: pre-existing customized files are preserved.
if (contact.userFile) {
ensureGuardianPersonaFile(contact.userFile);
}
// NOTE: We intentionally do NOT seed `users/<slug>.md` here. This is the
// inbound-message hot path — every new contact (Slack, phone, email, etc)
// would otherwise fire the `users/` directory watcher in
// config-watcher.ts and evict live conversations. Persona-file seeding
// is the sole responsibility of `createGuardianBinding`.

const contactResult = findContactChannel({
channelType: params.sourceChannel,
Expand Down
7 changes: 5 additions & 2 deletions assistant/src/permissions/defaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,11 @@ export function getDefaultRuleTemplates(): DefaultRuleTemplate[] {
}));
}
} catch {
// Guardian may not exist yet; rules will be emitted on the next
// template build once the guardian contact is created.
// If guardian resolution fails at default-rule computation, the rule
// will be missing until the trust cache is invalidated and rebuilt.
// Runtime guardian-creation paths invalidate the cache via
// `clearTrustCache()` in `contacts-write.createGuardianBinding` so
// that the next `getRules()` call picks up the new auto-allow rule.
}

const bootstrapDeleteRule: DefaultRuleTemplate = {
Expand Down
18 changes: 11 additions & 7 deletions assistant/src/prompts/persona-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,23 +237,27 @@ export function resolveGuardianPersona(): string | null {
}

/**
* Write the guardian persona template scaffold to `users/<slug>.md`
* Write the guardian persona template scaffold to `users/<userFile>`
* when the file does not yet exist. No-op when the file already
* exists (safe against clobbering user edits).
*
* Rejects slugs that fail basename validation (path traversal guard).
* @param userFile - A filename (not a bare slug), matching the shape
* of `Contact.userFile` — a basename with a `.md` suffix
* (e.g. `"sidd.md"`). The path traversal guard rejects values that
* are not a clean basename.
*
* Creates the parent `users/` directory if missing.
*/
export function ensureGuardianPersonaFile(slug: string): void {
if (basename(slug) !== slug || slug === ".." || slug === ".") {
export function ensureGuardianPersonaFile(userFile: string): void {
if (basename(userFile) !== userFile || userFile === ".." || userFile === ".") {
log.warn(
{ slug },
"Guardian persona slug contains path traversal; refusing to write",
{ userFile },
"Guardian persona userFile contains path traversal; refusing to write",
);
return;
}

const filePath = join(getWorkspaceDir(), "users", slug);
const filePath = join(getWorkspaceDir(), "users", userFile);
if (existsSync(filePath)) return;

mkdirSync(dirname(filePath), { recursive: true });
Expand Down
Loading