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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Unreleased

## Bug fixes
- Config: a child config layer (e.g. a project `.tuor/config.json`) that didn't
set `workdir` or `user` silently reset the value inherited from an upper layer
layer (e.g. `~/.config/tuor/config.json`) to the built-in default values
(workdir: `/`, user: `root`). These fields now fall through to the parent
layer when omitted; their defaults are applied only after all layers are
merged.


# 0.3.0 (2026-07-08)

Expand Down
28 changes: 27 additions & 1 deletion src/config/defaults.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,37 @@ import { describe, expect, test } from "vitest";
import { applyConfigDefaults } from "./defaults.ts";
import type { TuorConfig } from "./schema.ts";

/**
* A merged config as it reaches applyConfigDefaults: user/workdir are optional
* (no schema default) and only present when a layer set them explicitly.
*/
function config(overrides: Partial<TuorConfig> = {}): TuorConfig {
return { user: "root", workdir: "/", ...overrides };
return { ...overrides };
}

describe("applyConfigDefaults", () => {
describe("user / workdir", () => {
test("defaults user to root when omitted", () => {
expect(applyConfigDefaults(config()).user).toBe("root");
});

test("defaults workdir to / when omitted", () => {
expect(applyConfigDefaults(config()).workdir).toBe("/");
});

test("preserves an explicit user and workdir", () => {
const result = applyConfigDefaults(
config({ user: "dev", workdir: "/w" }),
);
expect(result.user).toBe("dev");
expect(result.workdir).toBe("/w");
});

test("infers guestHomeDir from the defaulted user when user is omitted", () => {
expect(applyConfigDefaults(config()).guestHomeDir).toBe("/root");
});
});

describe("network", () => {
test("defaults omitted network to restricted with empty allowlists", () => {
const result = applyConfigDefaults(config());
Expand Down
29 changes: 24 additions & 5 deletions src/config/defaults.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import type { NetworkSpec } from "../core/session.ts";
import { inferGuestHomeDir } from "./homedir.ts";
import type { TuorConfig } from "./schema.ts";
import type { TuorConfig, WorkdirConfig } from "./schema.ts";

// --- Types ---

/** Guest user assumed when no config layer sets one. */
export const DEFAULT_USER = "root";
/** Guest working directory assumed when no config layer sets one. */
export const DEFAULT_WORKDIR: WorkdirConfig = "/";

/**
* A {@link TuorConfig} with all *config-level* defaults materialized. This is
* the "effective config" a user reasons about: same shape as `config.json`
Expand All @@ -15,27 +20,41 @@ import type { TuorConfig } from "./schema.ts";
* guarantees the default was applied; its JSON shape is identical to a
* fully-populated `NetworkConfig`.
*/
export type DefaultedConfig = Omit<TuorConfig, "network" | "guestHomeDir"> & {
export type DefaultedConfig = Omit<
TuorConfig,
"network" | "guestHomeDir" | "user" | "workdir"
> & {
network: NetworkSpec;
guestHomeDir: string;
user: string;
workdir: WorkdirConfig;
};

// --- Public API ---

/**
* Fill the config-level defaults that don't come from the arktype schema:
* `network` (block-all when omitted) and `guestHomeDir` (inferred from `user`).
* `user`, `workdir`, `network` (block-all when omitted) and `guestHomeDir`
* (inferred from `user`).
*
* `user`/`workdir` are defaulted *here* rather than in the schema so that a
* child config layer that omits them doesn't clobber a value inherited from a
* parent layer during merge (their "omitted" would otherwise be indistinguish-
* able from an explicit default). This must run after `mergeConfigs`.
*
* Pure and dependency-free. Schema defaults (`user`, `workdir`, mount `mode`,
* Pure and dependency-free. The remaining schema defaults (mount `mode`,
* `nixLd`) are already applied by `parseConfig`; computed conversions (path
* expansion, env/secret split, nix→mounts, overlay state dirs, …) are *not*
* defaults and stay in {@link createSessionSpecFromConfig}.
*/
export function applyConfigDefaults(config: TuorConfig): DefaultedConfig {
const user = config.user ?? DEFAULT_USER;
return {
...config,
user,
workdir: config.workdir ?? DEFAULT_WORKDIR,
network: defaultNetwork(config.network),
guestHomeDir: config.guestHomeDir ?? inferGuestHomeDir(config.user),
guestHomeDir: config.guestHomeDir ?? inferGuestHomeDir(user),
};
}

Expand Down
34 changes: 30 additions & 4 deletions src/config/merge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@ import { describe, expect, test } from "vitest";
import { type ConfigLayer, findAllConfigDirs, mergeConfigs } from "./merge.ts";
import type { MountConfig, TuorConfig } from "./schema.ts";

/** Minimal valid config (matching what parseConfig returns with defaults filled). */
/**
* Minimal valid config, matching what parseConfig returns: user/workdir carry
* no schema default, so an unset layer genuinely omits them (they're defaulted
* post-merge in applyConfigDefaults).
*/
function config(overrides: Partial<TuorConfig> = {}): TuorConfig {
return { user: "root", workdir: "/", ...overrides };
return { ...overrides };
}

function layer(
Expand Down Expand Up @@ -104,6 +108,27 @@ describe("mergeConfigs", () => {
expect(result.workdir).toBe("/child-dir");
});

// Regression: a child layer that omits user/workdir must inherit the
// parent's value rather than clobbering it. Previously these fields carried
// a schema default ("root"/"/"), so a child that never set them still
// overrode the parent — e.g. a project .tuor/config.json silently reset a
// workdir configured in ~/.config/tuor/config.json.
test("user: parent used when child omits", () => {
const result = mergeConfigs([
layer("/a", { user: "parent" }),
layer("/b"),
]);
expect(result.user).toBe("parent");
});

test("workdir: parent used when child omits", () => {
const result = mergeConfigs([
layer("/a", { workdir: "/parent-dir" }),
layer("/b"),
]);
expect(result.workdir).toBe("/parent-dir");
});

test("rootfsSize: child overrides parent", () => {
const result = mergeConfigs([
layer("/a", { rootfsSize: "1G" }),
Expand Down Expand Up @@ -409,8 +434,9 @@ describe("mergeConfigs", () => {
}),
]);

// Scalars: most specific child wins (child has default "root" from helper)
expect(result.user).toBe("root");
// Scalars: most specific layer that *sets* the field wins. The closest
// layer omits user, so it falls through to the middle layer.
expect(result.user).toBe("project-user");

// Env: shallow merge
expect(result.env).toEqual({
Expand Down
10 changes: 7 additions & 3 deletions src/config/merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,13 @@ export function mergeConfigs(layers: ConfigLayer[]): TuorConfig {
/** Merge two configs where `child` overrides `parent`. */
function mergeTwoConfigs(parent: TuorConfig, child: TuorConfig): TuorConfig {
return {
// Scalars: child wins (fall back to parent for defaults)
user: child.user,
workdir: child.workdir,
// Scalars: child wins, falling back to parent when the child omits the
// field. user/workdir carry no schema default anymore, so "omitted" is
// genuinely undefined here — otherwise a child layer's silently-defaulted
// value would clobber a value inherited from a parent layer. Their defaults
// are applied post-merge in applyConfigDefaults.
...lastDefined(child.user, parent.user, "user"),
...lastDefined(child.workdir, parent.workdir, "workdir"),
...lastDefined(child.guestHomeDir, parent.guestHomeDir, "guestHomeDir"),
...lastDefined(child.rootfsSize, parent.rootfsSize, "rootfsSize"),
...lastDefined(child.nix, parent.nix, "nix"),
Expand Down
8 changes: 5 additions & 3 deletions src/config/schema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,12 @@ describe("parseConfig", () => {
});
});

test("fills in defaults for minimal config", () => {
test("leaves optional fields unset for minimal config", () => {
// user/workdir carry no schema default (they're defaulted post-merge in
// applyConfigDefaults), so parseConfig leaves them undefined here.
const config = parseConfig({});
expect(config.user).toBe("root");
expect(config.workdir).toBe("/");
expect(config.user).toBeUndefined();
expect(config.workdir).toBeUndefined();
expect(config.network).toBeUndefined();
expect(config.mounts).toBeUndefined();
expect(config.nix).toBeUndefined();
Expand Down
14 changes: 11 additions & 3 deletions src/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,12 @@ const types = scope({
* The user to open the shell under and to make mounted directories
* available for. Must currently be root due to Gondolin-related
* constraints.
*
* Optional (no schema default): the "root" default is applied post-merge in
* applyConfigDefaults, so an inherited value isn't clobbered by a child
* layer that merely omitted the field.
*/
user: "string > 0 = 'root'",
"user?": "string > 0",

/**
* Volumes are host-backed, initially empty directories that the VM guest
Expand All @@ -61,9 +65,13 @@ const types = scope({
"volumes?": "VolumeConfig[]",

/**
* Configure the default working directory for the VM guest
* Configure the default working directory for the VM guest.
*
* Optional (no schema default): the "/" default is applied post-merge in
* applyConfigDefaults, so an inherited value isn't clobbered by a child
* layer that merely omitted the field.
*/
workdir: "WorkdirConfig = '/'",
"workdir?": "WorkdirConfig",
},

// --------------------------------------------------------------------------
Expand Down