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
7 changes: 5 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ jobs:
registry: ${{ env.REGISTRY }}
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}
# load:true (PR path) cannot export manifest lists. Provenance/SBOM
# attestations produce multi-artifact images, so only enable them on
# push where we publish to the registry instead of loading locally.
- uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a # v7.3.0
id: build
with:
Expand All @@ -160,8 +163,8 @@ jobs:
org.opencontainers.image.title=Muster
org.opencontainers.image.description=Shared workspace for human and agent-driven security operations
org.opencontainers.image.source=${{ github.server_url }}/${{ github.repository }}
provenance: mode=max
sbom: true
provenance: ${{ github.event_name == 'push' && 'mode=max' || false }}
sbom: ${{ github.event_name == 'push' }}
cache-from: type=gha
cache-to: type=gha,mode=max
- name: Scan built image with Trivy
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/security.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,21 +67,25 @@ jobs:
scanners: vuln
exit-code: "1"
- uses: anchore/sbom-action@e22c389904149dbc22b58101806040fa8d37a610 # v0
if: always()
with:
image: muster:security
format: cyclonedx-json
output-file: muster-sbom.cdx.json
upload-artifact: false
- name: Generate provenance metadata and checksums
if: always()
run: |
printf '{"commit":"%s","workflow":"%s","runId":"%s","image":"muster:security"}\n' \
"$GITHUB_SHA" "$GITHUB_WORKFLOW" "$GITHUB_RUN_ID" > provenance.json
sha256sum muster-sbom.cdx.json provenance.json > SHA256SUMS
- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
if: always()
with:
name: security-artifacts
path: |
muster-sbom.cdx.json
provenance.json
SHA256SUMS
trivy.sarif
if-no-files-found: ignore
Comment on lines +77 to +91

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the relevant workflow region with line numbers.
sed -n '1,140p' .github/workflows/security.yml | cat -n

# Also locate all uses of upload-artifact in this workflow for context.
rg -n "upload-artifact|if-no-files-found|muster-sbom|trivy\.sarif|provenance\.json|SHA256SUMS" .github/workflows/security.yml

Repository: jusso-dev/Muster

Length of output: 4566


🌐 Web query:

GitHub Actions upload-artifact if-no-files-found partial path list missing files semantics

💡 Result:

In GitHub Actions, the upload-artifact action provides the if-no-files-found input to control behavior when the specified path does not match any files [1][2][3]. The semantics of if-no-files-found apply to the collection of files resulting from the path input (which supports file, directory, or wildcard patterns) [2][4][3]. When the action evaluates the provided paths and finds no files, it follows the behavior specified by this setting: - warn (default): The action completes successfully but outputs a warning in the workflow logs [1][2][5]. - error: The action fails, causing the workflow step to exit with an error message [1][2][3]. - ignore: The action completes successfully without outputting any warnings or errors [2][4][3]. Regarding "partial path list missing files": If you provide a list of paths (via a YAML multiline string) and some paths match files while others do not, the upload-artifact action generally evaluates the entire set. If at least one file is found across the provided paths, the action typically considers the task successful (regardless of whether specific paths in that list were empty). The if-no-files-found setting is triggered only if the final result of the evaluation is an empty set of files [1][2][3]. If you are experiencing issues where you believe files should be found but are not, ensure that: 1. You are using absolute paths from the root of the repository (or correctly relative to the project root), as the action does not necessarily respect working-directory settings from other steps [6]. 2. Hidden files are not being inadvertently excluded; you must set include-hidden-files: true to include them [6][7]. 3. You are using the latest version of the action (e.g., v4 or higher) to ensure you have access to the full if-no-files-found functionality [1][4][8].

Citations:


🌐 Web query:

"upload-artifact" "if-no-files-found" "all files are missing" documentation

💡 Result:

The if-no-files-found input parameter for the GitHub Actions upload-artifact action allows you to customize the behavior when the specified path results in no files being found [1][2]. The available options are [1][3]: - warn: Prints a warning in the logs but the action succeeds. This is the default behavior [1][2]. - error: Marks the workflow run as failed and displays an error message [2][3]. - ignore: Suppresses all warnings and errors; the action does not fail [2][3]. Usage Example: - uses: actions/upload-artifact@v7 with: name: my-artifact path: path/to/artifact/ if-no-files-found: error Note that this parameter triggers when no files are found across the entire provided path configuration [1]. Users have reported that when multiple paths are specified (e.g., using a list), the action may still succeed if some files are found even if others are missing, as it does not explicitly guarantee that every single specified path or file exists [4].

Citations:


Keep required evidence separate from optional diagnostics
.github/workflows/security.yml:77-91if-no-files-found only fails when nothing matches, so this upload can still succeed with provenance.json or trivy.sarif alone if muster-sbom.cdx.json or SHA256SUMS is missing. Split the required evidence into a strict upload or add explicit existence checks before uploading.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/security.yml around lines 77 - 91, Separate the required
evidence files muster-sbom.cdx.json and SHA256SUMS from optional diagnostics in
the security-artifacts upload step. Make the required upload fail when either
file is missing, while keeping provenance.json and trivy.sarif in a separate
upload that may continue with missing files; update the workflow around the
upload-artifact step without changing the artifact contents or generation
commands.

28 changes: 28 additions & 0 deletions apps/mcp-server/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{
"name": "@muster/mcp-server",
"version": "0.1.0",
"private": true,
"type": "module",
"scripts": {
"dev": "tsx watch src/index.ts",
"build": "tsc -p tsconfig.json",
"start": "node dist/index.js",
"typecheck": "tsc --noEmit",
"lint": "tsc --noEmit",
"test": "vitest run"
},
"dependencies": {
"@modelcontextprotocol/sdk": "1.30.0",
"@muster/config": "workspace:*",
"@muster/database": "workspace:*",
"@muster/mcp": "workspace:*",
"drizzle-orm": "catalog:",
"zod": "4.4.3"
},
"devDependencies": {
"@types/node": "^24.0.0",
"tsx": "^4.20.6",
"typescript": "catalog:",
"vitest": "4.1.10"
}
}
18 changes: 18 additions & 0 deletions apps/mcp-server/src/health.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { describe, expect, it } from "vitest";
import { checkDatabaseHealth } from "./health.ts";

describe("checkDatabaseHealth", () => {
it("is ready when the database responds", async () => {
const db = { execute: async () => undefined } as never;
expect(await checkDatabaseHealth(db)).toBe(true);
});

it("is not ready when the database is unreachable", async () => {
const db = {
execute: async () => {
throw new Error("connection refused");
},
} as never;
expect(await checkDatabaseHealth(db)).toBe(false);
});
});
18 changes: 18 additions & 0 deletions apps/mcp-server/src/health.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { sql } from "drizzle-orm";
import type { database } from "@muster/database";

/**
* A real dependency-aware readiness check, not a static liveness stub: a
* Postgres outage must surface as a non-ready response, not a false-positive
* "ready" that orchestrators route traffic to anyway.
*/
export async function checkDatabaseHealth(
db: ReturnType<typeof database>,
): Promise<boolean> {
try {
await db.execute(sql`select 1`);
return true;
} catch {
return false;
}
}
102 changes: 102 additions & 0 deletions apps/mcp-server/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import { randomUUID } from "node:crypto";
import {
createServer,
type IncomingMessage,
type ServerResponse,
} from "node:http";
import { StreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/streamableHttp.js";
import type { Transport } from "@modelcontextprotocol/sdk/shared/transport.js";
import { redactObservationText } from "@muster/config";
import { closeDatabase, database } from "@muster/database";
import { createMusterMcpServer, resolveInstallation } from "@muster/mcp";
import { checkDatabaseHealth } from "./health.ts";
import { gracefulShutdown } from "./shutdown.ts";

const db = database();

function bearerToken(header: string | undefined): string | null {
if (!header?.startsWith("Bearer ")) return null;
const token = header.slice("Bearer ".length).trim();
return token.length > 0 ? token : null;
}

function requestTraceId(request: IncomingMessage): string {
const header = request.headers["x-trace-id"];
const value = Array.isArray(header) ? header[0] : header;
return redactObservationText(value ?? randomUUID(), { maxStringLength: 200 });
}

function respondJson(response: ServerResponse, status: number, body: unknown) {
response.writeHead(status, { "content-type": "application/json" });
response.end(JSON.stringify(body));
}

const server = createServer(async (request, response) => {
const url = new URL(request.url ?? "/", "http://mcp-server.local");

if (request.method === "GET" && url.pathname === "/health") {
const healthy = await checkDatabaseHealth(db);
respondJson(response, healthy ? 200 : 503, {
status: healthy ? "ready" : "not_ready",
authority: "postgresql",
});
return;
}

if (url.pathname !== "/mcp") {
respondJson(response, 404, { error: "Not found" });
return;
}

// A missing, malformed, revoked, or cross-organisation credential all fail
// the same way here: a generic 401 that never reveals which case applied.
const token = bearerToken(request.headers.authorization);
const context = token ? await resolveInstallation(db, token) : null;
if (!context) {
respondJson(response, 401, { error: "Unauthorised" });
return;
}

const mcpServer = createMusterMcpServer({
db,
context,
traceId: requestTraceId(request),
});
// Omitting `sessionIdGenerator` (rather than setting it to `undefined`)
// selects stateless mode under `exactOptionalPropertyTypes`; every request
// is authorised independently by its own bearer token regardless.
const transport = new StreamableHTTPServerTransport({});
response.on("close", () => void transport.close());
try {
// The installed SDK's concrete transport class types `onclose`/`onerror`
// as `(() => void) | undefined` while `Transport` declares them as
// optional `() => void`; those are equivalent at runtime but disagree
// under `exactOptionalPropertyTypes`, hence the assertion.
await mcpServer.connect(transport as unknown as Transport);
await transport.handleRequest(request, response);
} catch (error) {
console.error(
"mcp.request.failed",
redactObservationText(error instanceof Error ? error.message : "unknown"),
);
if (!response.headersSent)
respondJson(response, 500, { error: "Request failed" });
}
});

// Kelpie tool calls poll for up to KELPIE_POLL_OPTIONS.timeoutMs (8s) inside
// the request; these bound the socket/request lifecycle around that with
// headroom, so a burst of concurrent bounded polls can't hold connections
// open indefinitely instead of being bounded like everything else here.
server.requestTimeout = 15_000;
server.headersTimeout = 12_000;
server.keepAliveTimeout = 5_000;

server.listen(Number(process.env.MCP_SERVER_PORT ?? 3003), "0.0.0.0");

async function shutdown() {
await gracefulShutdown(server, closeDatabase);
}

process.once("SIGINT", () => void shutdown());
process.once("SIGTERM", () => void shutdown());
Comment thread
jusso-dev marked this conversation as resolved.
37 changes: 37 additions & 0 deletions apps/mcp-server/src/shutdown.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { describe, expect, it } from "vitest";
import { gracefulShutdown } from "./shutdown.ts";

describe("gracefulShutdown", () => {
it("closes the database only after the server finishes draining", async () => {
const events: string[] = [];
const server = {
close: (callback: (error?: Error) => void) => {
setTimeout(() => {
events.push("server.closed");
callback();
}, 10);
},
};
const closeDb = async () => {
events.push("db.closed");
};
await gracefulShutdown(server, closeDb);
expect(events).toEqual(["server.closed", "db.closed"]);
});

it("propagates a server close error instead of closing the database", async () => {
const server = {
close: (callback: (error?: Error) => void) => {
callback(new Error("close failed"));
},
};
let dbClosed = false;
const closeDb = async () => {
dbClosed = true;
};
await expect(gracefulShutdown(server, closeDb)).rejects.toThrow(
"close failed",
);
expect(dbClosed).toBe(false);
});
});
20 changes: 20 additions & 0 deletions apps/mcp-server/src/shutdown.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
export interface CloseableServer {
close(callback: (error?: Error) => void): unknown;
}

/**
* `server.close()` is asynchronous: it stops accepting new connections but
* existing keep-alive requests continue until it emits its completion
* callback. Closing the database pool before that drain completes can tear
* it down under an in-flight MCP tool call (including its audit write), on
* every SIGTERM/rolling deploy. Await the callback first.
*/
export async function gracefulShutdown(
server: CloseableServer,
closeDb: () => Promise<void>,
): Promise<void> {
await new Promise<void>((resolve, reject) => {
server.close((error) => (error ? reject(error) : resolve()));
});
await closeDb();
}
5 changes: 5 additions & 0 deletions apps/mcp-server/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"extends": "../../tsconfig.json",
"compilerOptions": { "rootDir": "src", "outDir": "dist" },
"include": ["src/**/*.ts"]
}
10 changes: 5 additions & 5 deletions apps/web/components/room-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,13 @@ type TimelineItem = (typeof roomTimeline)[number];
type ThreadParent = TimelineItem | RoomMessageRecord;

const persistedTimelineIds = new Set([
"018f55d8-c4c7-7c3e-88ef-000000000701",
"018f55d8-c4c7-7c3e-88ef-000000000705",
"019e7a10-0000-7000-8000-000000000701",
"019e7a10-0000-7000-8000-000000000705",
]);
const seededThreadMessageIds = new Set([
"018f55d8-c4c7-7c3e-88ef-000000000702",
"018f55d8-c4c7-7c3e-88ef-000000000703",
"018f55d8-c4c7-7c3e-88ef-000000000704",
"019e7a10-0000-7000-8000-000000000702",
"019e7a10-0000-7000-8000-000000000703",
"019e7a10-0000-7000-8000-000000000704",
]);
const actorIdentity: Record<
string,
Expand Down
Loading
Loading