Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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 apps/server/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,14 @@ export const ensureServerDirectories = Effect.fn(function* (derivedPaths: Server
],
{ concurrency: "unbounded" },
);
// Logs, traces and the database are written with the process umask; keep them owner-only.
yield* fs
.chmod(derivedPaths.stateDir, 0o700)
.pipe(
Effect.catch((cause) =>
Effect.logWarning("Could not restrict state directory permissions.", { cause }),
),
);

const swept = sweepStalePendingAttachments({
attachmentsDir: derivedPaths.attachmentsDir,
Expand Down
67 changes: 67 additions & 0 deletions packages/shared/src/observability.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,73 @@ describe("observability", () => {
),
);

it.effect("keeps WebSocket credentials out of trace files and exported spans", () =>
Effect.scoped(
Effect.gen(function* () {
const fileSystem = yield* FileSystem.FileSystem;
const path = yield* Path.Path;
const tempDir = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-trace-sink-" });
const tracePath = path.join(tempDir, "shared.trace.ndjson");
const sink = yield* makeTraceSink({
filePath: tracePath,
maxBytes: 1024 * 1024,
maxFiles: 2,
batchWindowMs: 10_000,
});
const delegates: Array<Tracer.NativeSpan> = [];
const tracer = yield* makeLocalFileTracer({
filePath: tracePath,
maxBytes: 1024 * 1024,
maxFiles: 2,
batchWindowMs: 10_000,
sink,
delegate: Tracer.make({
span: (options) => {
const span = new Tracer.NativeSpan(options);
delegates.push(span);
return span;
},
}),
});
const url = "/ws?wsTicket=ticket-secret&dpop=proof-secret&clientSurface=web";

yield* Effect.annotateCurrentSpan({
"url.full": `http://127.0.0.1:3773${url}`,
"url.query": url.slice(4),
}).pipe(
Effect.withSpan("GET", { kind: "server" }),
Effect.provideService(Tracer.Tracer, tracer),
);
yield* Effect.die(new Error(`RequestParseError (GET ${url})`)).pipe(
Effect.tapCause((cause) => Effect.logError(cause)),
Effect.withSpan("DeviceHubProxy.proxyWebSocket"),
Effect.provideService(Tracer.Tracer, tracer),
Effect.provide(Logger.layer([Logger.tracerLogger], { mergeWithExisting: false })),
Effect.exit,
);
yield* sink.close();

const written = yield* fileSystem.readFileString(tracePath);
const exported = delegates.map((span) =>
JSON.stringify({
attributes: Object.fromEntries(span.attributes),
events: span.events.map(([, , attributes]) => attributes),
exit:
span.status._tag === "Ended" && Exit.isFailure(span.status.exit)
? Cause.pretty(span.status.exit.cause)
: "",
}),
);
for (const text of [written, ...exported]) {
assert.notInclude(text, "ticket-secret");
assert.notInclude(text, "proof-secret");
}
assert.include(written, "wsTicket=<redacted>&dpop=<redacted>&clientSurface=web");
assert.include(exported.join("\n"), "effect.cause");
}),
),
);

it.effect("drops only the invalid trace record when serialization fails", () =>
Effect.scoped(
Effect.gen(function* () {
Expand Down
34 changes: 29 additions & 5 deletions packages/shared/src/observability.ts
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,12 @@ function spanToTraceRecord(span: SerializableSpan): EffectTraceRecord {
};
}

// WebSocket tickets and their DPoP proofs ride in URL queries and stay valid after the request.
const URL_CREDENTIAL_PARAMS = /(^|[?&"\s])(wsTicket|dpop)=[^&#"\\\s]*/g;

const redactUrlCredentials = (text: string) =>
text.replace(URL_CREDENTIAL_PARAMS, "$1$2=<redacted>");

export const makeTraceSink = Effect.fn("makeTraceSink")(function* (options: TraceSinkOptions) {
const sink = yield* makeAsyncRotatingFileSink({
filePath: options.filePath,
Expand Down Expand Up @@ -520,7 +526,7 @@ export const makeTraceSink = Effect.fn("makeTraceSink")(function* (options: Trac
filePath: options.filePath,
push(record) {
try {
const text = `${JSON.stringify(record)}\n`;
const text = `${redactUrlCredentials(JSON.stringify(record))}\n`;
const bytes = Buffer.byteLength(text);
if (bytes > options.maxBytes) return;
if (bufferBytes + bytes > maxBufferBytes) {
Expand All @@ -541,6 +547,16 @@ export const makeTraceSink = Effect.fn("makeTraceSink")(function* (options: Trac
} satisfies TraceSink;
});

// Effect's HTTP errors, including failed upgrades, put the request URL in their message.
function redactExitCredentials(exit: Exit.Exit<unknown, unknown>): Exit.Exit<unknown, unknown> {
if (ExitRuntime.isSuccess(exit) || Cause.hasInterruptsOnly(exit.cause)) {
return exit;
}
const pretty = Cause.pretty(exit.cause);
const redacted = redactUrlCredentials(pretty);
return redacted === pretty ? exit : ExitRuntime.die(new Error(redacted));
}

class LocalFileSpan implements Tracer.Span {
readonly _tag = "Span";
readonly name: string;
Expand Down Expand Up @@ -590,20 +606,28 @@ class LocalFileSpan implements Tracer.Span {
endTime,
exit: traceExit,
};
this.delegate.end(endTime, traceExit);
this.delegate.end(endTime, redactExitCredentials(traceExit));

if (this.sampled) {
this.push(spanToTraceRecord(this));
}
}

attribute(key: string, value: unknown): void {
this.attributes.set(key, value);
this.delegate.attribute(key, value);
const recorded =
(key === "url.full" || key === "url.query") && typeof value === "string"
? redactUrlCredentials(value)
: value;
this.attributes.set(key, recorded);
this.delegate.attribute(key, recorded);
}

event(name: string, startTime: bigint, attributes?: Record<string, unknown>): void {
const nextAttributes = attributes ?? {};
const cause = attributes?.["effect.cause"];
const nextAttributes =
typeof cause === "string"
? { ...attributes, "effect.cause": redactUrlCredentials(cause) }
: (attributes ?? {});
this.events.push([name, startTime, nextAttributes]);
this.delegate.event(name, startTime, nextAttributes);
}
Expand Down
Loading