fix: review fixes — yield curve key, input validation, NaN guard, CI/CD

- Fix yield curve issuerType: G_N_C (AAA) default, G_N_A for all_gov
- Add input validation on SDMX key segments (reject path traversal)
- Add NaN guard on parseFloat in convert_currency
- Add .int().positive() to lastNObservations Zod schemas
- Fix timer leak in fetchWithRetry (try/finally on clearTimeout)
- Add CI workflow (test on PR, Node 18/20/22)
- Add publish workflow (npm publish on release)

Author: scka-de

.github/workflows/ci.yml

@@ -0,0 +1,24 @@
+name: CI
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+    branches: [main]
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        node-version: [18, 20, 22]
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-node@v4
+        with:
+          node-version: ${{ matrix.node-version }}
+          cache: npm
+      - run: npm ci
+      - run: npm run lint
+      - run: npm test
+      - run: npm run build

.github/workflows/publish.yml

@@ -0,0 +1,25 @@
+name: Publish to npm
+
+on:
+  release:
+    types: [published]
+
+jobs:
+  publish:
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      id-token: write
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-node@v4
+        with:
+          node-version: 22
+          registry-url: https://registry.npmjs.org
+          cache: npm
+      - run: npm ci
+      - run: npm test
+      - run: npm run build
+      - run: npm publish --provenance --access public
+        env:
+          NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}

src/ecb-client.ts

@@ -46,12 +46,16 @@ async function fetchWithRetry(
       const controller = new AbortController();
       const timeout = setTimeout(() => controller.abort(), config.timeoutMs);
 
-      const response = await fetch(url, {
-        method: "GET",
-        headers,
-        signal: controller.signal,
-      });
-      clearTimeout(timeout);
+      let response: Response;
+      try {
+        response = await fetch(url, {
+          method: "GET",
+          headers,
+          signal: controller.signal,
+        });
+      } finally {
+        clearTimeout(timeout);
+      }
 
       if (response.status >= 500 && attempt < config.maxRetries) {
         const delay = 2 ** attempt * 1000;

src/index.ts

@@ -55,6 +55,8 @@ Examples of questions this tool answers:
       .describe("End date (YYYY-MM-DD, YYYY-MM, or YYYY)"),
     lastNObservations: z
       .number()
+      .int()
+      .positive()
       .optional()
       .describe("Return only the last N data points"),
   },
@@ -91,6 +93,8 @@ Examples of questions this tool answers:
       .describe("End date (YYYY-MM-DD, YYYY-MM, or YYYY)"),
     lastNObservations: z
       .number()
+      .int()
+      .positive()
       .optional()
       .describe("Return only the last N data points"),
   },
@@ -131,6 +135,8 @@ Examples of questions this tool answers:
     endPeriod: z.string().optional().describe("End date (YYYY-MM, or YYYY)"),
     lastNObservations: z
       .number()
+      .int()
+      .positive()
       .optional()
       .describe("Return only the last N data points"),
   },
@@ -174,6 +180,8 @@ Examples of questions this tool answers:
       .describe("End date (YYYY-MM-DD, YYYY-MM, or YYYY)"),
     lastNObservations: z
       .number()
+      .int()
+      .positive()
       .optional()
       .describe("Return only the last N data points"),
   },
@@ -218,6 +226,8 @@ Examples of questions this tool answers:
     endPeriod: z.string().optional().describe("End date (YYYY-MM, or YYYY)"),
     lastNObservations: z
       .number()
+      .int()
+      .positive()
       .optional()
       .describe("Return only the last N data points"),
   },

src/sdmx-keys.ts

@@ -47,17 +47,29 @@ const ICP_MEASURE_MAP: Record<string, string> = {
   INDEX: "INX",
 };
 
+const VALID_SEGMENT = /^[A-Z0-9_+]+$/;
+
 function normalize(value: string): string {
   return value.trim().toUpperCase();
 }
 
+function validateSegment(value: string, label: string): string {
+  const normalized = normalize(value);
+  if (!VALID_SEGMENT.test(normalized)) {
+    throw new Error(
+      `Invalid ${label}: "${value}". Only letters, digits, and underscores are allowed.`,
+    );
+  }
+  return normalized;
+}
+
 /**
  * Exchange rates (EXR dataflow).
  * Template: {freq}.{target}.EUR.SP00.A
  */
 export function buildExrKey(target: string, freq = "D"): string {
-  const f = FREQ_MAP[normalize(freq)] || normalize(freq);
-  const t = normalize(target);
+  const f = FREQ_MAP[normalize(freq)] || validateSegment(freq, "frequency");
+  const t = validateSegment(target, "currency");
   return `${f}.${t}.EUR.SP00.A`;
 }
 
@@ -87,15 +99,15 @@ export function buildFmKey(type: string): string {
 
 /**
  * Yield curve (YC dataflow).
- * Template: B.U2.EUR.4F.G_N_A.SV_C_YM.{maturity}
+ * Template: B.U2.EUR.4F.{issuer}.SV_C_YM.{maturity}
  *
- * If issuerType is "all_gov", uses G_N_A (all government bonds).
- * If "aaa", uses G_N_A (AAA-rated, which is the ECB default for YC).
+ * issuerType "aaa" (default) uses G_N_C (AAA-rated government bonds).
+ * issuerType "all_gov" uses G_N_A (all government bonds regardless of rating).
  */
 export function buildYcKey(maturity?: string, issuerType?: string): string {
   const m = maturity ? `SR_${normalize(maturity)}` : "SR_10Y";
   const issuer =
-    issuerType && normalize(issuerType) === "ALL_GOV" ? "G_N_A" : "G_N_A";
+    issuerType && normalize(issuerType) === "ALL_GOV" ? "G_N_A" : "G_N_C";
   return `B.U2.EUR.4F.${issuer}.SV_C_YM.${m}`;
 }
 
@@ -107,8 +119,9 @@ export function buildYcKey(maturity?: string, issuerType?: string): string {
  * Measure defaults to "ANR" (annual rate of change).
  */
 export function buildIcpKey(country = "U2", measure = "annual_rate"): string {
-  const c = normalize(country);
-  const m = ICP_MEASURE_MAP[normalize(measure)] || normalize(measure);
+  const c = validateSegment(country, "country");
+  const m =
+    ICP_MEASURE_MAP[normalize(measure)] || validateSegment(measure, "measure");
   return `M.${c}.N.000000.4.${m}`;
 }
 
@@ -125,7 +138,7 @@ export function buildBsiKey(
   measure = "outstanding",
   country = "U2",
 ): string {
-  const c = normalize(country);
+  const c = validateSegment(country, "country");
   const agg = normalize(aggregate);
   const item = MONEY_AGGREGATE_MAP[agg];
   if (!item) {

src/tools/exchange-rate.ts

@@ -90,6 +90,9 @@ export async function handleConvertCurrency(
       return `No exchange rate data available for EUR/${to}.`;
     }
     rate = Number.parseFloat(rows[rows.length - 1].OBS_VALUE);
+    if (Number.isNaN(rate)) {
+      return `Exchange rate for EUR/${to} is not available (missing observation).`;
+    }
     rateDate = rows[rows.length - 1].TIME_PERIOD;
     result = amount * rate;
   } else if (isToEur) {
@@ -101,6 +104,9 @@ export async function handleConvertCurrency(
       return `No exchange rate data available for EUR/${from}.`;
     }
     rate = Number.parseFloat(rows[rows.length - 1].OBS_VALUE);
+    if (Number.isNaN(rate)) {
+      return `Exchange rate for EUR/${from} is not available (missing observation).`;
+    }
     rateDate = rows[rows.length - 1].TIME_PERIOD;
     result = amount / rate;
   } else {
@@ -126,6 +132,9 @@ export async function handleConvertCurrency(
 
     const fromRate = Number.parseFloat(fromRow.OBS_VALUE);
     const toRate = Number.parseFloat(toRow.OBS_VALUE);
+    if (Number.isNaN(fromRate) || Number.isNaN(toRate)) {
+      return `Exchange rate data is missing for ${Number.isNaN(fromRate) ? from : to}. Cannot compute cross-rate.`;
+    }
     rate = toRate / fromRate;
     rateDate = fromDate;
     result = amount * rate;

tests/sdmx-keys.test.ts

@@ -81,19 +81,29 @@ describe("buildFmKey", () => {
 
 describe("buildYcKey", () => {
   it("builds 10Y AAA yield curve key (defaults)", () => {
-    expect(buildYcKey()).toBe("B.U2.EUR.4F.G_N_A.SV_C_YM.SR_10Y");
+    expect(buildYcKey()).toBe("B.U2.EUR.4F.G_N_C.SV_C_YM.SR_10Y");
   });
 
   it("builds specific maturity", () => {
-    expect(buildYcKey("5Y")).toBe("B.U2.EUR.4F.G_N_A.SV_C_YM.SR_5Y");
+    expect(buildYcKey("5Y")).toBe("B.U2.EUR.4F.G_N_C.SV_C_YM.SR_5Y");
   });
 
   it("builds 3M maturity", () => {
-    expect(buildYcKey("3M")).toBe("B.U2.EUR.4F.G_N_A.SV_C_YM.SR_3M");
+    expect(buildYcKey("3M")).toBe("B.U2.EUR.4F.G_N_C.SV_C_YM.SR_3M");
   });
 
   it("is case-insensitive for maturity", () => {
-    expect(buildYcKey("10y")).toBe("B.U2.EUR.4F.G_N_A.SV_C_YM.SR_10Y");
+    expect(buildYcKey("10y")).toBe("B.U2.EUR.4F.G_N_C.SV_C_YM.SR_10Y");
+  });
+
+  it("uses G_N_A for all_gov issuer type", () => {
+    expect(buildYcKey("10Y", "all_gov")).toBe(
+      "B.U2.EUR.4F.G_N_A.SV_C_YM.SR_10Y",
+    );
+  });
+
+  it("uses G_N_C (AAA) by default", () => {
+    expect(buildYcKey("10Y", "aaa")).toBe("B.U2.EUR.4F.G_N_C.SV_C_YM.SR_10Y");
   });
 });
 

tests/tools/yield-curve.test.ts

@@ -44,7 +44,7 @@ describe("handleGetYieldCurve", () => {
 
     expect(client.fetchData).toHaveBeenCalledWith(
       "YC",
-      "B.U2.EUR.4F.G_N_A.SV_C_YM.SR_10Y",
+      "B.U2.EUR.4F.G_N_C.SV_C_YM.SR_10Y",
       expect.any(Object),
     );
   });
@@ -55,7 +55,7 @@ describe("handleGetYieldCurve", () => {
 
     expect(client.fetchData).toHaveBeenCalledWith(
       "YC",
-      "B.U2.EUR.4F.G_N_A.SV_C_YM.SR_5Y",
+      "B.U2.EUR.4F.G_N_C.SV_C_YM.SR_5Y",
       expect.any(Object),
     );
   });