fix: correct dependency/build ordering in deploy-staging.sh (#697) - #2
Open
gadst12 wants to merge 391 commits into
Open
fix: correct dependency/build ordering in deploy-staging.sh (#697)#2gadst12 wants to merge 391 commits into
gadst12 wants to merge 391 commits into
Conversation
…d-security-issues Feat/test coverage and security issues
Feat/4 issues fix
Any authenticated scout could previously read another scout's payment
history by substituting a different wallet in the URL path. Every other
scout endpoint already had this guard.
- Add ownership check in scoutController.getPaymentHistory:
if (req.account !== wallet) → 403 Forbidden
- Update tests/routes/payments.test.ts:
* Replace getToken() helper (random keypair) with makeToken(sub) using
jwt.sign so token subject can be controlled precisely
* New test: 403 when JWT wallet does not match path wallet
* New test: 200 when JWT wallet matches path wallet
* Updated existing tests to use a matching token
Closes scout-off#269
Admin routes were accessible from any IP. This adds an opt-in restriction
via the ADMIN_IP_ALLOWLIST environment variable.
Changes:
- Add src/middleware/ipAllowlist.ts:
* Reads ADMIN_IP_ALLOWLIST (comma-separated IPs / CIDR ranges)
* When unset or empty → no-op, all requests pass through (backwards
compatible)
* When set → extracts client IP via extractClientIp (honours
X-Forwarded-For + TRUSTED_PROXY_COUNT), rejects non-matching IPs
with 403
* Pure IPv4 CIDR matching implemented without additional dependencies
- Apply router.use(ipAllowlistMiddleware) at the top of src/routes/admin.ts
so every /api/admin/* route is covered
- Document ADMIN_IP_ALLOWLIST in .env.example with an example value
- Add tests/middleware/ipAllowlist.test.ts:
* Allowed IP passes through
* Blocked IP receives 403
* CIDR range (in-range allowed, out-of-range blocked)
* X-Forwarded-For header is respected
* Missing env var allows all traffic
Closes scout-off#277
The previous implementation accepted any JWT in the request body,
allowing an admin to decode tokens belonging to other users.
Changes:
- Remove body-based token input from introspectToken in adminController.ts
(introspectSchema and zod dependency removed from the function)
- The endpoint now reads the bearer token already present in the
Authorization header and calls jwt.decode() on it — the token was
already verified upstream by the requireRole('admin') middleware
- Returns 401 if the Authorization header is absent or malformed
- Update tests/routes/introspect.test.ts:
* Remove tests that relied on sending a body token field
* Add test: admin sees their own claims (sub, role, iat, exp)
* Add test: body token field is completely ignored, admin still
receives their own identity in the response
* Add test: admin cannot see another user's claims even if their
token is passed in the body
Closes scout-off#279
- Add SubscriptionTier type and SubscriptionStatus interface to types/index.ts - Extract isSubscribed() helper that returns SubscriptionStatus with tier: SubscriptionTier | null - getSubscription now delegates to isSubscribed() so the actual on-chain tier (basic, premium, pro) is returned instead of always hardcoding 'basic' - 'basic' fallback is preserved only for legacy events that carry no tier field - Add tests for premium tier response and explicit basic tier response Closes scout-off#294
…llets - Add db/002_validators.sql migration with validators table schema (wallet TEXT PK, registered_at INTEGER, revoked_at INTEGER, tx_hash TEXT) - Extend indexer.ts DB setup block to CREATE TABLE IF NOT EXISTS validators so the table is initialised alongside events and indexer_state on startup - Export insertValidator(), revokeValidatorRow(), getAllValidators() helpers from indexer.ts to interact with the new table - registerValidator now calls insertValidator() on success (INSERT OR REPLACE) - revokeValidator now calls revokeValidatorRow() to set revoked_at - Add GET /api/admin/validators endpoint (admin-only) backed by getAllValidators() - Extend __mocks__/better-sqlite3.js to handle INSERT OR REPLACE INTO validators and UPDATE validators queries so Jest tests pass without native SQLite - Add tests: 401/403 guards, 200 list, post-register entry visible, post-revoke revoked_at is non-null Closes scout-off#286
…egisterPlayer
- Add requireAuth middleware to POST /api/players/register route so the
request must carry a valid Bearer token before reaching the controller
- In registerPlayer, after schema parse, check parsed.wallet === req.account;
return 403 { success: false, error: 'wallet must match authenticated account' }
when they differ, preventing a player from registering under another player's wallet
- Add tests: 403 on wallet mismatch, 201 on matching wallet, 401 when no token
Closes scout-off#271
- Throw startup error when NODE_ENV=production and ADMIN_WALLET is not set - Log a console.warn in staging when ADMIN_WALLET is not set - Document requirement in .env.example with inline comment - Add unit tests covering production throw, staging warn, and safe envs Fixes scout-off#272
- Install helmet@8.0.0 - Mount helmet() before custom securityHeaders middleware in app.ts - Custom middleware still overrides HSTS, X-Frame-Options, etc. per config - Expand security header tests to verify helmet-only headers: cross-origin-opener-policy, cross-origin-resource-policy, x-permitted-cross-domain-policies, x-dns-prefetch-control - Verify x-powered-by is removed by helmet Fixes scout-off#281
- transactionId is now null when tx_hash is absent from event payload
instead of fabricating a mock-tx-${i} string
- Add PaymentHistoryItem type to src/types/index.ts with nullable transactionId
- Import and use PaymentHistoryItem in scoutController.ts
- Extend payments tests: null transactionId, real tx_hash passthrough,
regex assertion that /^mock-tx-/ never appears in responses
Fixes scout-off#296
…trail - Broaden AuditEvent interface in audit.ts with index signature so auth middleware can pass ip, path, requiredRole, reason without type errors - requireAuth now calls logAuditEvent(action:'auth_failed') on both 401 paths: missing token and invalid/expired token; includes ip, path, reason fields - requireRole now calls logAuditEvent(action:'auth_failed') on 401 paths and logAuditEvent(action:'auth_forbidden') on 403 role-mismatch; includes ip, path, requiredRole, wallet (from token sub), providedRole, reason - Raw JWT is never included in audit events - Add getIp() helper that reads x-forwarded-for to support proxy deployments - Add tests: auth_failed on missing token, auth_failed on invalid token, auth_forbidden on role mismatch, no raw JWT in event payload Closes scout-off#278
- Create src/utils/subscription.ts with getActiveSubscription(wallet)
returning { active, tier: SubscriptionTier | null, expiresAt: number | null }
- Implements the same two-step fallback: on-chain isSubscribed() then
indexed scout_subscribed events
- getSubscription() controller now delegates to getActiveSubscription()
- scoutHasPlayerAccess() now delegates to getActiveSubscription()
- Add SubscriptionTier type to src/types/index.ts
- Add 12 unit tests in tests/utils/subscription.test.ts covering both
the on-chain path and the indexed-events fallback
Fixes scout-off#295
- Add revoked_tokens SQLite table (jti TEXT PRIMARY KEY, revoked_at INTEGER, expires_at INTEGER) - Add src/services/tokenBlocklist.ts with revokeToken(), isTokenRevoked(), pruneExpiredTokens() - Update requireAuth, requireRole, requireRoles in auth.ts to check blocklist on every request - Add POST /api/admin/tokens/revoke endpoint (admin only) to adminController + admin route - Prune expired tokens at startup and on each revoke call - Add db/004_token_revocation.sql migration file - Add tests/middleware/tokenRevocation.test.ts - Fix pre-existing bugs: stellar.ts duplicate stellarHealth, validatorController missing logger import, adminController missing EventRecord import, playerController missing express/util imports, index.ts missing responseTime import, config.ts missing required() helper + duplicate logLevel - Add __mocks__/node-fetch.js to handle ESM-only node-fetch in Jest CJS environment - Add node-fetch to jest moduleNameMapper Closes scout-off#274
- Add db/002_trial_offers.sql with trial_offers table definition (scout, player_id, details_uri, ledger, tx_hash, created_at) - Add src/utils/migrations.ts runMigrations() utility that reads numbered SQL files from db/ in order and records applied versions in schema_migrations, making runs idempotent - Extend __mocks__/better-sqlite3.js to support schema_migrations, sqlite_master and PRAGMA table_info queries for test coverage - Add tests/utils/migrations.test.ts with 6 tests verifying: schema_migrations table creation, all files applied, trial_offers table created, idempotency, and correct column schema Closes scout-off#289
…#297) - Add Express.Request augmentation in src/types/index.ts declaring req.account (string | undefined) and req.role (string | undefined), picked up by all handlers without any cast - Update src/middleware/auth.ts: replace all (req as any).account and (req as any).role assignments with direct typed property access - Add submitTrialOffer handler to src/controllers/scoutController.ts that reads req.account directly — no (req as any) cast anywhere in the file - Register POST /api/scouts/trial-offer in src/routes/scout.ts - Add tests for submitTrialOffer: 401 without token, 400 missing body, 201 with valid authenticated request tsc --noEmit reports no new errors in the changed files. No functional change to existing handlers. Closes scout-off#297
…il (scout-off#299) - Create src/utils/validators.ts with a single STELLAR_ADDRESS_RE constant (/^G[A-Z2-7]{55}$/) exportable by any controller or service - Update src/controllers/adminController.ts: - Import STELLAR_ADDRESS_RE from ../utils/validators (replaces the inline definition on the old line 1) - Remove (req as any).account casts in registerValidator and revokeValidator — uses req.account directly via Express.Request augmentation added in src/types/index.ts - Add AdminEvent and FeeHistoryItem interfaces to src/types/index.ts so adminController.ts compiles without TS errors Both registerValidator and revokeValidator now validate with the same imported constant. No change in validation behaviour. Closes scout-off#299
…cout-off#301) GET /api/players/:playerId/milestones previously returned 200 with an empty array for any player ID, making it impossible to distinguish between 'player exists but has no milestones' and 'player does not exist'. Changes: - Extract getPlayerById(playerId) helper in playerController.ts that queries player_registered events — returns the payload or undefined - getPlayer() now uses getPlayerById() instead of inline event filter - getPlayerMilestones() calls getPlayerById() at the start; returns { success: false, error: 'Player not found' } with status 404 if the player does not exist; existing players with no milestones still return 200 with data: [] - Remove duplicate 'import { sanitizeInput }' statement - Add missing imports (Request, Response, NextFunction, ProgressLevel, getTierMeta) that were absent in the original file - Add test: non-existent player ID returns 404 Closes scout-off#301
fix(ipfs): dedupe pinJson uploads via short-lived hash cache to prevent duplicate CIDs on retry
…ones-404-player-not-found fix(player): return 404 when player not found in milestones endpoint
…-duplicate-stellar-regex fix(admin): remove duplicate STELLAR_ADDRESS_RE, extract to shared util
…ocation feat(scout-off#274): add token revocation/blocklist mechanism
…-offers-migration feat(db): add migration 002 for trial_offers table
…failures feat(scout-off#278): log all failed authentication attempts to audit trail
…merge tsconfig.json already had "strict": true, but tsc --noEmit reported 5 errors — adminController.ts's revokeValidator handler referenced revokeValidatorOnChain() and ValidatorActionError from src/services/stellar.ts, neither of which existed. The merge commit for PR scout-off#621 (revoke_validator on-chain wiring) only carried its adminController.ts/test changes through; the stellar.ts half of that diff never landed, so the build has been broken on main since. Restore the missing export block (verbatim from the original commit eaef3df) and add revokeValidatorOnChain/ValidatorActionError back to adminController.ts's import from '../services/stellar'. tsc --noEmit now passes with zero errors and no @ts-ignore/as any were introduced.
withdrawFees() unconditionally threw FeeWithdrawalError('No fees
available', 'NO_FEES'), so POST /api/admin/fees always returned 409
regardless of the platform's actual on-chain fee balance.
Replace the stub with a real invocation, mirroring the
build/simulate/assemble/sign/submit/poll flow used by
pauseContractOnChain() / cancelSubscriptionOnChain(): submit
withdraw_fees(recipient), poll for the confirmed transaction, and parse
the u128 return value via scValToNative(). Only throw NO_FEES when the
parsed amount is exactly zero (or the transaction returns no value at
all) rather than unconditionally. Detect the contract's paused-state
guard (error scout-off#10) at both the simulation and FAILED-transaction stages
and map it to FeeWithdrawalError(..., 'CONTRACT_PAUSED'); map any
RPC/transport failure to 'NETWORK_ERROR'. The adminController.ts
controller already consumed FeeWithdrawalResult/FeeWithdrawalError
correctly (including the CONTRACT_PAUSED branch) — only the service
function needed wiring.
Add unit test coverage for the success path, u128 zero/missing-value
NO_FEES cases, CONTRACT_PAUSED detection, and NETWORK_ERROR handling.
queryMilestones() always returned [], so the milestones endpoint only ever surfaced indexed events, missing any on-chain milestone not yet picked up by the indexer. Replace the stub with a real view-only simulateTransaction call to get_milestones(player_id) — mirroring isSubscribed()'s ephemeral-keypair pattern, since this is a read-only query that never signs or submits a transaction. Add parseMilestonesFromNative() to turn the decoded Vec<Milestone> into OnChainMilestone[], tolerating both the contract's native snake_case field names and camelCase as a defensive fallback. Since the contract's Milestone struct doesn't carry its own id, one is synthesized from the entry's position in the returned vector. Add 'MISSING_PLAYER' to PaymentError's code union and throw it when the simulation reports the contract's PlayerNotFound (#3) error, following the same best-effort error-string-matching convention used elsewhere in this file (e.g. cancelSubscriptionOnChain's scout-off#8/scout-off#9 handling). An empty result or an unknown-but-valid player still returns [] rather than throwing. Add unit test coverage for a fixture Vec<Milestone> response (asserting both field-mapping and that the call never signs/submits), the empty-array cases, MISSING_PLAYER detection, and NETWORK_ERROR handling.
POST /api/admin/validators/register returned 202 with a success message but only wrote to the local validators table — no contract call was ever made, so validators were never actually registered on-chain. Add registerValidatorOnChain() to src/services/stellar.ts, mirroring the build/simulate/assemble/sign/submit/poll flow used by unpauseContractOnChain() and cancelSubscriptionOnChain(). Introduces a shared ValidatorActionError class/ValidatorActionErrorCode union covering both register (ALREADY_REGISTERED) and revoke (ALREADY_REVOKED, NOT_REGISTERED) admin actions, plus UNAUTHORIZED and NETWORK_ERROR, rather than forking a second near-identical error type. Wire it into adminController.ts's registerValidator handler: audit-log the attempt before calling the chain, and only call insertValidator() with the confirmed transaction hash after the chain call succeeds — a failed/rejected chain call no longer leaves a local row that doesn't reflect on-chain state. The existing invalid-address 400 check is unchanged. Chain failures are audit-logged and mapped to an HTTP status (409 ALREADY_REGISTERED, 403 UNAUTHORIZED, 503 NETWORK_ERROR) without touching the local row. Extend tests/services/stellar.test.ts with success/poll/failure/ error-code coverage for registerValidatorOnChain, and update tests/routes/admin.test.ts and tests/routes/contract.test.ts to mock the new chain call and cover the transactionId-in-response and chain-failure paths at the route level, mirroring the existing revokeValidatorOnChain test coverage.
…subscription-brace-regression Fix missing closing brace on purchaseSubscription in src/services/stellar.ts
…ster-validator-onchain Wire registerValidator admin action to Soroban contract
…y-milestones-onchain Wire queryMilestones() to Soroban get_milestones contract call
…draw-fees-onchain Wire withdrawFees() to Soroban withdraw_fees contract call
…t-strict-mode Enable TypeScript strict mode in tsconfig.json
…t-submit-trial-offer-250 fix(routes): import missing submitTrialOffer handler
updateProfile() returned a synthetic stub-update-txid-{playerId.slice(0,8)}
without any on-chain interaction, so player profile updates were silently
dropped after IPFS pinning — nothing was ever written to the register
contract.
Replace the stub with a real invocation of update_profile(player_id,
metadata_uri), mirroring the build/simulate/assemble/sign/submit/poll
flow used by logTrialOffer()/cancelSubscriptionOnChain(). Switch the
missing-argument validation from a plain Error to PaymentError(...,
'INVALID_ACCOUNT') for consistency with the rest of this file, and throw
PaymentError(..., 'MISSING_PLAYER') when the contract simulation reports
the register contract's PlayerNotFound (#3) error — reusing the same
error code and best-effort string-matching convention already
established by queryMilestones() for the same contract error. Any other
RPC/transport failure at any step (getAccount, simulate, send, poll)
maps to NETWORK_ERROR.
Add unit test coverage for the success path, polling, MISSING_PLAYER
detection (both the simulation and FAILED-transaction stages),
NETWORK_ERROR handling at each RPC step, and an integration-style test
that performs a mocked get_player read-back after a successful update to
verify the new metadataUri is what a subsequent on-chain read would
report.
…pdates (scout-off#478) - Add EventBroadcaster singleton (src/services/eventBroadcaster.ts): EventEmitter-based pub/sub bus with subscribe/unsubscribe/broadcast. isEventRelevantToWallet() enforces per-wallet filtering across all 7 ContractEventType values — no cross-tenant leakage. - Hook broadcaster into indexer (src/services/indexer.ts): After insertMany() commits to SQLite, broadcast each event to SSE subscribers so clients are never notified before the write is durable. - Add GET /api/events/stream route (src/routes/events.ts): Authenticated with requireAuth (Bearer JWT, any role). Sets SSE headers (Content-Type: text/event-stream, Cache-Control: no-cache, X-Accel-Buffering: no). Sends an immediate 'connected' event on open. Periodic ': ping' keep-alive every SSE_KEEPALIVE_INTERVAL_MS ms (default 15 s). Cleans up subscriber and interval on close/aborted. Optional SSE_MAX_CONNECTIONS limit (returns 503 when reached). - Mount events router in app.ts under /api/events and /api/v1/events. - Tests (50 new, all passing): tests/routes/sseStream.test.ts — 401 on missing/invalid token, SSE headers, connected event, event delivery by type, cross-tenant isolation, multi-client filtering, disconnect cleanup, /api/v1 alias. tests/services/eventBroadcaster.test.ts — relevance filter for all 7 event types, lifecycle, error resilience. Closes scout-off#478
…cout-off#467) Design and implement a database driver abstraction layer that allows Scout-Off to run against either SQLite (default, for single-instance) or PostgreSQL (opt-in, for horizontal scaling). ## Architectural Changes ### DbDriver Interface (src/db/driver.ts) New abstraction for all database operations: - all<T>(sql, params): T[] - Query returning multiple rows - get<T>(sql, params): T | undefined - Single row query - value<T>(sql, params): T | undefined - Scalar query - run(sql, params): { changes, lastId } - Mutations - exec(sql): void - Raw SQL execution - transaction<T>(fn): T - Transactional wrapper - close(): void - Connection cleanup ### Implementations - SqliteDriver: Uses better-sqlite3 (existing behavior) - PostgresDriver: Wraps pg library with sync adapter Maintains synchronous interface via busy-wait pattern ### Database Initialization (src/db/index.ts, src/index.ts) - initDb() is now async - awaited before server start - Selects driver based on config.dbDriver - PostgreSQL: async connect() then sync wrapper - SQLite: maintains existing inline schema creation ### Migrations (src/db/migrate.ts, db/*_postgres.sql) - Uses DbDriver abstraction - Supports per-driver migration files (*_postgres.sql) - 19 new PostgreSQL migration files covering full schema - Auto-converts SQLite→PostgreSQL: - AUTOINCREMENT → SERIAL - INSERT OR IGNORE → ON CONFLICT DO NOTHING - datetime('now') → now() ### Docker Compose (docker-compose.yml) Added optional PostgreSQL 15 service with health checks and persistent volume. ### Configuration (src/config.ts) Already had: - DB_DRIVER env var: 'sqlite' (default) or 'postgres' - DATABASE_URL env var: PostgreSQL connection string - dbPath: SQLite file path (default: scout-off.db) ## Backwards Compatibility ✅ SQLite is still the default ✅ No migration needed for existing deployments ✅ All tests pass without modification ✅ Existing behavior completely preserved ## Acceptance Criteria ✅ DB_DRIVER config switch works ✅ All migrations have PostgreSQL equivalents ✅ docker-compose.yml has optional Postgres service ✅ docs/postgres-migration.md documents cutover ✅ Tests verified with both drivers ## Testing To test with PostgreSQL locally: docker-compose up -d postgres export DB_DRIVER=postgres export DATABASE_URL="postgresql://scout_user:scout_password@postgres:5432/scout_off" npm start ## Known Limitations - PostgreSQL driver uses busy-wait pattern (temporary; full async refactor is future work) - No nested transaction support (future enhancement with savepoints) - Requires RETURNING clause for lastInsertRowid extraction ## Files Changed Modified: - src/db/index.ts - src/db/migrate.ts - src/index.ts - docker-compose.yml New: - src/db/driver.ts - src/db/sqlite-driver.ts - src/db/postgres-driver.ts - docs/postgres-migration.md - db/*_postgres.sql (19 files)
…e-abstraction-postgresql feat: add SQLite-to-PostgreSQL migration path for horizontal scaling …
CI only ran against .nvmrc's pinned Node 20, so a regression that only manifests on Node 18 or 22 would not be caught before merge. Convert the lint and test jobs to a fail-fast:false strategy.matrix over node-version: [18, 20, 22], passing matrix.node-version to actions/setup-node instead of node-version-file. Coverage upload/artifact steps stay gated to node-version==20 so behavior for that version is unchanged and coverage isn't uploaded 3x. Declare the supported range explicitly via engines.node in package.json. Verified against clean Node 18/20/22/24 installs: no version-specific regressions surfaced other than an EBADENGINE warning on Node 18 for @noble/hashes (a transitive @stellar/stellar-sdk dependency requiring Node >=20.19) — non-fatal, and the full stellar.ts test suite still passes on Node 18. Also fixes two pre-existing, non-version-specific CI breakages discovered while verifying the matrix (both already broken on Node 20 before this change, so unrelated to the matrix itself): - tests/services/stellarModuleWiring.test.ts's require() calls tripped @typescript-eslint/no-var-requires, failing `npm run lint`. Added the eslint-disable-next-line comment used by every other test file in this repo that intentionally require()s a module inside an assertion. - .env.example was missing WEBHOOK_SECRET (referenced in src/config.ts), failing `node scripts/validate-env.js`. Note: a separate, pre-existing tsc error (adminController.ts references an undefined revokeValidatorOnChain in src/services/stellar.ts) still blocks the Type check step on every Node version, same as it does on main today — that's an unrelated incomplete on-chain wiring gap, not introduced or fixed by this change.
There was no way to onboard many players at once (e.g. migrating an
academy's existing roster) — each player had to go through the normal
self-registration flow individually.
Add POST /api/admin/players/import, admin-only, accepting either a JSON
{ players: [...] } body or a CSV/text body (wallet,position,region,
metadataUri per row). Mirrors the existing /api/admin/validators/import
endpoint's shape: each entry is validated against the exact registerSchema
used by POST /api/players/register and processed independently through
the same sanitize -> pin (if raw metadata given) -> upsertPlayer ->
dispatchEventWebhook path, so one bad row doesn't abort the batch. Returns
a per-row { row, status: 'success'|'error', playerId?, error? } result
plus a { total, succeeded, failed } summary. Batch size is capped via the
new config.playerImport.maxBatchSize (env PLAYER_IMPORT_MAX_BATCH,
default 500).
Also fixes the same pre-existing lint break as the Node-matrix branch
(tests/services/stellarModuleWiring.test.ts's require() calls needed the
eslint-disable-next-line comment used elsewhere in this repo) so `npm run
lint` is green on this branch independent of merge order.
CONTRIBUTING.md told contributors to run npm audit manually before every PR, but nothing enforced it in CI, so a dependency with a known high or critical vulnerability could still be merged if that manual step was skipped. Add an `audit` job to .github/workflows/ci.yml running `npm audit --omit=dev --audit-level=high`, alongside the existing lint/test/contracts jobs. Dev-only tooling is excluded via --omit=dev since it never ships to production. Fix the findings this surfaced against the current dependency tree so the job starts green: - form-data 4.0.0 -> 4.0.6 (critical: unsafe boundary RNG / CRLF injection) - axios 1.6.8 -> 1.18.1 (high: SSRF/prototype-pollution/DoS advisories) - express 4.18.2 -> 4.22.2, staying on the 4.x line (high: transitive path-to-regexp/qs/send/body-parser ReDoS and DoS advisories) - @opentelemetry/auto-instrumentations-node 0.48.0 -> 0.78.0 and @opentelemetry/sdk-node / exporter-trace-otlp-http 0.52.1 -> 0.221.0 (high: Prometheus exporter crash + Jaeger propagator DoS). This pulled in @opentelemetry/core 2.x as a peer; the app only touches the SDK through NodeSDK/OTLPTraceExporter/getNodeAutoInstrumentations in src/tracing.ts, whose API is unchanged, and @opentelemetry/api stays on ^1.9.0 (still within sdk-node@0.221's >=1.3.0 <1.10.0 peer range). `npm audit --omit=dev --audit-level=high` now reports 0 vulnerabilities. Document the exception process in CONTRIBUTING.md for a future finding that can't be fixed immediately (check for a non-breaking fix or package.json `overrides` first; otherwise a tracked, time-boxed, maintainer-approved override) so the gate doesn't become an unblockable dead end.
…-ci-gate Add npm audit CI job gating high/critical vulnerabilities
…er-import Add bulk player import endpoint for admins
…ion-matrix Add Node 18/20/22 version matrix to CI
…f#697) scripts/deploy-staging.sh previously ran `npm ci --omit=dev` before `npm run build`, which excluded devDependencies (including typescript) before tsc was needed. This caused every staging deploy to fail at the build step. Fix: run `npm ci` (full install, devDependencies included) first, then `npm run build`, then `npm prune --omit=dev` to strip dev packages after the build artifact exists. Also add a "Dry-run deploy script ordering" step to the build job in .github/workflows/deploy-staging.yml that mirrors this exact sequence (npm ci → npm run build → npm prune --omit=dev) and verifies dist/index.js is produced, so any future ordering regression is caught in CI before reaching the staging server. Fixes scout-off#697
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
16 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scripts/deploy-staging.shrannpm ci --omit=devbeforenpm run build, which strippeddevDependencies(includingtypescript) beforetscwas needed. Every staging deploy following this script would fail at the build step.Root Cause
typescriptis listed underdevDependenciesinpackage.json. Passing--omit=devtonpm ciinstalls only production dependencies, sotscis not onPATHwhennpm run build(which callstsc) runs next.Fix
scripts/deploy-staging.sh— reorder the three steps:npm ci --omit=devnpm ci(full install, devDependencies included)npm run buildnpm run buildnpm prune --omit=dev(strip devDeps after artifact exists)The rationale is documented in a comment block at the top of the script.
.github/workflows/deploy-staging.yml— add a "Dry-run deploy script ordering" step to thebuildjob that executes the same sequence (npm ci→npm run build→npm prune --omit=dev) and assertsdist/index.jsexists afterward. This ensures any future ordering regression is caught in CI before the tarball reaches the staging server.Testing
The dry-run step in the workflow mirrors exactly what the deploy script does on the server and confirms the build artifact is produced. This satisfies the acceptance criterion: "A test or CI check exercises the actual script (or an equivalent dry run) against a fresh checkout and confirms the build step succeeds."
Issue
closes scout-off#697