Skip to content

Commit 38ebdaf

Browse files
committed
fix: 8 CodeRabbit findings (Major/Minor/Trivial) + strict typecheck
Major fixes - package.json: wire `tsc --noEmit` into `build` via new `typecheck` script. tsup transpiles but does not enforce TS diagnostics; the safety net is back. - tsconfig.json: move to `module: NodeNext` / `moduleResolution: nodenext` and `target: ES2022` so the deprecated `node10` path no longer blocks tsc, `resolveJsonModule` added for completeness. - src/*.ts: real unknown-error narrowing in 3 catch blocks, null-safe payload access in download_attachment, GmailLabel/GmailMessagePart now typed via `gmail_v1.Schema$*` instead of hand-written shapes that were stricter than the SDK reality. - CHANGELOG.md: promote the accumulated content to `[0.1.0] - 2026-04-22` so the release-notes extractor can match the first tag. A fresh `[Unreleased]` stub keeps the next cycle unblocked. - codecov.yml: drop `if_no_uploads: success` / `if_not_found: success`. Missing coverage now fails the gate instead of silently no-op'ing. Add an `ignore` list for doc/config-only PRs that legitimately produce no coverage. - ASSURANCE_CASE.md CWE-79: reclassify from "N/A — no HTML output" to "out-of-scope for this process", documenting that download_email / read_email do surface HTML bodies and the consumer is responsible for sanitisation before rendering. Minor - ASSURANCE_CASE.md: Scorecard trigger description now matches the actual workflow (push to main + weekly + branch_protection_rule, not every commit). - README.md: `gmail.readonly` scope row now clarifies that filter tools require `gmail.settings.basic`. Trivial - ci.yml: coverage instrumentation runs only on the Node 20 axis; the 22/24 axes run plain `npm test` so we do not pay the vitest-v8 overhead three times for one coverage upload. `fail_ci_if_error` on Codecov is now `true`. - reply-all-helpers.test.ts: two regression tests lock the quoted- comma parser ('Doe, John' and multi-entry variants). Tests: 129/129. Build + lint + format:check clean.
1 parent cd0a71f commit 38ebdaf

11 files changed

Lines changed: 92 additions & 59 deletions

File tree

.github/workflows/ci.yml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,18 @@ jobs:
5959
- name: Build
6060
run: npm run build
6161

62-
- name: Test with coverage
62+
- name: Test
63+
if: matrix.node != '20'
64+
run: npm test
65+
66+
- name: Test with coverage (Node 20 only)
67+
if: matrix.node == '20'
6368
run: npm run test:coverage
6469

6570
- name: Upload coverage to Codecov
6671
if: matrix.node == '20'
6772
uses: codecov/codecov-action@57e3a136b779b570ffcdbf80b3bdc90e7fab3de2 # v6.0.0
6873
with:
69-
fail_ci_if_error: false
74+
fail_ci_if_error: true
7075
slug: klodr/gmail-mcp
7176
token: ${{ secrets.CODECOV_TOKEN }}

ASSURANCE_CASE.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ and scope filtering all live at that boundary.
145145
| **Fail closed** | Missing `~/.gmail-mcp/gcp-oauth.keys.json` → exit at startup with a clear error. Attachment path outside the jail → refuse before any write. Non-loopback OAuth callback hostname → reject at `authenticate()`. Invalid Zod input → refuse before the Gmail API call. |
146146
| **Minimise attack surface** | Single-file ESM bundle via `tsup` (no sourcemaps in the published tarball); only `dist/`, `README.md`, `LICENSE` in the npm files allowlist. No HTTP transport (stdio only) outside of the one-shot OAuth callback server. Tool list gated by OAuth scope. |
147147
| **Secrets are env-only / local-only** | OAuth refresh token at `~/.gmail-mcp/credentials.json` (mode `0o600`); client keys at `~/.gmail-mcp/gcp-oauth.keys.json` (user-provided). No secret ever travels over MCP stdout or MCP tool results. |
148-
| **Auditable & reproducible** | Every release is Sigstore-signed and SLSA-attested. Every commit triggers CI on Node 20/22/24 + CodeQL + Scorecard + Socket + CodeRabbit. |
148+
| **Auditable & reproducible** | Every release is Sigstore-signed and SLSA-attested. Every commit triggers CI on Node 20/22/24 + CodeQL + Socket + CodeRabbit. OpenSSF Scorecard runs on push to `main`, weekly on Monday, and on branch-protection rule changes (not on every PR commit). |
149149
| **Open source, MIT** | Anyone can audit. Project continuity documented in [CONTINUITY.md](./CONTINUITY.md). |
150150

151151
## 4. Common implementation weaknesses countered
@@ -158,7 +158,7 @@ Mapped to [CWE](https://cwe.mitre.org/) and [OWASP Top 10](https://owasp.org/Top
158158
| **CWE-59** Symlink following | Countered | Every leaf file write uses `fs.openSync` with `O_NOFOLLOW`; a pre-existing symlink at the destination causes the open to fail. |
159159
| **CWE-78 / CWE-94** Command / code injection | N/A | No `child_process`, no `eval`, no dynamic `require`. |
160160
| **CWE-89** SQL injection | N/A | No database. |
161-
| **CWE-79** XSS | N/A | No HTML output. |
161+
| **CWE-79** XSS | Out-of-scope for this process (MCP never renders HTML) — downstream responsibility | The `download_email` tool writes HTML bodies (via `emailToHtml()`) verbatim to `GMAIL_MCP_DOWNLOAD_DIR` and the `read_email` tool returns HTML string content to the MCP client. This MCP does not render HTML itself. If the consuming agent forwards that HTML to a browser, PDF pipeline, or any other HTML-executing surface, the agent must sanitise before rendering. Flagged transparently rather than claimed N/A. |
162162
| **CWE-88 / CWE-93 / CWE-113** CRLF / header injection | Countered | `sanitizeHeaderValue` strips `\r`, `\n`, `\0` from every user-supplied RFC-822 header value (`From`, `To`, `Cc`, `Bcc`, `Subject`, `In-Reply-To`, `References`). |
163163
| **CWE-117** Log injection | N/A | MCP emits no log file of its own (tracked as a future audit-log feature in [SECURITY.md](./SECURITY.md)). |
164164
| **CWE-200 / CWE-209** Information exposure / verbose errors | Countered | Error messages never include the OAuth refresh token or the Google OAuth client secret. |

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111

12+
- Placeholder for the next release cycle.
13+
14+
## [0.1.0] - 2026-04-22
15+
16+
### Added
17+
1218
- **Attachment jail** (`GMAIL_MCP_ATTACHMENT_DIR`, default `~/GmailAttachments/`, mode `0o700`). Every attachment path passed to `send_email` / `draft_email` / `reply_all` is `realpath`-canonicalized and rejected if it escapes the jail. Symlink-to-outside is rejected. Closes the headline prompt-injection exfiltration vector (a crafted inbound email instructing the agent to attach `~/.ssh/id_rsa` etc.).
1319
- **Download jail** (`GMAIL_MCP_DOWNLOAD_DIR`, default `~/GmailDownloads/`, mode `0o700`). `download_email` and `download_attachment` write exclusively inside this directory. The leaf is opened with `O_NOFOLLOW` so a pre-existing symlink at the destination cannot be used to escape. Post-`mkdir` the resolved path is re-verified against the jail root (TOCTOU defense).
1420
- **Zod schema bounds**: `SearchEmailsSchema.maxResults` ≤ 500, `ListInboxThreadsSchema.maxResults` ≤ 500, `GetInboxWithThreadsSchema.maxResults` ≤ 500 (≤ 100 when `expandThreads=true`), `Batch*EmailsSchema.messageIds` ≤ 1000, `Batch*EmailsSchema.batchSize` ≤ 100. Blocks resource-exhaustion requests.

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ See [llms-install.md](./llms-install.md) for an LLM-readable install guide.
144144

145145
| Scope shorthand | Full Gmail scope | What it grants |
146146
|---|---|---|
147-
| `gmail.readonly` | `…/auth/gmail.readonly` | Read messages, threads, labels, filters |
147+
| `gmail.readonly` | `…/auth/gmail.readonly` | Read messages, threads, labels (filter tools require `gmail.settings.basic`) |
148148
| `gmail.modify` | `…/auth/gmail.modify` | Readonly + apply/remove labels, delete messages |
149149
| `gmail.compose` | `…/auth/gmail.compose` | Create drafts |
150150
| `gmail.send` | `…/auth/gmail.send` | Send messages |

codecov.yml

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,15 @@ coverage:
55
target: auto
66
threshold: 0.5%
77
# Project coverage shouldn't drop more than 0.5% in any single PR.
8-
if_no_uploads: success
9-
if_not_found: success
10-
# PRs that don't touch source code (CI bumps, doc-only, GitHub
11-
# Actions updates) produce no coverage upload — pass instead of
12-
# blocking on a missing report.
8+
# if_no_uploads / if_not_found default to "failure" so a broken
9+
# upload step (vitest crash, Codecov outage, missing token) is
10+
# visible instead of silently turning the gate into a no-op.
11+
only_pulls: false
1312
patch:
1413
default:
1514
target: 95%
1615
threshold: 1.5%
17-
if_no_uploads: success
18-
if_not_found: success
16+
only_pulls: false
1917
# Strict patch gate (95% target, 1.5% threshold) — new code must
2018
# be ≥93.5% covered. The 1.5% threshold (vs an absolute 95%
2119
# requirement) exists for two narrow exceptions, not a blanket
@@ -24,6 +22,17 @@ coverage:
2422
# branches) are intentionally hard to exercise in unit tests.
2523
# - Pure-formatting PRs (Prettier reflows, ESLint cleanups)
2624
# surface as "new" lines but add no logic to test.
25+
# A missing upload still fails by default so broken CI surfaces.
26+
ignore:
27+
# Doc-only / config-only PRs never touch these paths, so Codecov
28+
# has no reason to demand coverage for them even though their file
29+
# trees live under the repository root.
30+
- "**/*.md"
31+
- "**/*.yaml"
32+
- "**/*.yml"
33+
- "**/*.json"
34+
- ".github/**"
35+
- "scripts/**"
2736

2837
comment:
2938
layout: "reach,diff,flags,files,footer"

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
"gmail-mcp": "./dist/index.js"
99
},
1010
"scripts": {
11-
"build": "tsup",
11+
"typecheck": "tsc --noEmit",
12+
"build": "npm run typecheck && tsup",
1213
"start": "node dist/index.js",
1314
"auth": "node dist/index.js auth",
1415
"test": "vitest run",

src/index.ts

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -84,22 +84,11 @@ const OAUTH_PATH = process.env.GMAIL_OAUTH_PATH || path.join(CONFIG_DIR, "gcp-oa
8484
const CREDENTIALS_PATH =
8585
process.env.GMAIL_CREDENTIALS_PATH || path.join(CONFIG_DIR, "credentials.json");
8686

87-
// Type definitions for Gmail API responses
88-
interface GmailMessagePart {
89-
partId?: string;
90-
mimeType?: string;
91-
filename?: string;
92-
headers?: Array<{
93-
name: string;
94-
value: string;
95-
}>;
96-
body?: {
97-
attachmentId?: string;
98-
size?: number;
99-
data?: string;
100-
};
101-
parts?: GmailMessagePart[];
102-
}
87+
// Type definitions for Gmail API responses.
88+
// Accepts both our canonical shape and googleapis' `Schema$MessagePart`,
89+
// which differs only by widening every optional `string` to `string | null`.
90+
import type { gmail_v1 as gmail_v1_types } from "googleapis";
91+
type GmailMessagePart = gmail_v1_types.Schema$MessagePart;
10392

10493
interface EmailContent {
10594
text: string;
@@ -478,8 +467,9 @@ async function main() {
478467
}
479468
}
480469
} catch (threadError: unknown) {
470+
const msg = threadError instanceof Error ? threadError.message : String(threadError);
481471
console.warn(
482-
`Warning: Could not fetch thread ${validatedArgs.threadId} for header resolution: ${threadError.message}`,
472+
`Warning: Could not fetch thread ${validatedArgs.threadId} for header resolution: ${msg}`,
483473
);
484474
// Continue without threading headers - degraded but not broken
485475
}
@@ -599,9 +589,10 @@ async function main() {
599589
} catch (error: unknown) {
600590
// Log attachment-related errors for debugging
601591
if (validatedArgs.attachments && validatedArgs.attachments.length > 0) {
592+
const msg = error instanceof Error ? error.message : String(error);
602593
console.error(
603594
`Failed to send email with ${validatedArgs.attachments.length} attachments:`,
604-
error.message,
595+
msg,
605596
);
606597
}
607598
throw error;
@@ -812,11 +803,12 @@ async function main() {
812803
],
813804
};
814805
} catch (error: unknown) {
806+
const msg = error instanceof Error ? error.message : String(error);
815807
return {
816808
content: [
817809
{
818810
type: "text",
819-
text: `Failed to download email: ${error.message}`,
811+
text: `Failed to download email: ${msg}`,
820812
},
821813
],
822814
};
@@ -1320,8 +1312,9 @@ async function main() {
13201312
};
13211313

13221314
filename =
1323-
findAttachment(messageResponse.data.payload) ||
1324-
`attachment-${validatedArgs.attachmentId}`;
1315+
(messageResponse.data.payload
1316+
? findAttachment(messageResponse.data.payload)
1317+
: null) || `attachment-${validatedArgs.attachmentId}`;
13251318
}
13261319

13271320
// Sanitize filename to prevent path traversal
@@ -1348,11 +1341,12 @@ async function main() {
13481341
],
13491342
};
13501343
} catch (error: unknown) {
1344+
const msg = error instanceof Error ? error.message : String(error);
13511345
return {
13521346
content: [
13531347
{
13541348
type: "text",
1355-
text: `Failed to download attachment: ${error.message}`,
1349+
text: `Failed to download attachment: ${msg}`,
13561350
},
13571351
],
13581352
};
@@ -1403,7 +1397,7 @@ async function main() {
14031397
}
14041398
};
14051399
if (msg.payload) {
1406-
processAttachmentParts(msg.payload as GmailMessagePart);
1400+
processAttachmentParts(msg.payload);
14071401
}
14081402

14091403
return {
@@ -1597,7 +1591,7 @@ async function main() {
15971591
}
15981592
};
15991593
if (msg.payload) {
1600-
processAttachmentParts(msg.payload as GmailMessagePart);
1594+
processAttachmentParts(msg.payload);
16011595
}
16021596

16031597
return {
@@ -1752,11 +1746,12 @@ async function main() {
17521746
throw new Error(`Unknown tool: ${name}`);
17531747
}
17541748
} catch (error: unknown) {
1749+
const msg = error instanceof Error ? error.message : String(error);
17551750
return {
17561751
content: [
17571752
{
17581753
type: "text",
1759-
text: `Error: ${error.message}`,
1754+
text: `Error: ${msg}`,
17601755
},
17611756
],
17621757
};

src/label-manager.ts

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,10 @@
66
import type { gmail_v1 } from "googleapis";
77
import { asGmailApiError } from "./gmail-errors.js";
88

9-
// Type definitions for Gmail API labels
10-
export interface GmailLabel {
11-
id: string;
12-
name: string;
13-
type?: string;
14-
messageListVisibility?: string;
15-
labelListVisibility?: string;
16-
messagesTotal?: number;
17-
messagesUnread?: number;
18-
color?: {
19-
textColor?: string;
20-
backgroundColor?: string;
21-
};
22-
}
9+
// Re-export googleapis' Schema$Label under our historical name so call
10+
// sites can keep using `GmailLabel` unchanged while benefiting from the
11+
// canonical (nullable) typing the SDK actually returns.
12+
export type GmailLabel = gmail_v1.Schema$Label;
2313

2414
/**
2515
* Creates a new Gmail label

src/reply-all-helpers.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,24 @@ describe("parseEmailAddresses", () => {
4949
it("ignores entries without @ symbol", () => {
5050
expect(parseEmailAddresses("invalid, user@example.com")).toEqual(["user@example.com"]);
5151
});
52+
53+
it("does not split on commas inside quoted display names", () => {
54+
// Regression: a naive split(",") would produce three garbage tokens
55+
// for '"Doe, John" <john@example.com>, jane@example.com'. The
56+
// quote-aware tokenizer must return two recipients.
57+
expect(parseEmailAddresses('"Doe, John" <john@example.com>, jane@example.com')).toEqual([
58+
"john@example.com",
59+
"jane@example.com",
60+
]);
61+
});
62+
63+
it("handles multiple quoted display names with commas", () => {
64+
expect(
65+
parseEmailAddresses(
66+
'"Smith, Alice" <alice@example.com>, "Jones, Bob" <bob@example.com>, "Brown, Carol" <carol@example.com>',
67+
),
68+
).toEqual(["alice@example.com", "bob@example.com", "carol@example.com"]);
69+
});
5270
});
5371

5472
describe("filterOutEmail", () => {

src/tools.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,9 @@ export interface ToolAnnotations {
343343
export interface ToolDefinition {
344344
name: string;
345345
description: string;
346+
// zod-to-json-schema@3's public signature widens to `z.ZodType<any>`;
347+
// using a tighter generic here causes a structural mismatch at the
348+
// consumer call site. The `any` is fenced inside ToolDefinition only.
346349
schema: z.ZodType<unknown>;
347350
scopes: string[]; // Any of these scopes grants access
348351
annotations: ToolAnnotations;
@@ -544,12 +547,17 @@ export const toolDefinitions: ToolDefinition[] = [
544547
},
545548
];
546549

547-
// Convert tool definitions to MCP tool format
550+
// Convert tool definitions to MCP tool format.
551+
// The cast bridges a generics mismatch between Zod v4's `ZodType` shape
552+
// and zod-to-json-schema@3's expected `ZodType<any, ZodTypeDef, any>`.
553+
// Runtime behaviour is unaffected — both APIs consume the same schema
554+
// instance, only the TS generic signatures differ.
548555
export function toMcpTools(tools: ToolDefinition[]) {
549556
return tools.map((tool) => ({
550557
name: tool.name,
551558
description: tool.description,
552-
inputSchema: zodToJsonSchema(tool.schema),
559+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
560+
inputSchema: zodToJsonSchema(tool.schema as any),
553561
annotations: tool.annotations,
554562
}));
555563
}

0 commit comments

Comments
 (0)