mirror of
https://github.com/NomaDamas/k-skill.git
synced 2026-06-24 02:04:11 +00:00
Protect hosted Seoul Bike proxy secrets
Sanitize Seoul Bike upstream fetch and parse failures before they can reach the global error handler, and reject blank nearby coordinates before JavaScript can coerce them to zero.\n\nConstraint: PR #277 round-3 review found server-side Seoul Open API keys could leak through exception messages containing keyed upstream URLs.\nRejected: Letting the global error handler format Seoul Bike upstream exceptions | it echoes exception messages and can expose the hosted proxy API key.\nConfidence: high\nScope-risk: narrow\nDirective: Keep server-side API-key-bearing upstream URLs out of client-visible error messages and logs for hosted no-user-key routes.\nTested: node --test packages/k-skill-proxy/test/server.test.js --test-name-pattern 'seoul bike'; PYTHONPATH=.:scripts python3 -m unittest scripts.test_seoul_bike; explicit app.inject smoke for sanitized Seoul Bike failures and blank coordinates; local fake-proxy seoul-bike nearby smoke; PATH="/Users/jeffrey/.pyenv/versions/3.11.9/bin:/Users/jeffrey/.codex/tmp/arg0/codex-arg0mxZmWx:/opt/homebrew/lib/node_modules/@openai/codex/node_modules/@openai/codex-darwin-arm64/vendor/aarch64-apple-darwin/path:/Users/jeffrey/.cmuxterm/omo-bin:/opt/homebrew/share/android-commandlinetools/platform-tools:/opt/homebrew/share/android-commandlinetools/emulator:/opt/homebrew/share/android-commandlinetools/cmdline-tools/latest/bin:/Users/jeffrey/.local/bin:/Users/jeffrey/.bun/bin:/opt/homebrew/opt/node@22/bin:/opt/homebrew/opt/openjdk@21/bin:/opt/homebrew/opt/postgresql@18/bin:/Users/jeffrey/.jenv/shims:/Users/jeffrey/.jenv/bin:/opt/homebrew/opt/imagemagick/bin:/opt/homebrew/Cellar/pyenv-virtualenv/1.4.0/shims:/Users/jeffrey/.pyenv/shims:/opt/homebrew/opt/openssl@3/bin:/Users/jeffrey/.rbenv/shims:/Users/jeffrey/.rbenv/bin:/Users/jeffrey/google-cloud-sdk/bin:/Applications/cmux.app/Contents/Resources/bin:/Users/jeffrey/Library/pnpm:/Users/jeffrey/.nvm/versions/node/v24.13.0/bin:/Users/jeffrey/.cops/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/opt/pmk/env/global/bin:/Library/Apple/usr/bin:/Library/TeX/texbin:/Users/jeffrey/.cargo/bin:/Users/jeffrey/Library/Application Support/JetBrains/Toolbox/scripts:/Library/Java/JavaVirtualMachines/zulu-17.jdk/Contents/Home/bin:/Users/jeffrey/xcode-projects/marshroom/cli" npm run ci.\nNot-tested: Live Seoul Open API network failure from production Cloud Run.
This commit is contained in:
Jeffrey (Dongkyu) Kim
parent
3965cd7396
commit
3ded0a049c
2 changed files with 112 additions and 14 deletions
|
|
@ -529,7 +529,10 @@ function parseNumberAlias(query, keys, { min, max, label }) {
|
|||
break;
|
||||
}
|
||||
}
|
||||
const value = Number(raw);
|
||||
if (typeof raw !== "string" || raw.trim() === "") {
|
||||
throw new Error(`Provide valid ${label}.`);
|
||||
}
|
||||
const value = Number(raw.trim());
|
||||
if (!Number.isFinite(value) || value < min || value > max) {
|
||||
throw new Error(`Provide valid ${label}.`);
|
||||
}
|
||||
|
|
@ -690,6 +693,18 @@ function buildSeoulBikeSemanticErrorPayload(error, config) {
|
|||
};
|
||||
}
|
||||
|
||||
function buildSeoulBikeUpstreamErrorPayload(config) {
|
||||
return {
|
||||
error: "upstream_error",
|
||||
message: "Seoul Bike upstream request failed.",
|
||||
proxy: {
|
||||
name: config.proxyName,
|
||||
cache: { hit: false, ttl_ms: config.cacheTtlMs },
|
||||
requested_at: new Date().toISOString()
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
function normalizeKosisSearchQuery(query) {
|
||||
const searchNm = trimOrNull(query.searchNm ?? query.search_nm ?? query.query ?? query.q);
|
||||
if (!searchNm) {
|
||||
|
|
@ -1998,10 +2013,16 @@ function buildServer({ env = process.env, provider = null, now = () => new Date(
|
|||
};
|
||||
}
|
||||
|
||||
const upstream = await proxySeoulBikeRealtimeRequest({
|
||||
...normalized,
|
||||
apiKey: config.seoulOpenApiKey
|
||||
});
|
||||
let upstream;
|
||||
try {
|
||||
upstream = await proxySeoulBikeRealtimeRequest({
|
||||
...normalized,
|
||||
apiKey: config.seoulOpenApiKey
|
||||
});
|
||||
} catch {
|
||||
reply.code(502);
|
||||
return buildSeoulBikeUpstreamErrorPayload(config);
|
||||
}
|
||||
|
||||
reply.code(upstream.statusCode);
|
||||
reply.header("content-type", upstream.contentType);
|
||||
|
|
@ -2009,7 +2030,13 @@ function buildServer({ env = process.env, provider = null, now = () => new Date(
|
|||
return upstream.body;
|
||||
}
|
||||
|
||||
const payload = JSON.parse(upstream.body);
|
||||
let payload;
|
||||
try {
|
||||
payload = JSON.parse(upstream.body);
|
||||
} catch {
|
||||
reply.code(502);
|
||||
return buildSeoulBikeUpstreamErrorPayload(config);
|
||||
}
|
||||
const semanticError = getSeoulOpenApiSemanticError(payload);
|
||||
if (semanticError) {
|
||||
reply.code(502);
|
||||
|
|
@ -2051,10 +2078,16 @@ function buildServer({ env = process.env, provider = null, now = () => new Date(
|
|||
};
|
||||
}
|
||||
|
||||
const upstream = await proxySeoulBikeStationsRequest({
|
||||
...normalized,
|
||||
apiKey: config.seoulOpenApiKey
|
||||
});
|
||||
let upstream;
|
||||
try {
|
||||
upstream = await proxySeoulBikeStationsRequest({
|
||||
...normalized,
|
||||
apiKey: config.seoulOpenApiKey
|
||||
});
|
||||
} catch {
|
||||
reply.code(502);
|
||||
return buildSeoulBikeUpstreamErrorPayload(config);
|
||||
}
|
||||
|
||||
reply.code(upstream.statusCode);
|
||||
reply.header("content-type", upstream.contentType);
|
||||
|
|
@ -2062,7 +2095,13 @@ function buildServer({ env = process.env, provider = null, now = () => new Date(
|
|||
return upstream.body;
|
||||
}
|
||||
|
||||
const payload = JSON.parse(upstream.body);
|
||||
let payload;
|
||||
try {
|
||||
payload = JSON.parse(upstream.body);
|
||||
} catch {
|
||||
reply.code(502);
|
||||
return buildSeoulBikeUpstreamErrorPayload(config);
|
||||
}
|
||||
const semanticError = getSeoulOpenApiSemanticError(payload);
|
||||
if (semanticError) {
|
||||
reply.code(502);
|
||||
|
|
@ -2104,9 +2143,17 @@ function buildServer({ env = process.env, provider = null, now = () => new Date(
|
|||
};
|
||||
}
|
||||
|
||||
const { upstream, rows, semanticError } = await fetchAllSeoulBikeRealtimeRows({
|
||||
apiKey: config.seoulOpenApiKey
|
||||
});
|
||||
let realtimeResult;
|
||||
try {
|
||||
realtimeResult = await fetchAllSeoulBikeRealtimeRows({
|
||||
apiKey: config.seoulOpenApiKey
|
||||
});
|
||||
} catch {
|
||||
reply.code(502);
|
||||
return buildSeoulBikeUpstreamErrorPayload(config);
|
||||
}
|
||||
|
||||
const { upstream, rows, semanticError } = realtimeResult;
|
||||
|
||||
reply.code(upstream.statusCode);
|
||||
reply.header("content-type", upstream.contentType);
|
||||
|
|
|
|||
|
|
@ -2073,6 +2073,48 @@ test("seoul bike realtime and stations endpoints do not cache upstream semantic
|
|||
assert.equal(stations.json().upstream.code, "ERROR-336");
|
||||
});
|
||||
|
||||
test("seoul bike routes sanitize upstream fetch failures without leaking API keys", async (t) => {
|
||||
const originalFetch = global.fetch;
|
||||
const secret = "SECRETSEOULKEY";
|
||||
let fetchCalls = 0;
|
||||
global.fetch = async (url) => {
|
||||
fetchCalls += 1;
|
||||
throw new Error(`network failure ${url}`);
|
||||
};
|
||||
|
||||
const app = buildServer({
|
||||
env: {
|
||||
SEOUL_OPEN_API_KEY: secret,
|
||||
KSKILL_PROXY_CACHE_TTL_MS: "60000"
|
||||
}
|
||||
});
|
||||
|
||||
t.after(async () => {
|
||||
global.fetch = originalFetch;
|
||||
await app.close();
|
||||
});
|
||||
|
||||
for (const url of [
|
||||
"/v1/seoul-bike/realtime?startIndex=1&endIndex=1",
|
||||
"/v1/seoul-bike/stations?startIndex=1&endIndex=1",
|
||||
"/v1/seoul-bike/nearby?lat=37.5717&lon=126.9763&radius_m=120&limit=5"
|
||||
]) {
|
||||
const first = await app.inject({ method: "GET", url });
|
||||
const second = await app.inject({ method: "GET", url });
|
||||
|
||||
assert.equal(first.statusCode, 502, url);
|
||||
assert.equal(second.statusCode, 502, `${url} repeat`);
|
||||
assert.equal(first.json().error, "upstream_error", url);
|
||||
assert.equal(first.json().message, "Seoul Bike upstream request failed.", url);
|
||||
assert.equal(first.json().proxy.cache.hit, false, url);
|
||||
assert.doesNotMatch(first.body, new RegExp(secret), url);
|
||||
assert.doesNotMatch(second.body, new RegExp(secret), `${url} repeat`);
|
||||
assert.doesNotMatch(first.body, /openapi\.seoul\.go\.kr/i, url);
|
||||
}
|
||||
|
||||
assert.equal(fetchCalls, 6, "sanitized upstream failures must not be cached");
|
||||
});
|
||||
|
||||
test("seoul bike nearby endpoint validates coordinates", async (t) => {
|
||||
const app = buildServer({ env: { SEOUL_OPEN_API_KEY: "seoul-key" } });
|
||||
t.after(async () => {
|
||||
|
|
@ -2082,6 +2124,15 @@ test("seoul bike nearby endpoint validates coordinates", async (t) => {
|
|||
const response = await app.inject({ method: "GET", url: "/v1/seoul-bike/nearby?lat=not-a-number&lon=126.9763" });
|
||||
assert.equal(response.statusCode, 400);
|
||||
assert.equal(response.json().error, "bad_request");
|
||||
|
||||
for (const url of [
|
||||
"/v1/seoul-bike/nearby?lat=&lon=126.9763",
|
||||
"/v1/seoul-bike/nearby?lat=37.5717&lon="
|
||||
]) {
|
||||
const blankResponse = await app.inject({ method: "GET", url });
|
||||
assert.equal(blankResponse.statusCode, 400, url);
|
||||
assert.equal(blankResponse.json().error, "bad_request", url);
|
||||
}
|
||||
});
|
||||
|
||||
test("seoul bike endpoints reject partially numeric integer query params", async (t) => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue