Skip to content

fix(security): address first-pass CodeQL alerts #1114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kunickiaj merged 1 commit into from
May 21, 2026
Merged

fix(security): address first-pass CodeQL alerts #1114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ on:

workflow_call:

permissions:
contents: read

jobs:
tsc:
name: TypeScript Check
Expand Down
6 changes: 5 additions & 1 deletion packages/cli/src/commands/db.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ import {
resolveDbOpt,
} from "../shared-options.js";

function escapeSqlLikePattern(value: string): string {
return value.replaceAll("\\", "\\\\").replaceAll("%", "\\%").replaceAll("_", "\\_");
}

function formatBytes(bytes: number): string {
if (bytes < 1024) return `${bytes} B`;
if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)} KB`;
Expand Down Expand Up @@ -308,7 +312,7 @@ renameProjectCmd.action((oldName: string, newName: string, opts: DbOpts & { appl
const store = new MemoryStore(resolveDbPath(resolveDbOpt(opts)));
try {
const dryRun = !opts.apply;
const escapedOld = oldName.replace(/%/g, "\\%").replace(/_/g, "\\_");
const escapedOld = escapeSqlLikePattern(oldName);
const suffixPattern = `%/${escapedOld}`;
const tables = ["sessions", "raw_event_sessions"] as const;
const counts: Record<string, number> = {};
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/commands/stats.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const statsCommand = statsCmd.action((opts: DbOpts & JsonOpts) => {
try {
const result = store.stats();
if (opts.json) {
// lgtm[js/clear-text-logging] This is intentional CLI stdout for `--json`, not an application log.
console.log(JSON.stringify(result, null, 2));
return;
}
Expand Down
52 changes: 29 additions & 23 deletions packages/core/src/coordinator-actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ import { ensureDeviceIdentity, loadPublicKey } from "./sync-identity.js";
const VALID_INVITE_POLICIES = new Set(["auto_admit", "approval_required"]);
const INVITE_IMPORT_TIMEOUT_S = 10;

function stripTrailingSlashes(value: string): string {
let end = value.length;
while (end > 0 && value.charCodeAt(end - 1) === 47) end--;
return end === value.length ? value : value.slice(0, end);
}

function coordinatorRemoteTarget(config = readCodememConfigFile()): {
remoteUrl: string | null;
adminSecret: string | null;
Expand Down Expand Up @@ -201,7 +207,7 @@ export async function coordinatorCreateGroupAction(opts: {
if (!adminSecret) throw new Error("Admin secret required.");
const payload = await remoteRequest(
"POST",
`${remote.replace(/\/+$/, "")}/v1/admin/groups`,
`${stripTrailingSlashes(remote)}/v1/admin/groups`,
adminSecret,
{ group_id: groupId, display_name: opts.displayName ?? null },
);
Expand Down Expand Up @@ -239,7 +245,7 @@ export async function coordinatorRenameGroupAction(opts: {
try {
payload = await remoteRequest(
"POST",
`${remote.replace(/\/+$/, "")}/v1/admin/groups/rename`,
`${stripTrailingSlashes(remote)}/v1/admin/groups/rename`,
adminSecret,
{ group_id: groupId, display_name: displayName },
);
Expand Down Expand Up @@ -276,7 +282,7 @@ export async function coordinatorArchiveGroupAction(opts: {
try {
payload = await remoteRequest(
"POST",
`${remote.replace(/\/+$/, "")}/v1/admin/groups/archive`,
`${stripTrailingSlashes(remote)}/v1/admin/groups/archive`,
adminSecret,
{ group_id: groupId },
);
Expand Down Expand Up @@ -314,7 +320,7 @@ export async function coordinatorUnarchiveGroupAction(opts: {
try {
payload = await remoteRequest(
"POST",
`${remote.replace(/\/+$/, "")}/v1/admin/groups/unarchive`,
`${stripTrailingSlashes(remote)}/v1/admin/groups/unarchive`,
adminSecret,
{ group_id: groupId },
);
Expand Down Expand Up @@ -349,7 +355,7 @@ export async function coordinatorListGroupsAction(opts?: {
if (!adminSecret) throw new Error("Admin secret required.");
const payload = await remoteRequest(
"GET",
`${remote.replace(/\/+$/, "")}/v1/admin/groups${includeArchived ? "?include_archived=1" : ""}`,
`${stripTrailingSlashes(remote)}/v1/admin/groups${includeArchived ? "?include_archived=1" : ""}`,
adminSecret,
);
return Array.isArray(payload?.items)
Expand Down Expand Up @@ -382,7 +388,7 @@ export async function coordinatorListScopesAction(opts: {
if (!adminSecret) throw new Error("Admin secret required.");
const payload = await remoteRequest(
"GET",
`${remote.replace(/\/+$/, "")}/v1/admin/groups/${encodeURIComponent(groupId)}/scopes${opts.includeInactive ? "?include_inactive=1" : ""}`,
`${stripTrailingSlashes(remote)}/v1/admin/groups/${encodeURIComponent(groupId)}/scopes${opts.includeInactive ? "?include_inactive=1" : ""}`,
adminSecret,
);
return Array.isArray(payload?.items)
Expand Down Expand Up @@ -427,7 +433,7 @@ export async function coordinatorCreateScopeAction(opts: {
if (!adminSecret) throw new Error("Admin secret required.");
const payload = await remoteRequest(
"POST",
`${remote.replace(/\/+$/, "")}/v1/admin/groups/${encodeURIComponent(groupId)}/scopes`,
`${stripTrailingSlashes(remote)}/v1/admin/groups/${encodeURIComponent(groupId)}/scopes`,
adminSecret,
{
scope_id: scopeId,
Expand Down Expand Up @@ -494,7 +500,7 @@ export async function coordinatorUpdateScopeAction(opts: {
try {
payload = await remoteRequest(
"PATCH",
`${remote.replace(/\/+$/, "")}/v1/admin/groups/${encodeURIComponent(groupId)}/scopes/${encodeURIComponent(scopeId)}`,
`${stripTrailingSlashes(remote)}/v1/admin/groups/${encodeURIComponent(groupId)}/scopes/${encodeURIComponent(scopeId)}`,
adminSecret,
{
label: opts.label ?? undefined,
Expand Down Expand Up @@ -553,7 +559,7 @@ export async function coordinatorListScopeMembershipsAction(opts: {
if (!adminSecret) throw new Error("Admin secret required.");
const payload = await remoteRequest(
"GET",
`${remote.replace(/\/+$/, "")}/v1/admin/groups/${encodeURIComponent(groupId)}/scopes/${encodeURIComponent(scopeId)}/members${opts.includeRevoked ? "?include_revoked=1" : ""}`,
`${stripTrailingSlashes(remote)}/v1/admin/groups/${encodeURIComponent(groupId)}/scopes/${encodeURIComponent(scopeId)}/members${opts.includeRevoked ? "?include_revoked=1" : ""}`,
adminSecret,
);
return Array.isArray(payload?.items)
Expand Down Expand Up @@ -594,7 +600,7 @@ export async function coordinatorGrantScopeMembershipAction(
if (!adminSecret) throw new Error("Admin secret required.");
const payload = await remoteRequest(
"POST",
`${remote.replace(/\/+$/, "")}/v1/admin/groups/${encodeURIComponent(groupId)}/scopes/${encodeURIComponent(scopeId)}/members`,
`${stripTrailingSlashes(remote)}/v1/admin/groups/${encodeURIComponent(groupId)}/scopes/${encodeURIComponent(scopeId)}/members`,
adminSecret,
{
device_id: deviceId,
Expand Down Expand Up @@ -658,7 +664,7 @@ export async function coordinatorRevokeScopeMembershipAction(
try {
await remoteRequest(
"POST",
`${remote.replace(/\/+$/, "")}/v1/admin/groups/${encodeURIComponent(groupId)}/scopes/${encodeURIComponent(scopeId)}/members/${encodeURIComponent(deviceId)}/revoke`,
`${stripTrailingSlashes(remote)}/v1/admin/groups/${encodeURIComponent(groupId)}/scopes/${encodeURIComponent(scopeId)}/members/${encodeURIComponent(deviceId)}/revoke`,
adminSecret,
{
membership_epoch: opts.membershipEpoch ?? null,
Expand Down Expand Up @@ -737,7 +743,7 @@ export async function coordinatorListDevicesAction(opts: {
if (!adminSecret) throw new Error("Admin secret required.");
const payload = await remoteRequest(
"GET",
`${remote.replace(/\/+$/, "")}/v1/admin/devices?group_id=${encodeURIComponent(groupId)}&include_disabled=${opts.includeDisabled ? "1" : "0"}`,
`${stripTrailingSlashes(remote)}/v1/admin/devices?group_id=${encodeURIComponent(groupId)}&include_disabled=${opts.includeDisabled ? "1" : "0"}`,
adminSecret,
);
return Array.isArray(payload?.items)
Expand Down Expand Up @@ -776,7 +782,7 @@ export async function coordinatorRenameDeviceAction(opts: {
try {
payload = await remoteRequest(
"POST",
`${remote.replace(/\/+$/, "")}/v1/admin/devices/rename`,
`${stripTrailingSlashes(remote)}/v1/admin/devices/rename`,
adminSecret,
{ group_id: groupId, device_id: deviceId, display_name: displayName },
);
Expand Down Expand Up @@ -823,7 +829,7 @@ export async function coordinatorDisableDeviceAction(opts: {
try {
await remoteRequest(
"POST",
`${remote.replace(/\/+$/, "")}/v1/admin/devices/disable`,
`${stripTrailingSlashes(remote)}/v1/admin/devices/disable`,
adminSecret,
{ group_id: groupId, device_id: deviceId },
);
Expand Down Expand Up @@ -864,7 +870,7 @@ export async function coordinatorEnableDeviceAction(opts: {
try {
await remoteRequest(
"POST",
`${remote.replace(/\/+$/, "")}/v1/admin/devices/enable`,
`${stripTrailingSlashes(remote)}/v1/admin/devices/enable`,
adminSecret,
{ group_id: groupId, device_id: deviceId },
);
Expand Down Expand Up @@ -905,7 +911,7 @@ export async function coordinatorRemoveDeviceAction(opts: {
try {
await remoteRequest(
"POST",
`${remote.replace(/\/+$/, "")}/v1/admin/devices/remove`,
`${stripTrailingSlashes(remote)}/v1/admin/devices/remove`,
adminSecret,
{ group_id: groupId, device_id: deviceId },
);
Expand Down Expand Up @@ -948,7 +954,7 @@ export async function coordinatorCreateInviteAction(opts: {
throw new Error("Admin secret required to create invites via the coordinator API.");
const payload = await remoteRequest(
"POST",
`${remote.replace(/\/+$/, "")}/v1/admin/invites`,
`${stripTrailingSlashes(remote)}/v1/admin/invites`,
adminSecret,
{
group_id: opts.groupId,
Expand Down Expand Up @@ -1035,7 +1041,7 @@ export async function coordinatorImportInviteAction(opts: {
// trailing slashes before comparing so harmless formatting differences
// (e.g. `https://coord.example.com` vs. `…/`) don't reject valid same-
// coordinator invites.
const normalizeCoordinatorUrl = (value: string): string => value.trim().replace(/\/+$/, "");
const normalizeCoordinatorUrl = (value: string): string => stripTrailingSlashes(value.trim());
const existingCoordinator = normalizeCoordinatorUrl(String(config.sync_coordinator_url ?? ""));
const incomingCoordinator = normalizeCoordinatorUrl(coordinatorUrl);
if (existingCoordinator && existingCoordinator !== incomingCoordinator) {
Expand All @@ -1048,7 +1054,7 @@ export async function coordinatorImportInviteAction(opts: {
try {
[status, response] = await requestJson(
"POST",
`${coordinatorUrl.replace(/\/+$/, "")}/v1/join`,
`${stripTrailingSlashes(coordinatorUrl)}/v1/join`,
{
body: {
token: String(payload.token),
Expand Down Expand Up @@ -1111,7 +1117,7 @@ export async function coordinatorListJoinRequestsAction(opts: {
if (!adminSecret) throw new Error("Admin secret required.");
const payload = await remoteRequest(
"GET",
`${remote.replace(/\/+$/, "")}/v1/admin/join-requests?group_id=${encodeURIComponent(opts.groupId)}`,
`${stripTrailingSlashes(remote)}/v1/admin/join-requests?group_id=${encodeURIComponent(opts.groupId)}`,
adminSecret,
);
return Array.isArray(payload?.items)
Expand Down Expand Up @@ -1145,7 +1151,7 @@ export async function coordinatorReviewJoinRequestAction(opts: {
: "/v1/admin/join-requests/deny";
const payload = await remoteRequest(
"POST",
`${remote.replace(/\/+$/, "")}${endpoint}`,
`${stripTrailingSlashes(remote)}${endpoint}`,
adminSecret,
{
request_id: opts.requestId,
Expand Down Expand Up @@ -1181,7 +1187,7 @@ export async function coordinatorListBootstrapGrantsAction(opts: {
if (!adminSecret) throw new Error("Admin secret required.");
const payload = await remoteRequest(
"GET",
`${remote.replace(/\/+$/, "")}/v1/admin/bootstrap-grants?group_id=${encodeURIComponent(opts.groupId)}`,
`${stripTrailingSlashes(remote)}/v1/admin/bootstrap-grants?group_id=${encodeURIComponent(opts.groupId)}`,
adminSecret,
);
return Array.isArray(payload?.items)
Expand Down Expand Up @@ -1211,7 +1217,7 @@ export async function coordinatorRevokeBootstrapGrantAction(opts: {
try {
await remoteRequest(
"POST",
`${remote.replace(/\/+$/, "")}/v1/admin/bootstrap-grants/revoke`,
`${stripTrailingSlashes(remote)}/v1/admin/bootstrap-grants/revoke`,
adminSecret,
{ grant_id: opts.grantId },
);
Expand Down
13 changes: 12 additions & 1 deletion packages/core/src/ingest-pipeline.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
} from "./ingest-events.js";
import { isLowSignalObservation, normalizeObservation } from "./ingest-filters.js";
import { type IngestOptions, ingest } from "./ingest-pipeline.js";
import { stripPrivate } from "./ingest-sanitize.js";
import { isSensitiveFieldName, stripPrivate } from "./ingest-sanitize.js";
import {
buildTranscript,
deriveRequest,
Expand Down Expand Up @@ -428,6 +428,17 @@ describe("ingest-sanitize", () => {
it("is case-insensitive", () => {
expect(stripPrivate("a <PRIVATE>b</PRIVATE> c")).toBe("a c");
});

it("removes stray closing tags before later private blocks", () => {
expect(stripPrivate("a </private> b <private>secret</private> c")).toBe("a b c");
});
});

describe("isSensitiveFieldName", () => {
it("detects camelCase API and private key names", () => {
expect(isSensitiveFieldName("apiKey")).toBe(true);
expect(isSensitiveFieldName("privateKey")).toBe(true);
});
});
});

Expand Down
74 changes: 58 additions & 16 deletions packages/core/src/ingest-sanitize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@
// Private content stripping
// ---------------------------------------------------------------------------

const PRIVATE_BLOCK_RE = /<private>.*?<\/private>/gis;
const PRIVATE_OPEN_RE = /<private>/i;
const PRIVATE_CLOSE_RE = /<\/private>/gi;
const PRIVATE_OPEN = "<private>";
const PRIVATE_CLOSE = "</private>";

/**
* Remove `<private>…</private>` blocks from text.
Expand All @@ -21,31 +20,74 @@ const PRIVATE_CLOSE_RE = /<\/private>/gi;
*/
export function stripPrivate(text: string): string {
if (!text) return "";
// Remove matched pairs
let redacted = text.replace(PRIVATE_BLOCK_RE, "");
// Orphaned opening tag — truncate everything after it
const openMatch = PRIVATE_OPEN_RE.exec(redacted);
if (openMatch) {
redacted = redacted.slice(0, openMatch.index);
let remaining = text;
let lowered = remaining.toLowerCase();
let output = "";
while (remaining) {
const openIndex = lowered.indexOf(PRIVATE_OPEN);
const closeIndex = lowered.indexOf(PRIVATE_CLOSE);
if (openIndex < 0) {
if (closeIndex < 0) return output + remaining;
Comment thread
kunickiaj marked this conversation as resolved.
output += remaining.slice(0, closeIndex);
remaining = remaining.slice(closeIndex + PRIVATE_CLOSE.length);
lowered = remaining.toLowerCase();
continue;
}
if (closeIndex >= 0 && closeIndex < openIndex) {
output += remaining.slice(0, closeIndex);
remaining = remaining.slice(closeIndex + PRIVATE_CLOSE.length);
lowered = remaining.toLowerCase();
continue;
}
output += remaining.slice(0, openIndex);
const blockCloseIndex = lowered.indexOf(PRIVATE_CLOSE, openIndex + PRIVATE_OPEN.length);
if (blockCloseIndex < 0) return output;
remaining = remaining.slice(blockCloseIndex + PRIVATE_CLOSE.length);
lowered = remaining.toLowerCase();
}
// Stray closing tags
redacted = redacted.replace(PRIVATE_CLOSE_RE, "");
return redacted;
return output;
}

// ---------------------------------------------------------------------------
// Sensitive field detection
// ---------------------------------------------------------------------------

const SENSITIVE_FIELD_RE =
/(?:^|_|-)(?:token|secret|password|passwd|api[_-]?key|authorization|private[_-]?key|cookie)(?:$|_|-)/i;

const REDACTED_VALUE = "[REDACTED]";

function fieldSegments(value: string): string[] {
const segments: string[] = [];
let current = "";
for (const ch of value) {
if (ch === "_" || ch === "-") {
const trimmed = current.trim();
if (trimmed) segments.push(trimmed);
current = "";
} else {
current += ch;
}
}
const trimmed = current.trim();
if (trimmed) segments.push(trimmed);
return segments;
}

export function isSensitiveFieldName(fieldName: string): boolean {
const normalized = fieldName.trim().toLowerCase();
if (!normalized) return false;
return SENSITIVE_FIELD_RE.test(normalized);
if (normalized.includes("apikey") || normalized.includes("privatekey")) return true;
const segments = fieldSegments(normalized);
if (
segments.some((part) =>
["token", "secret", "password", "passwd", "authorization", "cookie"].includes(part),
)
) {
return true;
}
return (
segments.length >= 2 &&
((segments.includes("api") && segments.includes("key")) ||
(segments.includes("private") && segments.includes("key")))
Comment thread
kunickiaj marked this conversation as resolved.
);
}

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