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
12 changes: 12 additions & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,18 @@ For releases that warrant narrative — redesigns, breaking changes, migration s
- **SDK UI:** the materialization detail dialog drops the "Mode" field and now renders its build-plan view from `Package.buildPlan`.
- Regenerate any SDK/Python/k6 clients against the updated `api-doc.yaml`.

## [Unreleased] — Package locations: `~/` expands, and relative paths anchor at the config

**A relative package `location` now resolves against the directory holding the config it appears in, not the server root.** Those are the same directory whenever the config is found at `<SERVER_ROOT>/publisher.config.json`, which covers the bundled samples, every Docker recipe in [docs/deployment.md](docs/deployment.md), and any setup that `cd`s to the config before starting. Nothing changes for them. Two cases keep the server root as the anchor: the config bundled inside the published package (a zero-arg `npx @malloy-publisher/server`), and a `--config` naming a directory rather than a file.

**Who is affected:** anyone whose `--config <path>` names a file in a directory other than the server root, including a subdirectory of it, and whose packages use a relative `location`. Those packages previously resolved against the server root (the working directory, unless `--server_root` was also passed) and now resolve next to the config. Fix either way: make the `location` absolute, or move the config next to the packages it points at, which is the arrangement this change exists to support.

**The symptom is quiet.** A location that cannot be mounted is not fatal to the process: the server still reports `serving`. It does fail the whole environment the location belongs to, so that environment is skipped and none of its packages load, including the ones that resolved fine. The reason is in the log: `Error initializing environment "<name>"; skipping environment`.

**`~/` in a `location` now works.** It was accepted and then never expanded, so it resolved to a literal `~` directory under the server root and failed to mount. Expansion is unconditional and happens before any anchor applies.

See [docs/configuration.md](docs/configuration.md) for the rule and the recommended layout.

## [Unreleased] — Source access gates (`#(authorize)`)

**Sources can now gate query access on givens.** A `#(authorize) "<bool expr>"` annotation (source-level) or `##(authorize)` (file-level) is evaluated against the request's [givens](docs/givens.md) before any query that reads the source runs; access is denied with **HTTP 403** unless at least one in-scope expression is `true` (OR semantics). Enforced on `POST /…/query`, the notebook-cell `GET`, `POST /…/compile`, and the MCP `malloy_executeQuery` tool. Malformed or invalid annotations fail model load with **424**.
Expand Down
10 changes: 5 additions & 5 deletions bun.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 31 additions & 1 deletion docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,35 @@ npx @malloy-publisher/server --port 4000 --config /path/to/your/publisher.config
cd /path/to/your/project && npx @malloy-publisher/server --port 4000
```

Your packages can live anywhere; they do not have to be inside this repo. A package `location` may be
absolute, start with `~/`, or be relative to the directory holding the config it appears in. Keeping
a config next to the packages it points at means the two move together, which is what makes the
config worth committing:

```
my-data/
publisher.config.json
sales/ # your package: publisher.json + sales.malloy
```

```json
{
"environments": [
{
"name": "local",
"packages": [{ "name": "sales", "location": "./sales" }]
}
]
}
```

Do not author packages under `publisher_data/`. That is storage Publisher manages for itself: it
copies each configured package in (or symlinks it, under `--watch-env`), and `--init` deletes the
whole tree.

A `location` can also be a `https://github.com/...`, `gs://`, or `s3://` URL, which Publisher
downloads. Only local directories are eligible for `--watch-env`.

To add a BigQuery-backed sample (`bigquery-hackernews`) alongside the bundled examples, copy
[`packages/server/publisher.config.example.bigquery.json`](../packages/server/publisher.config.example.bigquery.json)
over your `publisher.config.json` and set `GOOGLE_APPLICATION_CREDENTIALS`. For the database
Expand All @@ -33,7 +62,8 @@ connection reference (BigQuery, Snowflake, Postgres, DuckDB, and more), see
| `PUBLISHER_PORT` | `--port <n>` | `4000` | REST + static-app HTTP port. |
| `PUBLISHER_HOST` | `--host <addr>` | `0.0.0.0` | Host binding for both the REST and MCP servers. Set `127.0.0.1` to keep them loopback-only. |
| `MCP_PORT` | `--mcp_port <n>` | `4040` | MCP HTTP port. Serves the five MCP tools (`malloy_getContext`, `malloy_executeQuery`, `malloy_compile`, `malloy_reloadPackage`, `malloy_searchDocs`) and the agent skills as MCP prompts. |
| `SERVER_ROOT` | `--server_root <dir>` | `.` (cwd) | Directory containing `publisher.config.json`. |
| `SERVER_ROOT` | `--server_root <dir>` | `.` (cwd) | Where Publisher keeps its own storage (`publisher_data/`, `publisher.db`), and where it looks for `publisher.config.json` when `--config` is not passed. |
| `PUBLISHER_USE_BUNDLED_DEFAULT` | — | _unset_ | Set to `true` to fall back to the sample config bundled inside the installed package when neither `--config` is passed nor a `publisher.config.json` exists at the server root. The server sets this itself on a zero-flag start (so a bare `npx @malloy-publisher/server` boots the samples); passing `--config` or `--server_root` leaves it unset. Because the bundled config lives inside the install, relative package locations resolve against the server root in this mode rather than the config's directory. |
| `INITIALIZE_STORAGE` | `--init` | _unset_ | Set to `true` (or pass `--init`) to **wipe persisted storage** (`publisher_data/`) and re-sync it from the config on boot. A first boot with empty storage loads the config automatically, so you only need this to reset state or pick up config changes. Also exposed as the `start:init` / `start:dev:init` scripts. |
| `SHUTDOWN_DRAIN_DURATION_SECONDS` | `--shutdown_drain_duration_seconds <s>` | `0` | After SIGTERM, how long to keep serving in-flight and new requests (readiness reports not-ready immediately) before the server starts refusing new traffic. |
| `SHUTDOWN_GRACEFUL_CLOSE_TIMEOUT_SECONDS` | `--shutdown_graceful_close_timeout_seconds <s>` | `0` | Time to wait for in-flight requests to drain before forcing close. |
Expand Down
3 changes: 2 additions & 1 deletion packages/server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@
"node-cron": "^3.0.3",
"recursive-readdir": "^2.2.3",
"simple-git": "^3.28.0",
"uuid": "^11.0.3"
"uuid": "^11.0.3",
"winston": "^3.17.0"
},
"devDependencies": {
"@eslint/compat": "^1.2.7",
Expand Down
88 changes: 87 additions & 1 deletion packages/server/src/config.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { afterEach, beforeEach, describe, expect, it } from "bun:test";
import fs from "fs";
import path from "path";
import { getPublisherConfig, type PublisherConfig } from "./config";
import {
getPublisherConfig,
getPublisherConfigDir,
type PublisherConfig,
} from "./config";
import { PUBLISHER_CONFIG_NAME } from "./constants";

describe("Config Environment Variable Substitution", () => {
Expand Down Expand Up @@ -1408,3 +1412,85 @@ describe("getMaxConcurrentQueries", () => {
expect(() => getMaxConcurrentQueries()).toThrow();
});
});

describe("getPublisherConfigDir", () => {
const testRoot = path.join(process.cwd(), "test-temp-config-dir");
const elsewhere = path.join(testRoot, "elsewhere");
const savedConfigPath = process.env.PUBLISHER_CONFIG_PATH;

beforeEach(() => {
fs.mkdirSync(elsewhere, { recursive: true });
// Both knobs feed resolvePublisherConfigPath, and every spec file shares
// one process, so clear both rather than inherit whatever ran before.
delete process.env.PUBLISHER_CONFIG_PATH;
delete process.env.PUBLISHER_USE_BUNDLED_DEFAULT;
});

afterEach(() => {
fs.rmSync(testRoot, { recursive: true, force: true });
// Clear both, not just the one this describe saves: every spec file shares
// one process, so a knob left set here follows whatever runs next.
delete process.env.PUBLISHER_USE_BUNDLED_DEFAULT;
if (savedConfigPath === undefined) {
delete process.env.PUBLISHER_CONFIG_PATH;
} else {
process.env.PUBLISHER_CONFIG_PATH = savedConfigPath;
}
});

it("returns the server root when the config sits there", () => {
fs.writeFileSync(
path.join(testRoot, PUBLISHER_CONFIG_NAME),
JSON.stringify({ environments: [] }),
);
expect(getPublisherConfigDir(testRoot)).toBe(testRoot);
});

it("follows PUBLISHER_CONFIG_PATH to a config outside the server root", () => {
// The --config case: a relative package location must anchor next to
// the config that declares it, not at whatever cwd the server booted in.
const configPath = path.join(elsewhere, PUBLISHER_CONFIG_NAME);
fs.writeFileSync(configPath, JSON.stringify({ environments: [] }));
process.env.PUBLISHER_CONFIG_PATH = configPath;
expect(getPublisherConfigDir(testRoot)).toBe(elsewhere);
});

it("returns null when no config resolves, so callers fall back to the server root", () => {
expect(getPublisherConfigDir(testRoot)).toBeNull();
});

it("returns an absolute anchor even when --config is relative", () => {
// An anchor that is itself relative would re-resolve against the cwd of
// whoever reads it, which is the bug this whole change is fixing.
const rel = path.relative(
process.cwd(),
path.join(elsewhere, PUBLISHER_CONFIG_NAME),
);
fs.writeFileSync(
path.join(elsewhere, PUBLISHER_CONFIG_NAME),
JSON.stringify({ environments: [] }),
);
process.env.PUBLISHER_CONFIG_PATH = rel;
const anchor = getPublisherConfigDir(testRoot);
expect(anchor).not.toBeNull();
expect(path.isAbsolute(anchor as string)).toBe(true);
expect(anchor).toBe(elsewhere);
});

it("returns null when --config names a directory instead of a file", () => {
// existsSync is true for a directory, so without a file check the anchor
// would silently become that directory's PARENT, for a config that never
// loads (getPublisherConfig catches the read error and returns empty).
process.env.PUBLISHER_CONFIG_PATH = elsewhere;
expect(getPublisherConfigDir(testRoot)).toBeNull();
});

it("returns null for the bundled default rather than anchoring inside the install", () => {
// Zero-arg `npx @malloy-publisher/server` resolves the config that ships
// inside the package. Anchoring a user's relative location under
// node_modules would be meaningless, so the caller falls back to the
// server root, which is what this did before the anchor moved.
process.env.PUBLISHER_USE_BUNDLED_DEFAULT = "true";
expect(getPublisherConfigDir(testRoot)).toBeNull();
});
});
37 changes: 36 additions & 1 deletion packages/server/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,13 @@ function resolvePublisherConfigPath(serverRoot: string): {
return null;
}

type FilesystemPath = `./${string}` | `../${string}` | `/${string}`;
// Home paths are POSIX form only: `~/` expands, while bare `~`, `~user/…`,
// and Windows-style `~\` are not local paths and are rejected downstream.
type FilesystemPath =
| `./${string}`
| `../${string}`
| `/${string}`
| `~/${string}`;
type GcsPath = `gs://${string}`;
type ApiConnection = components["schemas"]["Connection"];
export type Theme = components["schemas"]["Theme"];
Expand Down Expand Up @@ -404,6 +410,35 @@ function processConfigValue(value: unknown): unknown {
return value;
}

/**
* Absolute directory that a relative package `location` is resolved against:
* the one holding the active config file.
*
* Null when there is nothing sensible to anchor to, leaving the caller to pick
* a base. That covers three cases: no config at all; the bundled default, which
* lives inside the installed package, where anchoring a user's relative path
* somewhere under node_modules would be meaningless (and which declares only
* remote locations of its own); and a `--config` that names a directory rather
* than a file, which cannot be read as a config, so its parent is not an anchor
* anyone asked for.
*/
export const getPublisherConfigDir = (serverRoot: string): string | null => {
const resolved = resolvePublisherConfigPath(serverRoot);
if (!resolved || resolved.isBundledDefault) {
return null;
}
try {
if (!fs.statSync(resolved.path).isFile()) {
return null;
}
} catch {
return null;
}
// Resolve: `--config` may be relative, and an anchor that is itself relative
// would re-resolve against the cwd of whoever reads it.
return path.resolve(path.dirname(resolved.path));
};

export const getPublisherConfig = (serverRoot: string): PublisherConfig => {
const resolved = resolvePublisherConfigPath(serverRoot);
if (!resolved) {
Expand Down
2 changes: 1 addition & 1 deletion packages/server/src/mcp/skills/skills_bundle.json

Large diffs are not rendered by default.

71 changes: 70 additions & 1 deletion packages/server/src/service/environment_store.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { isPublisherConfigFrozen } from "../config";
import { TEMP_DIR_PATH } from "../constants";
import { BadRequestError } from "../errors";
import { Environment } from "./environment";
import { EnvironmentStore } from "./environment_store";
import { EnvironmentStore, resolvePackageLocation } from "./environment_store";

type MockData = Record<string, unknown>;

Expand Down Expand Up @@ -1042,3 +1042,72 @@ describe("EnvironmentStore path-injection guards", () => {
);
});
});

describe("resolvePackageLocation", () => {
// Built with `path`, not literals: the helper joins with the platform
// separator and these specs run on windows-latest too (cross-platform-tests).
const HOME = path.join(path.sep, "home", "tester");
const CONFIG_DIR = path.join(path.sep, "etc", "publisher");

it("expands ~/ against the home directory", () => {
expect(
resolvePackageLocation("~/my-packages/sales", CONFIG_DIR, HOME),
).toBe(path.join(HOME, "my-packages", "sales"));
});

it("does not anchor an expanded ~/ path at the config dir", () => {
// Regression: `~/` satisfies isLocalPath but not path.isAbsolute, so an
// unexpanded tilde used to be joined onto the anchor, yielding
// `<anchor>/~/my-packages/sales`.
const resolved = resolvePackageLocation("~/x", CONFIG_DIR, HOME);
expect(resolved).not.toContain("~");
expect(resolved.startsWith(CONFIG_DIR)).toBe(false);
});

it("returns an absolute location untouched", () => {
const absolute = path.join(path.sep, "srv", "packages", "sales");
expect(resolvePackageLocation(absolute, CONFIG_DIR, HOME)).toBe(absolute);
});

it("anchors a relative location at the config dir, not the cwd", () => {
expect(resolvePackageLocation("./sales", CONFIG_DIR, HOME)).toBe(
path.join(CONFIG_DIR, "sales"),
);
});

it("anchors a ../ location at the config dir", () => {
expect(
resolvePackageLocation("../examples/storefront", CONFIG_DIR, HOME),
).toBe(path.join(CONFIG_DIR, "..", "examples", "storefront"));
});

it("throws when ~/ cannot be expanded because no home directory is set", () => {
// An empty home must fail loudly: `path.join("", "x")` would otherwise
// yield a relative "x" that silently anchors under the config dir.
expect(() => resolvePackageLocation("~/x", CONFIG_DIR, "")).toThrow(
/home directory is not set/,
);
});

it("never consults the home directory for non-tilde locations", () => {
// The same empty home that makes a ~/ location throw must be irrelevant
// to ./ and absolute locations; home is resolved lazily, tilde-only.
expect(resolvePackageLocation("./sales", CONFIG_DIR, "")).toBe(
path.join(CONFIG_DIR, "sales"),
);
const absolute = path.join(path.sep, "srv", "packages", "sales");
expect(resolvePackageLocation(absolute, CONFIG_DIR, "")).toBe(absolute);
});

it("expands only the POSIX ~/ prefix; bare ~ and ~user are not home paths", () => {
// Neither form satisfies isLocalPath, so in practice they are rejected
// upstream as non-local locations; pin here that the resolver itself
// never treats them as home-relative either.
expect(resolvePackageLocation("~", CONFIG_DIR, HOME)).toBe(
path.join(CONFIG_DIR, "~"),
);
expect(resolvePackageLocation("~user/foo", CONFIG_DIR, HOME)).toBe(
path.join(CONFIG_DIR, "~user", "foo"),
);
});
});
Loading
Loading