Modernise stack#209
Draft
connorads wants to merge 34 commits into
Draft
Conversation
- checkout/setup-node v4, CodeQL v3, Coveralls v2, ubuntu-24.04, dynamodb-action v4 - deploy-dev runs on pull_request, deploy-prod on push to master (environment approval gates unchanged)
openapi-cli is deprecated. Pin 1.25.15 (last line supporting Node 14) with a resolution for @redocly/openapi-core; bump to v2 with the Node 22 upgrade. Migrate .redocly.yaml to the apis/rules schema.
- Assert JSON parse failures on status + "Unexpected token" substring; the exact message is V8-specific and changes across Node versions - jest.useFakeTimers() without "modern": it is the default since Jest 27 and the argument is removed in Jest 29+
http://localhost/3000 -> http://localhost:3000. Only affects the IS_OFFLINE branch used in local development.
- pnpm-lock.yaml via pnpm import (preserves yarn.lock resolutions) - nodeLinker: hoisted - Jest <28 duplicates module instances under pnpm's symlinked layout (@jest/fake-timers instanceof checks fail) and the serverless v2/webpack toolchain expects npm-style hoisting - allowBuilds: esbuild only; its postinstall downloads the platform binary. All other build scripts stay blocked - mise.toml pins node/pnpm/java; Node 16 interim (14 has no darwin-arm64 build), bumped with the runtime flip - CI installs via pnpm/action-setup (standalone while CI node is 14)
Serverless v2 is EOL and nodejs14.x Lambdas are blocked from updates. osls (MIT fork of Serverless v3) deploys in place to the same CloudFormation stack with unchanged logical IDs. - serverless -> npm:osls@^4 alias (satisfies serverless-offline@14's peer range), serverless-webpack/webpack -> serverless-esbuild (exclude: [] keeps aws-sdk v2 bundled since nodejs22.x does not provide it; .html text loader for the swagger handler) - serverless-dynamodb-local -> serverless-dynamodb (the old fork's DynamoDB Local download URL is dead) - runtime nodejs22.x; drop lambdaHashingVersion; iamRoleStatements -> provider.iam.role.statements - dev script defaults SLACK_*/STATE_SECRET to dummies: osls fails hard on unresolved env vars where v2 only warned; deploys still pass real secrets via CI env - Node 22 in mise.toml, engines and workflows Packaged template diff vs the old stack: the three DynamoDB tables and ApiGatewayRestApi keep identical logical IDs and properties; only Lambda Versions and the ApiGateway Deployment (hash-named) churn.
v2 reached end-of-support in September 2025. - Repos take DynamoDBDocumentClient; params/result shapes unchanged, .promise() calls become send(Command) - marshallOptions.removeUndefinedValues on every DocumentClient: v2 silently dropped undefined values (optional lock Metadata, optional fields in the Slack Installation blob); v3 throws without it - Offline/test clients pass dummy credentials: the v3 credential chain throws against DynamoDB Local when none are configured - @aws-sdk/* stay bundled by esbuild (exclude: []) per AWS guidance rather than relying on the runtime copy Toolchain minimums forced by the SDK's type definitions: - typescript 4.9.5 (type import modifiers; full 5.x move comes with the lint overhaul), @types/node 22, skipLibCheck (2021-era Express type packages cannot lib-check against modern @types/node) - @types/express pinned to 4.17.21 to dedupe Bolt's stale transitives - jest 27.5.1/ts-jest 27.1.5 (node:-prefixed requires used by @smithy) Contract tests run against both repo implementations and the supertest suite exercises them through serverless-offline; all 95 pass.
Same project, new home; @vendia is deprecated. Import swap only.
Bolt 3 is EOL; Bolt 4 requires Express 5. ExpressReceiver and the inline installation store keep working unchanged. - Route params get explicit types: Express 5 types params as string | string[] to cover repeatable segments, these routes only use single :param segments - @types/express 5 to match the runtime - Known behaviour change: Express 5 routes async rejections to the error middleware (500 response) instead of crashing the process Offline smoke: /slack/install renders the OAuth v2 authorize URL with signed state; API suite passes byte-identical response assertions.
Standard bcrypt format: existing stored $2a$ token hashes keep verifying; new hashes use the $2b$ prefix. v3 ships its own types so @types/bcryptjs goes.
- fileParallelism: false preserves serial file execution; api and lock-bot suites share DynamoDB Local tables and serverless-offline - jest.* timer calls -> vi.*; globals stay on (tsconfig types vitest/globals, eslint globals until the flat-config rewrite) - Coverage via @vitest/coverage-v8 with lcov output for Coveralls; include limited to .ts so the html/json handler assets are skipped - .gitignore: .webpack -> .esbuild (serverless-esbuild build dir)
target/lib es2023, moduleResolution bundler (esbuild and vitest do the building), verbatimModuleSyntax, isolatedModules, noUncheckedIndexedAccess, noUnusedLocals/Parameters. Fallout fixes: - command parsers accept Slack's optional command text and always return a string (split()[0] can be undefined under noUncheckedIndexedAccess) - in-memory repo key parsing uses destructuring defaults - type-only imports throughout; unused Express params -> _req/_next lint:eslint is removed this commit: @typescript-eslint 4 cannot parse lib es2023. The Biome + ESLint 9 replacement lands next.
Biome is the formatter and main linter (recommended preset). ESLint survives only for the type-aware rules Biome cannot implement: switch-exhaustiveness-check (load-bearing on the Destination switch) and no-floating-promises, via typescript-eslint 8 flat config. The plan named the Ultracite preset, but Ultracite 6+ pivoted from Biome to oxlint; plain Biome recommended is used instead. Lint fixes surfaced by the new rules: - forEach callbacks no longer return values (useIterableCallbackReturn) - auth(req)! -> auth(req)?. (authorizer middleware already rejects unauthenticated requests) - catch binds no unused error var; crypto -> node:crypto - the token/trigger_id/response_url destructure keeps its explanatory comment instead of a stale eslint-disable biome format lands as the next (style-only) commit; lint:biome goes green there.
biome check --write: formatting, import sorting and import type modifiers. Style-only; no behaviour change.
The interim 1.25 pin existed only for Node 14 support. Drop the openapi-core resolution; rename .redocly.yaml to redocly.yaml (v2 config discovery).
biome (staged files, with autofix), eslint, tsc and redocly. Install with `hk install`; hk and pkl are pinned in mise.toml.
Stack, architecture, commands, test topology (:8000/:3000, serial) and deploy gates - including the CloudFormation logical-ID stability rule for the stateful tables. CLAUDE.md symlinks to it.
Grouped non-major updates, weekly aws-sdk batch, serverless toolchain grouped with the osls alias, 4-day minimumReleaseAge matching the package-manager quarantine.
- minimumReleaseAge: 5760 (4-day quarantine) applies on any machine and in CI, not only where a global config sets it - Drop nodeLinker: hoisted - it existed for Jest <28 and the serverless v2/webpack toolchain, both gone. Full gate (lint, suite, sls package) passes on the default isolated layout
Express 5 forwards async handler rejections only to error middleware registered after the route, so handleErrors registered before the routes never saw them and unhandled errors fell through to the default HTML handler. Split the Express app into app.ts (exported for supertest) with index.ts as the serverless-express shim, and register handleErrors after the routes.
esbuild already emits source maps but Node ignores them without --enable-source-maps, so Lambda stack traces pointed at bundled JS. Verified via sls package diff: only the three function env blocks change; table and ApiGatewayRestApi resources are byte-identical.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| app.get<ChannelParams>( | ||
| "/api/teams/:team/channels/:channel/locks", | ||
| authorizer, |
|
|
||
| app.get<LockParams>( | ||
| "/api/teams/:team/channels/:channel/locks/:lock", | ||
| authorizer, |
|
|
||
| app.post<ChannelParams>( | ||
| "/api/teams/:team/channels/:channel/locks", | ||
| authorizer, |
Comment on lines
+58
to
+82
| async (req, res) => { | ||
| // authorizer has already rejected requests without valid basic auth | ||
| const username = auth(req)?.name; | ||
| const { channel, team } = req.params; | ||
| const lock = req.body as Lock; | ||
| if (lock.owner !== username) { | ||
| const error = `${username} cannot lock for another user ${lock.owner}`; | ||
| console.log("Cannot lock", { username, body: req.body, error }); | ||
| res.status(403).json({ message: error }); | ||
| } else { | ||
| const lockOwner = await lockRepo.getOwner(lock.name, channel, team); | ||
| if (!lockOwner) { | ||
| await lockRepo.setOwner(lock.name, lock.owner, channel, team); | ||
| console.log("Added lock", { lock }); | ||
| res.status(201).json(lock); | ||
| } else if (lockOwner === lock.owner) { | ||
| console.log("Lock exists", { lock }); | ||
| res.status(200).json(lock); | ||
| } else { | ||
| const error = `${lock.name} is already locked by ${lockOwner}`; | ||
| console.log("Cannot lock", { lock, lockOwner, error }); | ||
| res.status(403).json({ message: error }); | ||
| } | ||
| } | ||
| }, |
|
|
||
| app.delete<LockParams>( | ||
| "/api/teams/:team/channels/:channel/locks/:lock", | ||
| authorizer, |
Comment on lines
+89
to
+106
| async (req, res) => { | ||
| const { team, channel, lock: lockName } = req.params; | ||
| const lockOwner = await lockRepo.getOwner(lockName, channel, team); | ||
| if (!lockOwner) { | ||
| console.log("No lock to delete", { lockName }); | ||
| res.status(204).end(); | ||
| } else if (lockOwner === auth(req)?.name) { | ||
| const lock = { name: lockName, owner: lockOwner } as Lock; | ||
| await lockRepo.delete(lockName, channel, team); | ||
| console.log("Lock deleted", { lock }); | ||
| res.status(204).end(); | ||
| } else { | ||
| const lock = { name: lockName, owner: lockOwner } as Lock; | ||
| const error = `Cannot unlock ${lock.name}, locked by ${lockOwner}`; | ||
| console.log("Cannot unlock", { lock, error }); | ||
| res.status(403).json({ message: error }); | ||
| } | ||
| }, |
The AWS SDK v3 default retry strategy can hang a request indefinitely against DynamoDB Local - a stalled retry is bounded by no request timeout - so the contract tests' beforeEach table recreation hung until vitest's 10s hookTimeout, failing all 27 dynamodb-repo tests in CI. maxAttempts: 1 makes local calls fail fast. Only affects the local/offline client; real DynamoDB keeps default retries via its own client.
|
coverage: 95.604% (-3.8%) from 99.401% — modernise-stack into master |
Plugin is unmaintained (last publish 2023-03) and incompatible with osls v4: every version, incl. latest 2.0.3, calls the removed provider.request() API, so its post-deploy updateStage hook throws "provider.request() is no longer available" and fails the deploy. osls v4 has no native stage-level default throttling (provider.apiGateway schema is additionalProperties:false with no throttle keys), and osls emits no AWS::ApiGateway::Stage resource to attach MethodSettings to, so there is no clean in-template replacement. Existing stages keep their already-applied 200/100 throttle (sticky stage setting); account-level API Gateway limits still apply. sls package diff vs before: only the plugin's RestApiIdForApigThrottling output is removed. Tables, IamRoleLambdaExecution and ApiGatewayRestApi are byte-identical (no REPLACE/DELETE).
Sonar flags third-party actions pinned to tags (githubactions:S7637, MAJOR x4) as a supply-chain risk, driving a C security rating. Pin every third-party `uses:` to a full commit SHA with a `# vX` comment; also pin the trusted actions/* and github/* refs for consistency. Local reusable workflow refs are not SHA-pinnable and stay as-is. Add helpers:pinGitHubActionDigests so Renovate self-maintains the pins.
Express leaks its stack via X-Powered-By (typescript:S5689, MINOR), driving a B security rating. Disable the header on both Express apps (api and swagger). Also sweep the one-line smells Sonar flagged: trimRight -> trimEnd, mark private static key helpers readonly, and drop a redundant Promise.resolve in an already-async fetchInstallation.
vitest include: ["src/**/*.ts"] counted serverless glue and the HTTP-integration-tested API app at their in-process v8 coverage, dropping Coveralls from 99.4% to 48.5%. Exclude files v8 cannot measure honestly: the serverless-express index shims, the manually-smoke-tested OAuth store, and app.ts/middleware.ts (exercised over real HTTP by api.test.ts against serverless-offline, a separate process). Restores lines to 99.25%. Add a unit test for handle-command's default export - fully injectable (type-only imports), so no infra needed. Covers ack, destination -> response_type mapping, secret stripping from logs, and the error branch.
|
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
3 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.
No description provided.