Skip to content

Commit 4b6a5f7

Browse files
committed
fix: remove secret cache (breaks rotation) + fix deploy script trap overwrite
Address PR #31 review feedback: 1. Remove secretCache from get-secret.ts - Caching versions/latest forever prevents secret rotation from taking effect on warm Cloud Run instances (🔴 critical regression per review) - The singleton SecretManagerServiceClient alone fixes the gRPC leak; per-request round-trips to Secret Manager are acceptable - Added explicit comment explaining why we don't cache 2. Fix EXIT trap overwrite in deploy-quicknode-filter.sh - Per-call 'trap rm EXIT' in deploy_webhook() overwrote the previous trap on repeated calls, leaking the first temp file - Replace with global TMP_FILES array + single script-level EXIT trap (same fix proposed by Cursor Bugbot)
1 parent 809dfed commit 4b6a5f7

File tree

2 files changed

+19
-16
lines changed

2 files changed

+19
-16
lines changed

bin/deploy-quicknode-filter.sh

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,17 @@ GOVERNOR_WEBHOOK_ID="73a99141-e8cb-411a-9732-c42a031cebe6"
3333

3434
QN_API_BASE="https://api.quicknode.com/webhooks/rest/v1/webhooks"
3535

36+
# Global array to track temp files created by deploy_webhook invocations.
37+
# A single EXIT trap at script level cleans them all up, avoiding the problem
38+
# of per-call traps overwriting the previous trap registration.
39+
TMP_FILES=()
40+
cleanup_temp_files() {
41+
if ((${#TMP_FILES[@]} > 0)); then
42+
rm -f "${TMP_FILES[@]}"
43+
fi
44+
}
45+
trap cleanup_temp_files EXIT
46+
3647
# ------------------------------------------------------------------------------
3748
log() { printf '\n\033[1m%s\033[0m\n' "$*"; }
3849
success() { printf '✅ %s\n' "$*"; }
@@ -96,9 +107,7 @@ deploy_webhook() {
96107
# The internal template ID for PATCH is "evmAbiFilterGo" (evmAbiFilter is the display name).
97108
local payload_file
98109
payload_file=$(mktemp /tmp/qn_payload.XXXXXX.json)
99-
# Ensure temp file is always cleaned up, even on SIGINT/SIGTERM
100-
# shellcheck disable=SC2064
101-
trap "rm -f '${payload_file}'" EXIT
110+
TMP_FILES+=("${payload_file}")
102111

103112
# Build templateArgs payload: abiJson must be a raw JSON string (not a parsed object).
104113
# Use env vars to avoid shell quoting issues with large ABI strings.

src/utils/get-secret.ts

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,24 +4,19 @@ import config from "../config.js";
44
/**
55
* Singleton gRPC client — creating a new SecretManagerServiceClient per request
66
* leaks gRPC channels and causes memory growth (~488 MiB OOM within 30 min).
7+
* One client is created at module load time and reused for all requests.
8+
*
9+
* Note: we intentionally do NOT cache secret values here. Caching `versions/latest`
10+
* would break secret rotation — rotated credentials would not take effect until Cloud
11+
* Run recycles the instance. The singleton client alone eliminates the gRPC leak.
712
*/
813
const secretManager = new SecretManagerServiceClient();
914

1015
/**
11-
* In-memory cache for secrets. Secrets don't change during an instance's lifetime,
12-
* so we can safely cache them and avoid repeated Secret Manager round-trips.
13-
*/
14-
const secretCache = new Map<string, string>();
15-
16-
/**
17-
* Load a secret from Secret Manager (cached per instance lifetime)
16+
* Load a secret from Secret Manager.
17+
* Uses a singleton gRPC client to prevent channel leaks on warm instances.
1818
*/
1919
export default async function getSecret(secretId: string): Promise<string> {
20-
const cached = secretCache.get(secretId);
21-
if (cached) {
22-
return cached;
23-
}
24-
2520
try {
2621
const secretFullResourceName = `projects/${config.GCP_PROJECT_ID}/secrets/${secretId}/versions/latest`;
2722
const [version] = await secretManager.accessSecretVersion({
@@ -36,7 +31,6 @@ export default async function getSecret(secretId: string): Promise<string> {
3631
);
3732
}
3833

39-
secretCache.set(secretId, secret);
4034
return secret;
4135
} catch (error) {
4236
console.error(

0 commit comments

Comments
 (0)