Skip to content

Commit a9ad9c0

Browse files
committed
fix: аудит-фиксы — статистика, ретрай-идемпотентность, SSRF defense-in-depth (v1.2.0)
- get_statistics: подсчёт data-строк вместо мёртвой проверки пустого TSV; одиночная граница dateFrom/dateTo → ошибка вместо тихого LAST_30_DAYS - get_bid_modifiers: запрос VideoAdjustmentFieldNames - list_campaigns: конвертация денежных полей Funds из микроединиц - getAll: LimitedBy = курсор после последней слитой страницы - client: ретрай 5xx/сети только для идемпотентных методов; таймаут покрывает тело; defense-in-depth валидация service (нет ://, не с /) - upload_ad_image: таймаут + лимит размера + только http(s) - request_id в тексте ошибки; кэш GeoRegions; MAX_TOOL_LIMIT в assets/media - docs: Node 20+, get_balance в TOOLS.md
1 parent b57081c commit a9ad9c0

25 files changed

Lines changed: 608 additions & 99 deletions

CHANGELOG.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,49 @@
55
Формат основан на [Keep a Changelog](https://keepachangelog.com/ru/1.0.0/),
66
проект придерживается [семантического версионирования](https://semver.org/lang/ru/).
77

8+
## [Unreleased]
9+
10+
## [1.2.0] — 2026-07-02
11+
12+
### Исправлено
13+
- `get_statistics`: «0 строк при фильтре по кампании» больше не мёртвая проверка — живой Reports
14+
всегда возвращает строку-заголовок, поэтому теперь считаем DATA-строки, а не пустой TSV; при 0
15+
строках с фильтром `campaignIds` падаем с понятной ошибкой (SEARCH_QUERY-агрегация не затронута).
16+
- `get_statistics`: одиночная граница диапазона (`dateFrom` без `dateTo` или наоборот) — теперь
17+
ошибка, а не молчаливый `LAST_30_DAYS`; пара дат вместе с предустановленным `dateRangeType`
18+
форсирует `CUSTOM_DATE` вместо тихого игнорирования дат.
19+
- `get_bid_modifiers`: запрашивается `VideoAdjustmentFieldNames` — корректировка для видео
20+
(VIDEO_ADJUSTMENT) больше не теряется молча.
21+
- `list_campaigns`: денежные поля единого счёта (`Funds`: Sum, Balance, SumAvailableForTransfer,
22+
Spend) конвертируются из микроединиц в валюту аккаунта, как и обещает описание тула.
23+
- `getAll` (autoPaginate): при обрезке на потолке страниц `LimitedBy` теперь указывает курсор
24+
после последней слитой страницы (а не устаревшее значение первой), чтобы ручная пагинация с
25+
`offset` продолжалась с правильного места.
26+
27+
### Безопасность / устойчивость
28+
- Клиент: HTTP 5xx и сетевые ошибки/таймауты повторяются только для идемпотентных (read: get/has/
29+
check) методов — write (add/update/delete/set) больше не рискует продублироваться после ошибки
30+
шлюза. Rate-limit коды (429/506/52) по-прежнему повторяются для любого метода. То же для `callV4`
31+
(повтор только при `Action=Get`).
32+
- Клиент: таймаут теперь покрывает и чтение тела ответа (тело читается внутри охраняемой зоны
33+
`fetchWithTimeout`), а не только заголовки.
34+
- `raw_request`/клиент: defense-in-depth-валидация `service` (не должен содержать `://` или
35+
начинаться с `/`) — путь не может увести запрос с Authorization-заголовком на чужой хост.
36+
- `upload_ad_image`: загрузка картинки по URL ограничена по времени (AbortController) и размеру
37+
(>10 MB → ошибка до кодирования, по Content-Length и по факту), не-http(s) URL отклоняются.
38+
39+
### Добавлено / улучшено
40+
- `YandexDirectError`: `request_id` дописывается в текст ошибки (когда есть) — проще диагностировать.
41+
- `get_regions`: справочник GeoRegions кэшируется на клиента (не качается заново на каждый вызов).
42+
- `get_statistics` (SEARCH_QUERY): `zeroConversionsOnly` без `Conversions` в `fieldNames` теперь
43+
явная ошибка с подсказкой, а не тихо проигнорированный фильтр.
44+
- Единый потолок `limit` (`MAX_TOOL_LIMIT`) в тулах assets/media вместо магического `10000`.
45+
46+
### Документация
47+
- README/`docs/DEVELOPMENT.md`: требуемая версия Node — 20+ (было 18+); CI-матрица 20/22/24.
48+
- `docs/TOOLS.md`: добавлена строка `get_balance`; уточнён формат вывода `get_statistics`
49+
(TSV для обычных типов, вычисленная JSON-сводка для SEARCH_QUERY).
50+
851
## [1.1.5] — 2026-07-01
952

1053
### Добавлено

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ YANDEX_DIRECT_TOKEN = "ваш_токен"
164164

165165
## Требования
166166

167-
- Node.js 18+ (запускается через `npx`, отдельная установка не нужна).
167+
- Node.js 20+ (запускается через `npx`, отдельная установка не нужна).
168168
- OAuth-токен Яндекс Директа — см. [Получение токена](#получение-токена).
169169

170170
## Ограничения

docs/DEVELOPMENT.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Разработка
22

3-
Требования: Node.js 18+.
3+
Требования: Node.js 20+.
44

55
```bash
66
npm install
@@ -40,4 +40,4 @@ YANDEX_DIRECT_SANDBOX=true YANDEX_DIRECT_TOKEN=ваш_токен npm run smoke
4040

4141
## CI
4242

43-
GitHub Actions прогоняет `typecheck` + `build` + `test` на Node 18/20/22 при push и pull request.
43+
GitHub Actions прогоняет `typecheck` + `build` + `test` на Node 20/22/24 при push и pull request.

docs/TOOLS.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
| Инструмент | Что делает |
88
| --- | --- |
99
| `get_account_info` | Данные аккаунта: логин, валюта, тип, страна. |
10+
| `get_balance` | Баланс единого счёта (Amount, AmountAvailableForTransfer, Currency) через Live v4 `AccountManagement` — единственный метод API, отдающий баланс. Деньги в валюте счёта; отрицательная сумма = задолженность. |
1011
| `get_quota` | Остаток дневной квоты API (Units: потрачено / осталось / лимит). |
1112
| `get_regions` | Поиск id регионов по названию (нужны для `create_ad_group`). |
1213
| `get_dictionaries` | Справочники (валюты, часовые пояса, константы, …). |
@@ -43,7 +44,7 @@
4344

4445
| Инструмент | Что делает |
4546
| --- | --- |
46-
| `get_statistics` | TSV-отчёт через сервис Reports. |
47+
| `get_statistics` | Отчёт через сервис Reports. Обычные типы (CAMPAIGN/ADGROUP/AD/CRITERIA/ACCOUNT) возвращают TSV-строки; `SEARCH_QUERY_PERFORMANCE_REPORT` — вычисленную сводку (JSON: тоталы по всем строкам + топ-N + хвост + счётчики нулевых кликов/конверсий). |
4748
| `raw_request` | Прямой вызов любого сервиса/метода (полное покрытие API). Записи требуют `confirmWrite=true`. |
4849

4950
## Деньги и пагинация

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "mcp-yandex-direct",
3-
"version": "1.1.5",
3+
"version": "1.2.0",
44
"description": "MCP server for the Yandex Direct API v5 — manage PPC campaigns, ad groups, ads, keywords and pull statistics from AI agents.",
55
"mcpName": "io.github.askads/mcp-yandex-direct",
66
"type": "module",

server.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@
88
"url": "https://github.com/askads/mcp-yandex-direct",
99
"source": "github"
1010
},
11-
"version": "1.1.5",
11+
"version": "1.2.0",
1212
"packages": [
1313
{
1414
"registryType": "npm",
1515
"registryBaseUrl": "https://registry.npmjs.org",
1616
"identifier": "mcp-yandex-direct",
17-
"version": "1.1.5",
17+
"version": "1.2.0",
1818
"transport": {
1919
"type": "stdio"
2020
},

src/client.test.ts

Lines changed: 151 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,41 @@ test("callV4() targets the sandbox v4 base in sandbox mode", async () => {
6666
}
6767
});
6868

69+
test("callV4() retries a 5xx for a Get action then returns data", async () => {
70+
let calls = 0;
71+
const mock = mockFetch(() => {
72+
calls++;
73+
if (calls === 1) return new Response("gateway", { status: 502 });
74+
return new Response(JSON.stringify({ data: { Accounts: [] } }), { status: 200 });
75+
});
76+
try {
77+
const client = new YandexDirectClient({ token: "T", lang: "ru", sandbox: false, retryBaseMs: 0 });
78+
const result = await client.callV4("AccountManagement", { Action: "Get", SelectionCriteria: {} });
79+
assert.deepEqual(result, { Accounts: [] });
80+
assert.equal(calls, 2);
81+
} finally {
82+
mock.restore();
83+
}
84+
});
85+
86+
test("callV4() does NOT retry a 5xx for a non-Get action (no duplicate write)", async () => {
87+
let calls = 0;
88+
const mock = mockFetch(() => {
89+
calls++;
90+
return new Response("gateway", { status: 502 });
91+
});
92+
try {
93+
const client = new YandexDirectClient({ token: "T", lang: "ru", sandbox: false, retryBaseMs: 0 });
94+
await assert.rejects(
95+
() => client.callV4("AccountManagement", { Action: "Update" }),
96+
/Live v4 "AccountManagement" failed with HTTP 502/,
97+
);
98+
assert.equal(calls, 1);
99+
} finally {
100+
mock.restore();
101+
}
102+
});
103+
69104
test("call() targets sandbox, sends bearer token and parses result", async () => {
70105
const mock = mockFetch(
71106
() => new Response(JSON.stringify({ result: { Campaigns: [] } }), { status: 200 }),
@@ -214,7 +249,10 @@ test("getAll stops at maxPages and flags the truncation loudly", async () => {
214249
// Hitting the cap is explicit, not a bare LimitedBy that the model may ignore.
215250
assert.equal(result._truncated, true);
216251
assert.match(result._truncatedNote ?? "", /more objects remain/);
217-
assert.notEqual(result.LimitedBy, undefined);
252+
// LimitedBy is the cursor AFTER the last merged page (page 2 → offset 2), not the stale
253+
// page-1 value copied from the first page's scalar (which was 1).
254+
assert.equal(result.LimitedBy, 2);
255+
assert.match(result._truncatedNote ?? "", /LimitedBy=2/);
218256
} finally {
219257
mock.restore();
220258
}
@@ -327,7 +365,14 @@ test("call() aborts and reports a timeout when the request hangs", async () => {
327365
);
328366
})) as typeof fetch;
329367
try {
330-
const client = new YandexDirectClient({ token: "T", lang: "ru", sandbox: true, timeoutMs: 10 });
368+
// maxRetries:0 so the timeout surfaces immediately (a hung read is otherwise retried).
369+
const client = new YandexDirectClient({
370+
token: "T",
371+
lang: "ru",
372+
sandbox: true,
373+
timeoutMs: 10,
374+
maxRetries: 0,
375+
});
331376
await assert.rejects(() => client.call("campaigns", "get", {}), /timed out after 10ms/);
332377
} finally {
333378
globalThis.fetch = original;
@@ -371,3 +416,107 @@ test("report() gives up on a persistent 5xx after maxPolls", async () => {
371416
mock.restore();
372417
}
373418
});
419+
420+
test("call() rejects a service path that resolves to a foreign origin and never fetches", async () => {
421+
// SSRF guard: an absolute/scheme-bearing service, or a backslash/protocol-relative one,
422+
// resolves to a foreign origin and would rebase the token-bearing request onto another
423+
// host — reject before fetching. (The backslash form slips past a naive `startsWith("/")`
424+
// string test, which is why the guard compares the resolved origin.)
425+
const mock = mockFetch(() => new Response(JSON.stringify({ result: {} }), { status: 200 }));
426+
try {
427+
const client = new YandexDirectClient({ token: "T", lang: "ru", sandbox: true });
428+
for (const evil of ["https://evil.example/steal", "http://evil.example/x", "\\\\evil.example/x"]) {
429+
await assert.rejects(() => client.call(evil, "get", {}), /foreign origin/);
430+
}
431+
assert.equal(mock.calls.length, 0);
432+
// A normal relative service still works.
433+
const result = await client.call("campaigns", "get", {});
434+
assert.deepEqual(result, {});
435+
assert.equal(mock.calls.length, 1);
436+
} finally {
437+
mock.restore();
438+
}
439+
});
440+
441+
test("call() does NOT retry an HTTP 5xx for a write method (no duplicate write)", async () => {
442+
// A write (add/update/delete/set) may have committed before the gateway error, so a blind
443+
// retry could duplicate it. Only reads (get/has/check) are retried on 5xx.
444+
let calls = 0;
445+
const mock = mockFetch(() => {
446+
calls++;
447+
return new Response("bad gateway", { status: 502 });
448+
});
449+
try {
450+
const client = new YandexDirectClient({ token: "T", lang: "ru", sandbox: true, retryBaseMs: 0 });
451+
await assert.rejects(() => client.call("campaigns", "add", {}), /HTTP 502/);
452+
assert.equal(calls, 1); // single attempt, no retry
453+
} finally {
454+
mock.restore();
455+
}
456+
});
457+
458+
test("call() retries a rate-limit code even for a write method (request not processed)", async () => {
459+
// 506/52 mean the request was NOT processed (like 429), so retrying a write is safe.
460+
let calls = 0;
461+
const mock = mockFetch(() => {
462+
calls++;
463+
if (calls === 1) {
464+
return new Response(
465+
JSON.stringify({ error: { error_code: 506, error_string: "Too many requests" } }),
466+
{ status: 200 },
467+
);
468+
}
469+
return new Response(JSON.stringify({ result: { AddResults: [{ Id: 1 }] } }), { status: 200 });
470+
});
471+
try {
472+
const client = new YandexDirectClient({ token: "T", lang: "ru", sandbox: true, retryBaseMs: 0 });
473+
const result = await client.call("campaigns", "add", {});
474+
assert.deepEqual(result, { AddResults: [{ Id: 1 }] });
475+
assert.equal(calls, 2);
476+
} finally {
477+
mock.restore();
478+
}
479+
});
480+
481+
test("call() retries a network error for a read method, then succeeds", async () => {
482+
let calls = 0;
483+
const mock = mockFetch(() => {
484+
calls++;
485+
if (calls === 1) throw Object.assign(new Error("ECONNRESET"), { code: "ECONNRESET" });
486+
return new Response(JSON.stringify({ result: { ok: true } }), { status: 200 });
487+
});
488+
try {
489+
const client = new YandexDirectClient({ token: "T", lang: "ru", sandbox: true, retryBaseMs: 0 });
490+
const result = await client.call("campaigns", "get", {});
491+
assert.deepEqual(result, { ok: true });
492+
assert.equal(calls, 2);
493+
} finally {
494+
mock.restore();
495+
}
496+
});
497+
498+
test("call() does NOT retry a network error for a write method", async () => {
499+
let calls = 0;
500+
const mock = mockFetch(() => {
501+
calls++;
502+
throw Object.assign(new Error("ECONNRESET"), { code: "ECONNRESET" });
503+
});
504+
try {
505+
const client = new YandexDirectClient({ token: "T", lang: "ru", sandbox: true, retryBaseMs: 0 });
506+
await assert.rejects(() => client.call("campaigns", "add", {}), /ECONNRESET/);
507+
assert.equal(calls, 1);
508+
} finally {
509+
mock.restore();
510+
}
511+
});
512+
513+
test("YandexDirectError appends request_id to the message when present", () => {
514+
const err = new YandexDirectError({
515+
error_code: 54,
516+
error_string: "No units",
517+
request_id: "abc123",
518+
});
519+
assert.match(err.message, /\[54\] No units/);
520+
assert.match(err.message, /request_id: abc123/);
521+
assert.equal(err.requestId, "abc123");
522+
});

0 commit comments

Comments
 (0)