Feat/esigner #650
Merged
Feat/esigner #650
Conversation
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
Contributor
|
Warning Rate limit exceeded@coodos has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 3 minutes and 48 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a new eSigner platform: a TypeORM + Express backend (entities, migrations, controllers, services, SSE auth/signing flows, webhook sync, web3-adapter subscriber) and a SvelteKit frontend (auth, files, signing UI, stores, components) plus platform eVault provisioning and mappings. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser
participant API
participant DB
participant Wallet
participant SSE as SSE Stream
Note over Browser,API: Offer & QR login flow
Browser->>API: GET /api/auth/offer
API->>DB: create session id
API-->>Browser: sessionId + qr/deeplink
User->>Wallet: scan QR / open deeplink
Wallet->>Wallet: sign payload (userId_md5)
Wallet-->>Browser: redirect with ename, session, signature, appVersion
Browser->>API: POST /api/auth/login
API->>DB: verify signature, find/create user
API-->>Browser: user + JWT
API->>SSE: emit login event for sessionId
SSE-->>Browser: login event received
sequenceDiagram
participant Signer
participant Browser
participant API
participant DB
participant Notif
participant ChatDB as Chat DB
Signer->>Browser: click "Sign"
Browser->>API: POST /api/signatures/session
API->>DB: validate invitation, create session
API-->>Browser: sessionId + qrData
Signer->>Wallet: scan QR and sign
Wallet->>API: POST /api/signatures/payload
API->>DB: verify signature, persist SignatureContainer, update FileSignee
alt all signees signed
API->>Notif: sendFullySignedNotification
Notif->>ChatDB: create system message
end
API-->>Browser: emit SSE event status=completed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks❌ Failed checks (2 warnings, 1 inconclusive)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Fix all issues with AI Agents 🤖
In @platforms/esigner-api/package.json:
- Line 9: The "build" script uses the Unix-only "cp -r" which will fail on
Windows; update the "build" script in package.json (the "build" key) to use a
cross-platform approach—either invoke a Node-based copy step (e.g., use a small
Node script or npm-run-all) or use a CLI package like "cpy-cli" and replace "cp
-r src/web3adapter/mappings dist/web3adapter/" with the cpy-cli equivalent, and
add "cpy-cli" to devDependencies (e.g., "cpy-cli": "^5.0.0") so the build works
on all platforms.
- Around line 15-30: Update the vulnerable dependencies in package.json: bump
"axios" to at least 1.8.2 (prefer 1.12.0+), "express" to ≥4.19.0 (or 5.2.1),
"multer" to ≥2.0.2, and "jsonwebtoken" to 9.0.3; after editing the
"dependencies" block (the package names are unique identifiers here) run your
package manager to update the lockfile (npm/yarn/pnpm install), re-run
dependency scanning/audit, and run the test suite to ensure no regressions.
In @platforms/esigner-api/src/controllers/AuthController.ts:
- Around line 108-112: Replace the generic Error thrown when user lookup fails
so the controller returns an HTTP 401/404 instead of a 500: in AuthController
(the method using this.userService.findByEname), when user is falsy use an
HTTP-specific error or set the response status to 401 (Unauthorized) or 404 (Not
Found) with a clear message rather than throw new Error("User not found");
ensure you use the project's existing HTTP error helper/exception class (or
res.status/.json flow) so the framework returns the correct status code.
In @platforms/esigner-api/src/controllers/FileController.ts:
- Around line 154-156: The Content-Disposition header in FileController is
vulnerable to header injection via file.name; sanitize the filename before using
it by stripping CR/LF characters and quotes and replacing or percent-encoding
other unsafe characters, then use the sanitized value in the header (or set an
RFC5987 filename* with encodeURIComponent for non-ASCII names) so
Content-Disposition uses a safe filename; ensure this change is applied where
res.setHeader('Content-Disposition', `attachment; filename="${file.name}"`) is
set.
In @platforms/esigner-api/src/controllers/SignatureController.ts:
- Around line 47-53: The SSE endpoint currently sets
Access-Control-Allow-Origin: '*' which is too permissive for the signing
workflow; update the res.writeHead usage in SignatureController (the code path
that writes the SSE headers via res.writeHead) to restrict allowed origins by
checking req.headers.origin against a configured whitelist (e.g., an environment
variable or TRUSTED_ORIGINS list) and only set Access-Control-Allow-Origin to
the allowed origin when it matches, otherwise reject the request or return a
403; ensure the header logic also handles missing origins and includes the same
whitelist mechanism for Access-Control-Allow-Headers if needed.
- Around line 55-58: The initial getSessionStatus check sets up SSE polling even
when session is missing; change the handler so after calling
this.signatureService.getSessionStatus(id) you return early if session is
null/undefined (e.g., send a 404 or an SSE "not found" event and end the
response) and do not proceed to set up the polling interval (the
setInterval/interval logic) or any further writes; ensure any created interval
is not started when session is absent.
In @platforms/esigner-api/src/controllers/WebhookController.ts:
- Around line 105-106: The inline fragile parsing in WebhookController.ts that
maps adminIds using a.split("(")[1].split(")")[0] should be replaced with a
small shared utility function (e.g., parseAdminIdFromLabel or extractAdminId) to
centralize logic and handle edge cases; implement the utility to safely check
for parentheses, trim results, and return the original string or null/undefined
on parse failure, then update the mapping of adminIds to call this function and
filter out or log invalid results accordingly.
- Around line 174-182: Replace the repeated fragile parsing in WebhookController
by creating and using a small safe extractor (e.g., extractIdFromParentheses)
and call it for local.data.sender and local.data.group before calling
this.userService.getUserById and this.groupService.getGroupById; the extractor
should accept a string, return the inner id or null/undefined if the pattern
doesn't match (no thrown errors), and WebhookController should guard the service
calls with that result so malformed strings are ignored rather than causing
runtime exceptions.
- Around line 91-94: The inline parser in the anonymous async handler of
WebhookController that uses ref.split("(")[1].split(")")[0] is fragile; replace
it with a validated extraction (e.g. run a regex like /\(([^)]+)\)/ against ref)
and only call this.userService.getUserById(userId) when a userId is actually
captured; if the format is invalid, handle the case explicitly (return null or
throw a controlled error and/or log a warning) to avoid runtime exceptions.
In @platforms/esigner-api/src/database/data-source.ts:
- Line 22: The migration glob in the data-source configuration currently uses
path.join(__dirname, "migrations", "*.ts") which fails in production because
compiled migrations are .js; update the migrations entry to include both
extensions (e.g., use "*. {ts,js}" pattern) so it works with ts-node in
development and compiled JS in production, leaving the rest of the DataSource
config unchanged and ensuring the change is applied where migrations: [...] is
defined.
In @platforms/esigner-api/src/database/entities/File.ts:
- Around line 32-33: The size property is declared as number but maps to a
PostgreSQL bigint which TypeORM returns as a string; change the
@Column("bigint") property to use a string type or add a transformer. Either
update the property signature to size!: string; or keep size!: number and add a
Column transformer (e.g., BigIntStringTransformer with to(value) =>
value?.toString() and from(dbValue) => dbValue === null ? null : Number(dbValue)
or BigInt(dbValue) as appropriate) and attach it to the @Column decorator for
the size field.
In @platforms/esigner-api/src/database/entities/FileSignee.ts:
- Around line 56-57: The relationship cardinality is inconsistent:
FileSignee.signature is declared as OneToOne while SignatureContainer.fileSignee
is declared as ManyToOne; make them symmetric by choosing the correct
multiplicity and updating the opposing decorator accordingly — either change
SignatureContainer.fileSignee from ManyToOne to OneToOne if a FileSignee has
only one SignatureContainer, or change FileSignee.signature from OneToOne to
OneToMany if a FileSignee can have multiple SignatureContainer entries; update
the TypeORM decorators and inverse side references (FileSignee.signature and
SignatureContainer.fileSignee) so both sides use the matching decorator and
inverse property name.
- Around line 38-42: The nullable Date columns signedAt and declinedAt in the
FileSignee entity need explicit DB types; update their decorators to specify a
timestamp type (e.g., use type: 'timestamp' or type: 'timestamptz' for
timezone-aware values) along with nullable: true so schema generation is correct
for the signedAt and declinedAt fields in the FileSignee class.
In @platforms/esigner-api/src/database/entities/Group.ts:
- Around line 14-15: The Group entity uses the default table name which is the
reserved word "group"; update the @Entity decorator on the Group class to
specify an explicit safe table name (for example @Entity('groups') or
@Entity('group_entity')) so the ORM creates a non-reserved table name; after
changing the decorator, search for any raw SQL or migration code that referenced
the old "group" table name and update those references and regenerate/run
migrations as needed to keep schema in sync.
In @platforms/esigner-api/src/database/entities/SignatureContainer.ts:
- Around line 48-50: SignatureContainer currently defines fileSignee with
@ManyToOne but FileSignee defines the inverse as @OneToOne; change the relation
on SignatureContainer to @OneToOne to match the inverse. Replace the @ManyToOne
decorator on the fileSignee property with @OneToOne(() => FileSignee,
(fileSignee) => fileSignee.signature, { nullable: true }) and keep the existing
@JoinColumn({ name: "fileSigneeId" }); also add the OneToOne import from TypeORM
to the imports list.
In @platforms/esigner-api/src/database/entities/UserEVaultMapping.ts:
- Around line 23-24: The UserEVaultMapping entity's userProfileId property is
declared nullable in the Column decorator but uses definite assignment syntax
(userProfileId!: string), which misrepresents its optionality; change the
declaration to an optional property (userProfileId?: string or userProfileId:
string | null) so the TypeScript type matches Column({ nullable: true }). Update
the UserEVaultMapping class (and analogous entities in eCurrency-api and
dreamsync-api) to use the optional/nullable type and adjust any code that
assumes it is always defined.
In @platforms/esigner-api/src/index.ts:
- Around line 23-48: The server is starting before the DB init finishes; move
the app.listen(...) call into the AppDataSource.initialize() successful branch
so the HTTP server only starts after AppDataSource.initialize() resolves (inside
the .then async block where PlatformEVaultService is used), and ensure the
.catch still exits on initialization error; update any references to app.listen,
AppDataSource.initialize, and PlatformEVaultService.getInstance() accordingly so
the server startup depends on successful DB initialization.
- Around line 51-63: The CORS setup uses origin: "*" together with credentials:
true which browsers reject; update the cors configuration passed to app.use(...)
so origin is not a wildcard when credentials are enabled — read an explicit
allowlist (e.g., ALLOWED_ORIGINS or an array variable) and implement origin as
either an array of allowed origins or a dynamic function that checks
req.header('Origin') against that allowlist and returns the origin when allowed
(otherwise block), and keep credentials: true only when a specific origin is
returned; adjust the cors call where cors({...}) is invoked to use this
allowlist/validator instead of origin: "*".
In @platforms/esigner-api/src/services/FileService.ts:
- Around line 82-100: In getUserFiles, the logic that decides whether to push
invited files is wrong because invitedFileIds is derived from invitedFiles
(always true) and the OR condition causes duplicates; replace it by computing a
set of owned IDs (e.g., ownedFileIds from ownedFiles.map(f => f.id)) and then
iterate invitedFiles and push fileSignee.file only if fileSignee.file exists and
its fileId is not in ownedFileIds (use fileSignee.fileId), ensuring invited
files already owned are not added and duplicates are avoided.
In @platforms/esigner-api/src/services/GroupService.ts:
- Around line 58-79: The fallback in GroupService that uses
groupRepository.createQueryBuilder(...) to load all private groups into memory
and then filter by sortedMemberIds causes unbounded memory use; replace it with
a DB-side query that matches groups with the exact set of member IDs (e.g., join
group.members, group by group.id, HAVING COUNT(*) = memberCount AND
array_agg(member.id) contains exactly the sortedMemberIds) so filtering happens
in SQL, or if DB-side set-equality is not available implement paginated fetches
(use take/skip or a cursor) over the createQueryBuilder result and compare each
page against sortedMemberIds to avoid loading all groups at once. Ensure the
replacement still returns the same group type or null and continues to use
sortedMemberIds for the comparison.
In @platforms/esigner-api/src/services/MessageService.ts:
- Around line 91-104: The updateMessage method currently accepts a
Partial<Message> and uses Object.assign on currentMessage, which allows callers
to overwrite protected fields (e.g., id, createdAt, sender, group); restrict
updates by whitelisting allowed fields (e.g., content, subject, status) before
applying changes: create a safeUpdate object by picking only allowed keys from
messageData (or validate/omit protected keys), assign those to currentMessage,
and then call this.messageRepository.save(currentMessage); reference
updateMessage, currentMessage, messageData, and messageRepository.save when
making the change.
In @platforms/esigner-api/src/services/PlatformEVaultService.ts:
- Around line 88-90: The axios GET for registry entropy in PlatformEVaultService
(the call that destructures data: { token: registryEntropy } from axios.get(new
URL("/entropy", registryUrl).toString())) lacks a request timeout; add an axios
timeout option (e.g., pass { timeout: <ms> } as the second argument to
axios.get) so the request fails fast on network hangs and surface a clear error,
and ensure the surrounding code handles the timeout error appropriately
(catch/rethrow or log) to avoid unhandled promise rejections.
- Around line 99-107: The axios.post call that sends provisioning data (using
provisionerUrl, registryEntropy, verificationId and namespace uuidv4()) lacks a
timeout; fix it by passing an axios request config as the third argument to
axios.post with a sensible timeout (e.g. { timeout: 5000 }) so the request won’t
hang indefinitely, and ensure existing error handling covers timeout errors (or
add try/catch where the call is made if not present).
- Around line 95-97: The code uses a hardcoded fallback UUID for verificationId
(process.env.DEMO_VERIFICATION_CODE || "d66b7138-..."), which is a security
risk; update the PlatformEVaultService initialization to NOT fall back to a
hardcoded value: require DEMO_VERIFICATION_CODE to be present (or add an
explicit demo-only config flag) and throw or log a clear error in the
constructor/init when process.env.DEMO_VERIFICATION_CODE is missing, removing
the literal UUID; reference the verificationId variable and the
DEMO_VERIFICATION_CODE env var so callers fail fast instead of using the
insecure default.
In @platforms/esigner-api/src/services/SignatureService.ts:
- Around line 92-102: The sessions Map is never cleared — the setTimeout only
marks a session.status as "expired" but leaves it in this.sessions causing
memory leaks; update the cleanup to remove the session entry
(this.sessions.delete(sessionId)) when it expires and/or add a periodic sweeper
(e.g., a cleanup method on SignatureService that iterates this.sessions and
deletes entries older than the TTL) so expired or stale sessions are actually
removed; locate the setTimeout block surrounding sessionId/session and change it
to delete the entry after marking expired (and consider adding a recurring timer
that calls the sweeper to handle any missed or long-lived entries).
- Line 35: The in-memory sessions Map on SignatureService (private sessions:
Map<string, SigningSession>) loses state across restarts and between instances,
breaking getSigningSessionStatus SSE polling in load‑balanced setups; replace
the Map with a shared store (e.g., Redis) by introducing a Redis-backed session
repository used by the SigningService methods that create, fetch, update and
delete SigningSession objects, persist sessions with the same 15-minute TTL
using EXPIRE/TTL semantics, update the code paths that reference sessions
(session creation, lookup in getSigningSessionStatus, and cleanup) to use the
repository API (get/set/del) and ensure serialization/deserialization of
SigningSession, and add retry/error handling for Redis failures.
- Line 1: The import of the Node crypto module at the top of SignatureService.ts
is unused; remove the line "import crypto from \"crypto\";" from the file
(SignatureService.ts) so the linter no longer flags an unused import and to keep
SignatureService free of dead dependencies.
- Around line 297-309: The block that creates and saves the signature (inside
SignatureService) is over-indented, indicating a missing or misplaced brace
around the surrounding control structure; inspect the try/catch and any
enclosing if/for/async blocks around the code that calls
this.signatureRepository.create(...) and this.signatureRepository.save(...),
close or move the misplaced brace so the try block and its corresponding
catch/finally align with the surrounding scope, and ensure signatureContainer,
session, invitation, and payloadToVerify remain in scope and the save call is
inside the intended try block.
In @platforms/esigner-api/src/services/UserService.ts:
- Around line 19-32: findOrCreateUser currently queries by the raw ename which
can differ from findByEname's normalization (e.g., leading '@'), risking
duplicates; normalize the ename at the start of findOrCreateUser the same way
findByEname does (strip any leading '@' and apply the same casing rules) before
calling this.userRepository.findOne and before passing to createBlankUser, so
lookups and creations use the canonical ename; keep token creation unchanged and
return the created/found user and token.
In @platforms/esigner-api/src/utils/jwt.ts:
- Line 3: The code currently defines JWT_SECRET with a hardcoded fallback
('your-secret-key') which is a security risk; replace that pattern by removing
the default and enforcing presence of the environment secret: change the
JWT_SECRET initialization (the const JWT_SECRET) to read process.env.JWT_SECRET
and if it's undefined or empty throw an Error (or exit during startup) with a
clear message indicating JWT_SECRET is required, so the app fails fast instead
of using a weak default; ensure any modules importing JWT_SECRET will receive
the validated value.
In @platforms/esigner-api/src/utils/version.ts:
- Around line 7-20: The compareVersions function lacks input validation; update
compareVersions to first verify both inputs are non-null, non-empty strings,
then split on '.', trim segments and parse each with parseInt (base 10), and
validate every segment is a finite integer >= 0 (reject NaN or negative values);
if validation fails throw a clear Error (do not silently treat invalid segments
as 0), otherwise proceed with the existing numeric comparison logic using the
validated integer arrays (keep the function name compareVersions and same return
semantics: -1, 0, 1).
In @platforms/esigner-api/src/web3adapter/watchers/subscriber.ts:
- Around line 15-20: The code constructs adapter = new Web3Adapter(...) using
direct casts like process.env.ESIGNER_MAPPING_DB_PATH as string which can hide
missing env vars; validate each required env var (ESIGNER_MAPPING_DB_PATH,
PUBLIC_REGISTRY_URL, PUBLIC_ESIGNER_BASE_URL and any others used for
schemasPath/dbPath) before creating the Web3Adapter, e.g. check for
undefined/empty and either throw a clear error or exit early with a descriptive
message, and only pass real string values into the Web3Adapter constructor (or
provide safe defaults) instead of using "as string". Ensure you reference the
adapter/Web3Adapter instantiation and the specific options schemasPath, dbPath,
registryUrl, and platform when adding the checks.
- Around line 204-232: The current try-catch only wraps setTimeout registration
so async errors from the callback (which awaits
this.adapter.mappingDb.getGlobalId and this.adapter.handleChange) become
unhandled; move the error handling inside the setTimeout async callback: wrap
the callback body in its own try-catch-finally that awaits getGlobalId and
handleChange, logs any caught error (including tableName and changeKey context),
and still calls this.pendingChanges.delete(changeKey) in the finally; keep the
early returns for this.adapter.lockedIds checks and ensure the outer try-catch
only guards scheduling if you keep it.
In @platforms/esigner/package.json:
- Line 19: package.json currently lists both "@sveltejs/adapter-static" in
devDependencies and "@sveltejs/adapter-node" in dependencies, which conflicts;
remove the duplicate adapter so only one adapter is present by deleting the
"@sveltejs/adapter-static" entry from devDependencies (or alternatively move
"@sveltejs/adapter-node" into devDependencies and replace it with
"@sveltejs/adapter-static" in dependencies if you intend static generation);
ensure only one of the adapter package names ("@sveltejs/adapter-static" or
"@sveltejs/adapter-node") remains and that adapters live in the correct
dependencies section based on your chosen deployment strategy.
- Line 40: Update the axios dependency in package.json from "axios": "^1.6.7" to
a patched minimum of "axios": "^1.8.2" (preferably "axios": "^1.13.2"), then run
your package manager (npm install or yarn install) to update node_modules and
regenerate the lockfile, commit the updated package.json and lockfile, and run
test suite and a vuln scan (npm audit or Snyk) to verify the CVEs are resolved.
In @platforms/esigner/src/app.css:
- Line 1: Replace deprecated Tailwind v3 opacity utilities in the Svelte page:
in platforms/esigner/src/routes/(protected)/files/[id]/+page.svelte update both
occurrences of the class using bg-opacity-50 to the new slash opacity syntax
(e.g., replace bg-opacity-50 with bg-black/50 where the background color is
black) so the components at the noted locations (around the classes referenced
at lines ~460 and ~507) use the v4-compatible bg-color/opacity form.
In @platforms/esigner/src/app.d.ts:
- Around line 1-10: The file ends with extra blank lines after the final export
{}; statement; remove the trailing empty lines so the file terminates
immediately after export {}; with a single newline. Edit app.d.ts to delete the
two blank lines following export {} so the file ends cleanly with that statement
plus one newline.
In @platforms/esigner/src/lib/components/UserMenuDropdown.svelte:
- Around line 43-48: The backdrop div used to capture outside clicks (the
element with onclick={closeDropdown}) incorrectly has role="button" and
tabindex="-1"; remove both attributes from that div so it is not exposed as a
focusable/button role to assistive tech and remains a non-interactive
click-catcher.
In @platforms/esigner/src/lib/stores/auth.ts:
- Around line 10-13: The code reads localStorage directly (const token =
localStorage.getItem('esigner_auth_token')) which will throw during SSR; guard
this so it only runs in the browser (e.g., check typeof window !== 'undefined'
or import SvelteKit's browser flag) and then set
apiClient.defaults.headers.common['Authorization'] = `Bearer ${token}` only when
the token exists client-side; update the initialization in the auth store (the
token variable and the apiClient header assignment) to run inside that browser
check so server-side execution never touches localStorage.
In @platforms/esigner/src/lib/stores/files.ts:
- Around line 80-84: The multipart upload call in files.ts is explicitly setting
'Content-Type': 'multipart/form-data' on the apiClient.post('/api/files',
formData, ...) which overrides the browser-set boundary and breaks parsing;
remove the explicit Content-Type header (either delete the headers option or
remove the 'Content-Type' property) so the browser can set the correct
Content-Type/boundary when sending the FormData.
In @platforms/esigner/src/lib/stores/signatures.ts:
- Around line 51-53: The catch block that sets the error store currently uses
err instanceof Error which misses Axios response messages; update the handler
around the error variable 'err' used in the signing session creation to detect
Axios errors (e.g., axios.isAxiosError(err)) and extract a more specific message
such as err.response?.data?.message || err.message, then call error.set(...)
with that extracted message before rethrowing err so the store contains the
Axios-provided error text.
- Around line 37-39: The catch block in signatures.ts currently uses err.message
which may miss server messages from Axios; update the catch handler (the catch
for the fetch signatures flow where error.set(...) and throw err occur) to
detect Axios-style errors and prefer err.response?.data?.message (or
err.response?.data?.error) before falling back to err.message or a default like
'Failed to fetch signatures', then call error.set(...) with that resolved string
and rethrow the original err.
In @platforms/esigner/src/lib/utils/axios.ts:
- Around line 13-16: The functions setAuthToken, removeAuthToken, and
removeAuthId access localStorage directly which breaks during SSR; update each
to guard localStorage access with a browser check (e.g., typeof window !==
'undefined' or SvelteKit's browser flag) before calling
localStorage.setItem/removeItem, and only set
apiClient.defaults.headers.common['Authorization'] when running in the browser
(or handle header mutation irrespective of storage but guard storage access) so
these functions no longer throw during server-side rendering.
In @platforms/esigner/src/routes/(auth)/auth/+page.svelte:
- Around line 93-129: The EventSource created in watchEventStream is never
closed on component unmount; modify watchEventStream to return the created
EventSource (or store it in an outer-scope variable) and register a Svelte
onDestroy handler that calls eventSource.close() to clean up the connection when
the component is destroyed; reference the watchEventStream function and the
EventSource instance to ensure you close the same object created for the
session.
- Line 6: The onDestroy import is unused; add cleanup to close the EventSource
created in your onMount handler instead of removing the import: keep the
existing onMount that assigns the EventSource to a scoped variable (e.g.,
eventSource) and add onDestroy(() => { if (eventSource) eventSource.close(); })
so the SSE connection is closed on component teardown; ensure the eventSource
variable is in a scope accessible to both onMount and onDestroy.
- Around line 102-112: Wrap the JSON.parse call inside the eventSource.onmessage
handler with a try/catch to handle malformed SSE payloads: catch parsing errors,
set a user-friendly errorMessage (and/or log the error), close eventSource, and
return early so the rest of the handler (the version_mismatch check) does not
run on invalid data; update the eventSource.onmessage block where
JSON.parse(e.data as string) is used and ensure any references to data,
errorMessage and eventSource are handled inside the catch.
In @platforms/esigner/src/routes/(protected)/files/[id]/+page.svelte:
- Around line 22-42: The subscriptions created inside onMount
(isAuthenticated.subscribe, signatures.subscribe, currentUser.subscribe) are
never unsubscribed; capture each subscribe's unsubscribe function (e.g., const
unsubAuth = isAuthenticated.subscribe(...), const unsubSigs =
signatures.subscribe(...), const unsubUser = currentUser.subscribe(...)) and
return a cleanup function from onMount that calls unsubAuth(), unsubSigs(), and
unsubUser(); keep the existing loadFile/fetchFileSignatures logic intact and
ensure the returned function runs on component unmount to avoid memory leaks.
- Around line 106-136: watchSigningSession leaves the EventSource open on
component unmount and calls JSON.parse without protection; wrap the JSON.parse
call inside a try/catch in the sseConnection.onmessage handler to safely handle
malformed payloads (log or toast an error and return), ensure
sseConnection.onerror closes the connection and surfaces errors, and add a
component teardown/destroy hook that checks and closes sseConnection (the same
sseConnection variable created with new EventSource(...) in watchSigningSession)
to guarantee the SSE is closed when the component unmounts.
In @platforms/esigner/src/routes/(protected)/files/+page.svelte:
- Around line 31-40: The isAuthenticated.subscribe call inside onMount creates a
persistent subscription that isn't cleaned up; capture the unsubscribe function
returned by isAuthenticated.subscribe (e.g., const unsubscribe =
isAuthenticated.subscribe(...)) and ensure it's called when the component is
destroyed by returning a cleanup function from onMount or calling unsubscribe
inside onDestroy; keep the existing logic that calls goto('/auth') or
fetchInvitations()/fetchDocuments() within the subscription callback and simply
invoke unsubscribe() in the teardown to avoid the memory leak.
In @platforms/esigner/src/routes/(protected)/files/new/+page.svelte:
- Around line 22-27: The isAuthenticated.subscribe(...) inside onMount creates a
subscription that is never unsubscribed; capture the unsubscribe function
returned by isAuthenticated.subscribe in onMount (e.g., const unsubscribe =
isAuthenticated.subscribe(...)) and ensure you call it when the component is
destroyed (use onDestroy(unsubscribe) or call unsubscribe inside an onDestroy
callback) so the subscription is cleaned up; reference the onMount,
isAuthenticated.subscribe, unsubscribe, onDestroy and goto symbols when making
the change.
In @platforms/esigner/src/routes/+page.svelte:
- Line 4: The import list includes an unused symbol; remove the unused import
`isAuthenticated` from the import statement that currently reads `import {
isAuthenticated, initializeAuth } from '$lib/stores/auth';` so that only
`initializeAuth` is imported, keeping the component clean and avoiding
unused-import warnings.
🟠 Major comments (33)
platforms/esigner/package.json-40-40 (1)
40-40: Upgrade axios to patch known security vulnerabilities.The axios version
1.6.7(Jan 2024) contains multiple security vulnerabilities:
- CVE-2025-27152 — SSRF/credential leakage when absolute URLs are used (fixed in 1.8.2)
- CVE-2025-58754 — data: URL handler unbounded memory allocation (DoS)
- CVE-2024-57965 — origin-check helper concern (fixed in 1.7.8+)
Upgrade to at minimum
1.8.2to patch critical CVEs, or ideally to1.13.2(latest stable).platforms/esigner-api/package.json-9-9 (1)
9-9: Build script may fail on Windows.The
cp -rcommand is Unix-specific and will fail on Windows environments. Consider using a cross-platform solution.🔎 Proposed fix using cross-platform tools
Replace the build script with a cross-platform alternative:
- "build": "tsc && cp -r src/web3adapter/mappings dist/web3adapter/", + "build": "tsc && node -e \"require('fs').cpSync('src/web3adapter/mappings', 'dist/web3adapter/mappings', {recursive: true})\"",Or use a dedicated package like
cpy-cli:- "build": "tsc && cp -r src/web3adapter/mappings dist/web3adapter/", + "build": "tsc && cpy 'src/web3adapter/mappings/**' dist/web3adapter/mappings",Then add to devDependencies:
"cpy-cli": "^5.0.0"platforms/esigner-api/src/utils/version.ts-7-20 (1)
7-20: Add input validation and error handling.The
compareVersionsfunction lacks validation for malformed version strings. Invalid inputs like empty strings, null, or non-numeric segments will produce incorrect results:
"".split('.')produces[""]Number("")produces0Number("abc")producesNaNNaNcomparisons always returnfalseThis could allow authentication bypass if used in security-critical paths (as indicated by AuthController.ts usage).
🔎 Proposed fix with input validation
export function compareVersions(version1: string, version2: string): number { + // Validate input format + const versionRegex = /^\d+(\.\d+)*$/; + if (!version1 || !version2 || !versionRegex.test(version1) || !versionRegex.test(version2)) { + throw new Error(`Invalid version format. Expected format: X.Y.Z (e.g., "1.0.0")`); + } + const v1Parts = version1.split('.').map(Number); const v2Parts = version2.split('.').map(Number); for (let i = 0; i < Math.max(v1Parts.length, v2Parts.length); i++) { const v1Part = v1Parts[i] || 0; const v2Part = v2Parts[i] || 0; if (v1Part < v2Part) return -1; if (v1Part > v2Part) return 1; } return 0; }platforms/esigner-api/package.json-15-30 (1)
15-30: Upgrade dependencies to patch known security vulnerabilities.Multiple critical and high-severity CVEs have been confirmed in production dependencies:
- axios 1.6.7: CVE-2025-27152 (SSRF/credential leakage), CVE-2024-57965 (origin-validation), CVE-2025-58754 (DoS). Upgrade to ≥1.8.2 (or 1.12.0+ for complete coverage).
- express 4.18.2: CVE-2024-29041 (open redirect), CVE-2024-10491 (response.links injection), CVE-2024-45296, CVE-2024-52798 (transitive dependencies). Upgrade to ≥4.19.0 or 5.2.1.
- multer 1.4.5-lts.1: CVE-2025-47944 (unhandled exception/crash), CVE-2025-47935 (memory leak), CVE-2025-7338 (DoS). Upgrade to ≥2.0.2.
- jsonwebtoken 9.0.2: Minor patch available (9.0.3).
All flagged packages have patched versions available. Update to the latest available versions and re-run dependency scanning.
platforms/esigner/src/lib/components/UserMenuDropdown.svelte-43-48 (1)
43-48: Fix backdrop accessibility issue.The backdrop has
role="button"buttabindex="-1", creating an accessibility violation. This makes it appear as a button to assistive technologies while being keyboard-inaccessible.Remove both attributes since the backdrop is only meant to capture outside clicks, not be a focusable element.
🔎 Proposed fix
<!-- Backdrop --> <div class="fixed inset-0 z-10" onclick={closeDropdown} - role="button" - tabindex="-1" ></div>platforms/esigner-api/src/controllers/SignatureController.ts-47-53 (1)
47-53: Overly permissive CORS for a security-sensitive endpoint.
Access-Control-Allow-Origin: '*'allows any origin to connect to this SSE stream for signing session status updates. Consider restricting this to trusted origins, especially given the security-sensitive nature of the signing workflow.🔎 Proposed fix
res.writeHead(200, { 'Content-Type': 'text/event-stream', 'Cache-Control': 'no-cache', 'Connection': 'keep-alive', - 'Access-Control-Allow-Origin': '*', + 'Access-Control-Allow-Origin': process.env.ESIGNER_ALLOWED_ORIGIN || 'http://localhost:5173', 'Access-Control-Allow-Headers': 'Cache-Control' });platforms/esigner-api/src/controllers/SignatureController.ts-93-96 (1)
93-96: Error response after SSE headers may fail silently.If an error occurs after
res.writeHead()(line 47) but within the try block (e.g., during interval execution), the catch block attemptsres.status(500).json(). Once headers are written for SSE, this will either throw or fail silently. Consider sending an SSE error event instead.🔎 Proposed fix
} catch (error) { console.error("Error getting signing session status:", error); - res.status(500).json({ error: "Failed to get signing session status" }); + // Check if headers have been sent (SSE mode) + if (res.headersSent) { + res.write(`data: ${JSON.stringify({ type: "error", message: "Internal server error" })}\n\n`); + res.end(); + } else { + res.status(500).json({ error: "Failed to get signing session status" }); + } }platforms/esigner-api/src/controllers/AuthController.ts-48-56 (1)
48-56: Generated session UUID is not persisted or validated.The
sessionUUID is generated ingetOfferbut not stored. Whenloginis called with this session, there's no verification that it's a legitimate session created by this server. An attacker could use any UUID.Consider storing sessions temporarily (e.g., in Redis or in-memory with TTL) and validating in
loginthat the session exists and hasn't expired.platforms/esigner/src/lib/stores/files.ts-80-84 (1)
80-84: Remove explicitContent-Typeheader for multipart requests.When using
FormData, the browser automatically setsContent-Type: multipart/form-datawith the correct boundary. Explicitly setting it overrides the boundary, which will cause the server to fail parsing the request body.🔎 Proposed fix
- const response = await apiClient.post('/api/files', formData, { - headers: { - 'Content-Type': 'multipart/form-data', - }, - }); + const response = await apiClient.post('/api/files', formData);platforms/esigner/src/routes/(protected)/files/+page.svelte-31-40 (1)
31-40: Subscription not unsubscribed — potential memory leak.The
isAuthenticated.subscribe()call returns an unsubscribe function that should be called when the component is destroyed. Otherwise, the subscription persists after navigation.🔎 Proposed fix
+ import { onDestroy } from 'svelte'; + + let unsubscribe: () => void; + onMount(() => { - isAuthenticated.subscribe((auth) => { + unsubscribe = isAuthenticated.subscribe((auth) => { if (!auth) { goto('/auth'); } else { fetchInvitations(); fetchDocuments(); } }); }); + + onDestroy(() => { + if (unsubscribe) unsubscribe(); + });platforms/esigner-api/src/index.ts-23-48 (1)
23-48: Server starts before database initialization completes.The
app.listen()call at line 109 executes immediately, whileAppDataSource.initialize()runs asynchronously. This means the server starts accepting requests before the database connection is established, which can cause requests to fail.🔎 Proposed fix: Move server startup inside the initialization callback
// Initialize database connection and adapter AppDataSource.initialize() .then(async () => { console.log("Database connection established"); console.log("Web3 adapter initialized"); // Initialize platform eVault for eSigner try { const platformService = PlatformEVaultService.getInstance(); const exists = await platformService.checkPlatformEVaultExists(); if (!exists) { console.log("🔧 Creating platform eVault for eSigner..."); const result = await platformService.createPlatformEVault(); console.log(`✅ Platform eVault created successfully: ${result.w3id}`); } else { console.log("✅ Platform eVault already exists for eSigner"); } } catch (error) { console.error("❌ Failed to initialize platform eVault:", error); // Don't exit the process, just log the error } + + // Start server after successful initialization + app.listen(port, () => { + console.log(`eSigner API server running on port ${port}`); + }); }) .catch((error: any) => { console.error("Error during initialization:", error); process.exit(1); }); - -// Start server -app.listen(port, () => { - console.log(`eSigner API server running on port ${port}`); -});Also applies to: 109-111
platforms/esigner-api/src/database/entities/SignatureContainer.ts-48-50 (1)
48-50: Relation type mismatch withFileSigneeentity.
SignatureContainerdeclares aManyToOnerelation toFileSignee, butFileSignee(from the relevant snippets) declares aOneToOneinverse relation. These must match: if one side isOneToOne, the other should also beOneToOne.🔎 Proposed fix: Change to OneToOne
- @ManyToOne(() => FileSignee, (fileSignee) => fileSignee.signature, { nullable: true }) + @OneToOne(() => FileSignee, (fileSignee) => fileSignee.signature, { nullable: true }) @JoinColumn({ name: "fileSigneeId" }) fileSignee!: FileSignee | null;Also add the import:
import { Entity, PrimaryGeneratedColumn, Column, CreateDateColumn, UpdateDateColumn, ManyToOne, JoinColumn, + OneToOne, } from "typeorm";platforms/esigner/src/routes/(auth)/auth/+page.svelte-93-129 (1)
93-129: EventSource is never closed on component unmount.The
EventSourcecreated inwatchEventStreamis not cleaned up when the component is destroyed, causing a resource leak and potential errors if events arrive after navigation.🔎 Proposed fix: Store reference and close on destroy
+ let eventSource: EventSource | null = null; + + onDestroy(() => { + if (eventSource) { + eventSource.close(); + } + }); + onMount(async () => { // ... existing code ... function watchEventStream(id: string) { const sseUrl = new URL(`/api/auth/sessions/${id}`, PUBLIC_ESIGNER_BASE_URL).toString(); - const eventSource = new EventSource(sseUrl); + eventSource = new EventSource(sseUrl); // ... rest of function ... }Committable suggestion skipped: line range outside the PR's diff.
platforms/esigner-api/src/web3adapter/watchers/subscriber.ts-15-20 (1)
15-20: Unsafe environment variable access may cause runtime crashes.Casting
process.envvalues withas stringsilently bypasses TypeScript's undefined check. If these environment variables are missing, the adapter will receiveundefined(cast to string"undefined"), causing subtle bugs or failures.🔎 Proposed fix: Add validation
dotenv.config({ path: path.resolve(__dirname, "../../../../../.env") }); + +const ESIGNER_MAPPING_DB_PATH = process.env.ESIGNER_MAPPING_DB_PATH; +const PUBLIC_REGISTRY_URL = process.env.PUBLIC_REGISTRY_URL; +const PUBLIC_ESIGNER_BASE_URL = process.env.PUBLIC_ESIGNER_BASE_URL; + +if (!ESIGNER_MAPPING_DB_PATH || !PUBLIC_REGISTRY_URL || !PUBLIC_ESIGNER_BASE_URL) { + throw new Error( + "Missing required environment variables: ESIGNER_MAPPING_DB_PATH, PUBLIC_REGISTRY_URL, or PUBLIC_ESIGNER_BASE_URL" + ); +} + export const adapter = new Web3Adapter({ schemasPath: path.resolve(__dirname, "../mappings/"), - dbPath: path.resolve(process.env.ESIGNER_MAPPING_DB_PATH as string), - registryUrl: process.env.PUBLIC_REGISTRY_URL as string, - platform: process.env.PUBLIC_ESIGNER_BASE_URL as string, + dbPath: path.resolve(ESIGNER_MAPPING_DB_PATH), + registryUrl: PUBLIC_REGISTRY_URL, + platform: PUBLIC_ESIGNER_BASE_URL, });platforms/esigner-api/src/database/entities/Group.ts-14-15 (1)
14-15: "Group" is a SQL reserved keyword - specify explicit table name.The
@Entity()decorator without a table name will create a table called "group", which is a reserved keyword in PostgreSQL and other databases. This can cause query issues.🔎 Proposed fix
-@Entity() +@Entity("groups") export class Group {platforms/esigner-api/src/services/FileService.ts-82-100 (1)
82-100: Logic error in getUserFiles: invited files may be duplicated or incorrectly filtered.The condition at line 93 (
!invitedFileIds.has(fileSignee.fileId) || !ownedFiles.find(...)) is confusing and potentially incorrect. The first part will always be false sinceinvitedFileIdswas built frominvitedFiles. The intent seems to be to add invited files that aren't already owned.🔎 Proposed fix
- const invitedFileIds = new Set(invitedFiles.map(fs => fs.fileId)); + const ownedFileIds = new Set(ownedFiles.map(f => f.id)); const allFiles = [...ownedFiles]; // Add invited files that aren't already in the list for (const fileSignee of invitedFiles) { - if (!invitedFileIds.has(fileSignee.fileId) || !ownedFiles.find(f => f.id === fileSignee.fileId)) { + if (!ownedFileIds.has(fileSignee.fileId)) { if (fileSignee.file) { allFiles.push(fileSignee.file); } } }platforms/esigner/src/routes/(protected)/files/[id]/+page.svelte-22-42 (1)
22-42: Memory leak: Subscriptions are never cleaned up on component unmount.The
isAuthenticated.subscribe(),signatures.subscribe(), andcurrentUser.subscribe()calls create subscriptions that persist after the component unmounts. In Svelte,onMountshould return a cleanup function to unsubscribe.🔎 Proposed fix
onMount(async () => { - isAuthenticated.subscribe((auth) => { + const unsubAuth = isAuthenticated.subscribe((auth) => { if (!auth) { goto('/auth'); } }); const fileId = $page.params.id; await loadFile(fileId); await fetchFileSignatures(fileId); // Watch for signature changes - signatures.subscribe(() => { + const unsubSigs = signatures.subscribe(() => { checkIfUserSigned(); }); // Watch for user changes - currentUser.subscribe(() => { + const unsubUser = currentUser.subscribe(() => { checkIfUserSigned(); }); + + return () => { + unsubAuth(); + unsubSigs(); + unsubUser(); + sseConnection?.close(); + }; });platforms/esigner/src/routes/(protected)/files/[id]/+page.svelte-106-136 (1)
106-136: SSE connection not closed on component unmount; missing error handling for JSON.parse.The
sseConnectionis only closed when specific events occur, but if the user navigates away before signing completes, the connection remains open. Additionally,JSON.parse(e.data)can throw if the server sends malformed data.🔎 Proposed fix
function watchSigningSession(sessionId: string) { const baseUrl = PUBLIC_ESIGNER_BASE_URL || 'http://localhost:3004'; const sseUrl = new URL(`/api/signatures/session/${sessionId}`, baseUrl).toString(); sseConnection = new EventSource(sseUrl); sseConnection.onmessage = (e) => { - const data = JSON.parse(e.data); + let data; + try { + data = JSON.parse(e.data); + } catch { + console.error('Failed to parse SSE message:', e.data); + return; + } if (data.type === 'signed' && data.status === 'completed') {platforms/esigner-api/src/services/UserService.ts-19-32 (1)
19-32: Ename normalization inconsistency: findOrCreateUser doesn't normalize like findByEname.
findByEnamehandles both@useranduserformats, butfindOrCreateUseruses exact match. This could cause duplicate users if one is created with@userand another lookup usesuser.🔎 Proposed fix
async findOrCreateUser( ename: string ): Promise<{ user: User; token: string }> { - let user = await this.userRepository.findOne({ - where: { ename }, - }); + // Use findByEname for consistent normalization + let user = await this.findByEname(ename); if (!user) { - user = await this.createBlankUser(ename); + // Normalize ename before creating + const normalizedEname = ename.startsWith('@') ? ename.slice(1) : ename; + user = await this.createBlankUser(normalizedEname); } const token = signToken({ userId: user.id }); return { user, token }; }platforms/esigner/src/lib/stores/auth.ts-10-13 (1)
10-13: localStorage access during SSR will throw.Accessing
localStoragedirectly in SvelteKit code that may run server-side will cause errors during SSR. Wrap in a browser check or ensure this code only runs client-side.🔎 Proposed fix
+import { browser } from '$app/environment'; + export const initializeAuth = async () => { authInitialized.set(false); + if (!browser) { + authInitialized.set(true); + return false; + } const token = localStorage.getItem('esigner_auth_token');platforms/esigner-api/src/controllers/WebhookController.ts-105-106 (1)
105-106: Same fragile string parsing for admin IDs.The inline parsing
a.split("(")[1].split(")")[0]is duplicated and equally fragile. Extract to a shared utility function with proper error handling.platforms/esigner-api/src/services/GroupService.ts-58-79 (1)
58-79: Performance concern: fallback loads all private groups into memory.For non-2-member lookups, this fetches every private group into memory and filters in-memory. This will degrade as the number of groups grows.
Consider adding a database-level query that can handle arbitrary member counts, or at minimum add a limit or pagination to prevent unbounded memory usage.
platforms/esigner-api/src/controllers/WebhookController.ts-174-182 (1)
174-182: Repeated fragile parsing pattern for sender and group.Lines 175 and 180 use the same unsafe split pattern. This should use the same safe extraction utility suggested earlier.
platforms/esigner-api/src/services/PlatformEVaultService.ts-88-90 (1)
88-90: Add timeout to registry entropy request.Same concern - no timeout on the axios GET request.
🔎 Proposed fix
const { data: { token: registryEntropy }, - } = await axios.get(new URL("/entropy", registryUrl).toString()); + } = await axios.get(new URL("/entropy", registryUrl).toString(), { timeout: 10000 });platforms/esigner-api/src/controllers/FileController.ts-154-156 (1)
154-156: Header injection risk in Content-Disposition.If
file.namecontains newlines, quotes, or special characters, it could lead to header injection or malformed headers. Sanitize the filename before using it in the header.🔎 Proposed fix
+ // Sanitize filename for Content-Disposition header + const sanitizedFilename = file.name.replace(/["\r\n]/g, '_'); res.setHeader('Content-Type', file.mimeType); - res.setHeader('Content-Disposition', `attachment; filename="${file.name}"`); + res.setHeader('Content-Disposition', `attachment; filename="${sanitizedFilename}"`); res.setHeader('Content-Length', file.size.toString());platforms/esigner-api/src/controllers/WebhookController.ts-26-31 (1)
26-31: Fire-and-forget HTTP call without error handling or timeout.The axios POST to
ANCHR_URLhas no.catch(), no timeout, and noawait. Unhandled promise rejection could crash the process in certain Node.js versions. Add error handling or at minimum a catch block.🔎 Proposed fix
if (process.env.ANCHR_URL) { - axios.post( + axios.post( new URL("esigner", process.env.ANCHR_URL).toString(), - req.body - ); + req.body, + { timeout: 5000 } + ).catch(err => { + console.error("Failed to forward webhook to ANCHR_URL:", err.message); + }); }platforms/esigner-api/src/services/PlatformEVaultService.ts-99-107 (1)
99-107: Add timeout to external HTTP requests.The axios POST to the provisioner has no timeout. External service calls should have timeouts to prevent hanging requests.
🔎 Proposed fix
const { data } = await axios.post( new URL("/provision", provisionerUrl).toString(), { registryEntropy, namespace: uuidv4(), verificationId, publicKey: "0x00000000000000000000000000000000000000", }, + { timeout: 30000 } // 30 second timeout );platforms/esigner-api/src/services/PlatformEVaultService.ts-95-97 (1)
95-97: Hardcoded fallback verification ID is a security concern.Using a hardcoded UUID as fallback for
DEMO_VERIFICATION_CODEcould be a security risk if accidentally used in production. Consider requiring the environment variable or failing explicitly.🔎 Proposed fix
- const verificationId = - process.env.DEMO_VERIFICATION_CODE || - "d66b7138-538a-465f-a6ce-f6985854c3f4"; + const verificationId = process.env.DEMO_VERIFICATION_CODE; + if (!verificationId) { + throw new Error("DEMO_VERIFICATION_CODE environment variable is required"); + }platforms/esigner/src/routes/(protected)/files/new/+page.svelte-22-27 (1)
22-27: Subscription memory leak.The
isAuthenticated.subscribe()call returns an unsubscribe function that is never called. This can cause memory leaks on component destruction.🔎 Proposed fix
+ import { onDestroy } from 'svelte'; + + let unsubscribe: () => void; + onMount(async () => { - isAuthenticated.subscribe((auth) => { + unsubscribe = isAuthenticated.subscribe((auth) => { if (!auth) { goto('/auth'); } }); // ... rest of onMount }); + + onDestroy(() => { + if (unsubscribe) unsubscribe(); + });platforms/esigner-api/src/controllers/WebhookController.ts-91-94 (1)
91-94: Fragile string parsing without validation.The pattern
ref.split("(")[1].split(")")[0]assumes a specific format. If the input doesn't match (e.g., missing parentheses), this will throw or produce undefined. Add validation.🔎 Proposed fix
+ // Helper to safely extract ID from ref format "type(id)" + const extractIdFromRef = (ref: string): string | null => { + const match = ref.match(/\(([^)]+)\)/); + return match ? match[1] : null; + }; + if (ref && typeof ref === "string") { - const userId = ref.split("(")[1].split(")")[0]; - return await this.userService.getUserById(userId); + const userId = extractIdFromRef(ref); + if (userId) { + return await this.userService.getUserById(userId); + } } return null;platforms/esigner-api/src/services/MessageService.ts-91-104 (1)
91-104: UnrestrictedPartial<Message>allows overwriting protected fields.Using
Object.assignwithPartial<Message>permits callers to overwriteid,createdAt,sender, orgrouprelations. Consider restricting updateable fields to prevent accidental or malicious overwrites.🔎 Proposed fix
- async updateMessage(id: string, messageData: Partial<Message>): Promise<Message | null> { + async updateMessage( + id: string, + messageData: Pick<Partial<Message>, 'text' | 'isArchived'> + ): Promise<Message | null> { // Get the current message, merge the data, and save it to trigger ORM events const currentMessage = await this.getMessageById(id); if (!currentMessage) { throw new Error("Message not found"); } - // Merge the new data with the existing message - Object.assign(currentMessage, messageData); + // Only update allowed fields + if (messageData.text !== undefined) { + currentMessage.text = messageData.text; + } + if (messageData.isArchived !== undefined) { + currentMessage.isArchived = messageData.isArchived; + } // Save the merged message to trigger ORM subscribers const updatedMessage = await this.messageRepository.save(currentMessage); return updatedMessage; }platforms/esigner-api/src/services/SignatureService.ts-92-102 (1)
92-102: Sessions are never removed from the Map, leading to memory leaks.The
setTimeoutonly marks sessions as expired but never removes them from thesessionsMap. Over time, this will cause unbounded memory growth.🔎 Proposed fix
// Set up expiration cleanup setTimeout(() => { const session = this.sessions.get(sessionId); if (session && session.status === "pending") { session.status = "expired"; - this.sessions.set(sessionId, session); + // Keep expired session briefly for status queries, then remove + this.sessions.set(sessionId, session); + setTimeout(() => { + this.sessions.delete(sessionId); + }, 5 * 60 * 1000); // Remove 5 minutes after expiration } }, 15 * 60 * 1000);Alternatively, implement a periodic cleanup that removes sessions older than a threshold.
platforms/esigner-api/src/services/SignatureService.ts-35-35 (1)
35-35: Sessions won't be accessible across server instances in load-balanced deployments.The in-memory
Mapmeans signing sessions are lost on restart and won't be accessible if requests are distributed across multiple server instances. ThegetSigningSessionStatusendpoint uses SSE polling to check session state—if the session was created on one server instance but the polling request routes to another instance, it will fail.For single-instance deployments this is acceptable given the 15-minute session TTL, but horizontal scaling would break the session status polling. If this service is expected to run behind a load balancer, session state should be moved to Redis or a database before production deployment.
| "devDependencies": { | ||
| "@eslint/compat": "^1.2.5", | ||
| "@eslint/js": "^9.18.0", | ||
| "@sveltejs/adapter-static": "^3.0.8", |
Contributor
There was a problem hiding this comment.
Remove conflicting SvelteKit adapter causing pipeline failure.
The package includes both @sveltejs/adapter-static (line 19) and @sveltejs/adapter-node (line 39). SvelteKit projects must use exactly one adapter. This conflict is likely causing the pnpm run check failure in CI.
Since adapter-node is in dependencies (correct placement for adapters), remove adapter-static from devDependencies.
🔎 Proposed fix
"@sveltejs/adapter-static": "^3.0.8",Remove line 19 entirely, or if you need static generation instead:
- "@sveltejs/adapter-static": "^3.0.8",and move adapter-node out of dependencies, replacing it with adapter-static in devDependencies.
Also applies to: 39-39
🤖 Prompt for AI Agents
In @platforms/esigner/package.json around line 19, package.json currently lists
both "@sveltejs/adapter-static" in devDependencies and "@sveltejs/adapter-node"
in dependencies, which conflicts; remove the duplicate adapter so only one
adapter is present by deleting the "@sveltejs/adapter-static" entry from
devDependencies (or alternatively move "@sveltejs/adapter-node" into
devDependencies and replace it with "@sveltejs/adapter-static" in dependencies
if you intend static generation); ensure only one of the adapter package names
("@sveltejs/adapter-static" or "@sveltejs/adapter-node") remains and that
adapters live in the correct dependencies section based on your chosen
deployment strategy.
sosweetham
approved these changes
Jan 5, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Fix all issues with AI Agents 🤖
In @platforms/esigner/src/routes/(protected)/files/new/+page.svelte:
- Line 409: The placeholder text currently reads "Search by name or ename..."
which is unclear; update the input's placeholder attribute (placeholder="Search
by name or ename...") to use standard terminology such as "email" or include
both "email" and "username" for clarity — e.g., change to "Search by name or
email..." or "Search by name, email, or username..." so users understand what to
enter.
- Around line 22-38: The isAuthenticated.subscribe created inside onMount is
never cleaned up causing a leak; capture the returned unsubscribe from
isAuthenticated.subscribe (or switch to Svelte's $isAuthenticated
auto-subscription) and ensure you call unsubscribe when the component is
destroyed—e.g., return the unsubscribe function from onMount (or call it in
onDestroy) so the subscription created by isAuthenticated.subscribe is properly
removed while keeping the existing redirect logic that calls goto('/auth') and
leaving fetchFiles and the API user fetch as-is.
- Around line 344-381: Extract the inline onclick logic from the Next button
into a new async function (e.g., handleNextStep) and call that from the button;
move all branches that reference uploadedFile, selectedFile, handleFileUpload,
apiClient.patch, displayName, description, isLoading and currentStep into this
function, preserving try/catch/finally behavior and ensuring isLoading is
toggled correctly, returned errors are handled the same way
(console.error/alert) and currentStep is set to 2 only after successful
upload/update; update the button to use onclick={handleNextStep} and keep the
disabled binding and label logic unchanged.
- Around line 229-238: The drag-drop div (the element using dragOver,
handleDragOver, handleDragLeave, handleDrop and role="button") needs a keyboard
handler so keyboard users can open the file picker; add a keydown handler (e.g.,
handleKeyDown) on that same element that listens for Enter and Space and invokes
the same action the click/file-select uses (trigger the hidden file input or
call the existing file-selection function used elsewhere), and implement
handleKeyDown to preventDefault for Space and call that file-select trigger;
ensure the handler is wired up where handleDragOver/handleDragLeave/handleDrop
are defined.
🧹 Nitpick comments (3)
platforms/esigner/src/routes/(protected)/files/new/+page.svelte (3)
10-10: Consider stronger TypeScript typing instead ofany.Replace
anytypes with proper interfaces or type definitions for better type safety and IDE support. For example, define aFileinterface and aUserinterface.🔎 Suggested approach
Define types at the top of the script section:
<script lang="ts"> import { onMount } from 'svelte'; import { goto } from '$app/navigation'; import { isAuthenticated } from '$lib/stores/auth'; import { files, fetchFiles, uploadFile } from '$lib/stores/files'; import { apiClient } from '$lib/utils/axios'; import { inviteSignees } from '$lib/stores/invitations'; + + interface FileData { + id: string; + name: string; + displayName?: string; + description?: string; + size: number; + mimeType: string; + } + + interface UserData { + id: string; + name?: string; + ename: string; + } let currentStep = $state(1); - let selectedFile = $state<any>(null); + let selectedFile = $state<FileData | null>(null); let uploadedFile = $state<File | null>(null); let searchQuery = $state(''); - let searchResults = $state<any[]>([]); - let selectedUsers = $state<any[]>([]); + let searchResults = $state<UserData[]>([]); + let selectedUsers = $state<UserData[]>([]);Also applies to: 13-14
54-54: Consider replacingalert()with a more user-friendly notification system.Using browser
alert()dialogs interrupts the user experience and looks dated. Consider implementing a toast notification system or inline error messages.
393-396: Add null safety checks forselectedFile.In Steps 2 and 3, the code accesses properties of
selectedFile(e.g.,selectedFile?.mimeType,selectedFile?.size) using optional chaining, which is good. However, these steps should only be reachable whenselectedFileis defined. Consider adding a guard clause or assertion at the beginning of each step.🔎 Suggested approach
Add runtime checks at the start of Steps 2 and 3:
<!-- Step 2: Invite Signees --> {#if currentStep === 2} {#if !selectedFile} <div class="text-red-600">Error: No file selected</div> {:else} <div class="bg-white rounded-lg shadow-sm border border-gray-200 p-6"> <!-- existing content --> </div> {/if} {/if}Also applies to: 504-507
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
platforms/esigner/.svelte-kit/ambient.d.tsis excluded by!**/.svelte-kit/**platforms/esigner/.svelte-kit/generated/server/internal.jsis excluded by!**/.svelte-kit/**,!**/generated/**
📒 Files selected for processing (3)
.env.exampleplatforms/esigner/src/routes/(protected)/files/[id]/+page.svelteplatforms/esigner/src/routes/(protected)/files/new/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- platforms/esigner/src/routes/(protected)/files/[id]/+page.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-13T10:34:52.527Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 415
File: infrastructure/eid-wallet/src/env.d.ts:8-8
Timestamp: 2025-11-13T10:34:52.527Z
Learning: In infrastructure/eid-wallet, PUBLIC_PLATFORM_URL should not be added to .env.example or configured as a static environment variable. The platform URL is extracted dynamically through URI parsing according to the protocol specification, and all fallbacks for platform URL are being removed.
Applied to files:
.env.example
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 66-66: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
.env.example (1)
66-66: Remove the suggestion—this addition is consistent with established codebase patterns.The PR 415 learning applies specifically to
PUBLIC_PLATFORM_URL, which is extracted dynamically and not included in.env.example. However,PUBLIC_ESIGNER_BASE_URLis a service-specific base URL, not a platform URL. The codebase already includes over a dozen service base URLs in.env.example(e.g.,PUBLIC_REGISTRY_URL,PUBLIC_PROVISIONER_URL,PUBLIC_PICTIQUE_BASE_URL,PUBLIC_CERBERUS_BASE_URL,PUBLIC_EVOTING_BASE_URL), all configured with fallbacks in their respective implementations. AddingPUBLIC_ESIGNER_BASE_URLfollows this established pattern and does not contradict PR 415.Likely an incorrect or invalid review comment.
platforms/esigner/src/routes/(protected)/files/new/+page.svelte (2)
157-171: LGTM: Utility functions are well-implemented.Both
formatFileSizeandgetFileIconare clean, straightforward helper functions that enhance the user experience.
174-567: Overall UI implementation looks solid.The three-step wizard is well-structured with clear visual progression, appropriate loading states, and user-friendly feedback throughout the flow. The component handles both upload and selection scenarios effectively.
Comment on lines
+22
to
+38
| onMount(async () => { | ||
| isAuthenticated.subscribe((auth) => { | ||
| if (!auth) { | ||
| goto('/auth'); | ||
| } | ||
| }); | ||
| // Get current user ID from API | ||
| try { | ||
| const response = await apiClient.get('/api/users'); | ||
| currentUserId = response.data.id; | ||
| } catch (err) { | ||
| console.error('Failed to get current user:', err); | ||
| } | ||
| fetchFiles(); | ||
| }); |
Contributor
There was a problem hiding this comment.
Memory leak: unsubscribe from store subscription.
The isAuthenticated.subscribe() call creates a subscription that is never cleaned up, causing a memory leak. In Svelte, you should either use the auto-subscripting $ syntax or return an unsubscribe function from onMount.
🔎 Proposed fix
onMount(async () => {
- isAuthenticated.subscribe((auth) => {
+ const unsubscribe = isAuthenticated.subscribe((auth) => {
if (!auth) {
goto('/auth');
}
});
// Get current user ID from API
try {
const response = await apiClient.get('/api/users');
currentUserId = response.data.id;
} catch (err) {
console.error('Failed to get current user:', err);
}
fetchFiles();
+
+ return unsubscribe;
});Alternatively, use Svelte's auto-subscription syntax:
onMount(async () => {
- isAuthenticated.subscribe((auth) => {
- if (!auth) {
- goto('/auth');
- }
- });
+ if (!$isAuthenticated) {
+ goto('/auth');
+ }
// Get current user ID from API
try {
const response = await apiClient.get('/api/users');
currentUserId = response.data.id;
} catch (err) {
console.error('Failed to get current user:', err);
}
fetchFiles();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onMount(async () => { | |
| isAuthenticated.subscribe((auth) => { | |
| if (!auth) { | |
| goto('/auth'); | |
| } | |
| }); | |
| // Get current user ID from API | |
| try { | |
| const response = await apiClient.get('/api/users'); | |
| currentUserId = response.data.id; | |
| } catch (err) { | |
| console.error('Failed to get current user:', err); | |
| } | |
| fetchFiles(); | |
| }); | |
| onMount(async () => { | |
| const unsubscribe = isAuthenticated.subscribe((auth) => { | |
| if (!auth) { | |
| goto('/auth'); | |
| } | |
| }); | |
| // Get current user ID from API | |
| try { | |
| const response = await apiClient.get('/api/users'); | |
| currentUserId = response.data.id; | |
| } catch (err) { | |
| console.error('Failed to get current user:', err); | |
| } | |
| fetchFiles(); | |
| return unsubscribe; | |
| }); |
Suggested change
| onMount(async () => { | |
| isAuthenticated.subscribe((auth) => { | |
| if (!auth) { | |
| goto('/auth'); | |
| } | |
| }); | |
| // Get current user ID from API | |
| try { | |
| const response = await apiClient.get('/api/users'); | |
| currentUserId = response.data.id; | |
| } catch (err) { | |
| console.error('Failed to get current user:', err); | |
| } | |
| fetchFiles(); | |
| }); | |
| onMount(async () => { | |
| if (!$isAuthenticated) { | |
| goto('/auth'); | |
| } | |
| // Get current user ID from API | |
| try { | |
| const response = await apiClient.get('/api/users'); | |
| currentUserId = response.data.id; | |
| } catch (err) { | |
| console.error('Failed to get current user:', err); | |
| } | |
| fetchFiles(); | |
| }); |
🤖 Prompt for AI Agents
In @platforms/esigner/src/routes/(protected)/files/new/+page.svelte around lines
22-38, The isAuthenticated.subscribe created inside onMount is never cleaned up
causing a leak; capture the returned unsubscribe from isAuthenticated.subscribe
(or switch to Svelte's $isAuthenticated auto-subscription) and ensure you call
unsubscribe when the component is destroyed—e.g., return the unsubscribe
function from onMount (or call it in onDestroy) so the subscription created by
isAuthenticated.subscribe is properly removed while keeping the existing
redirect logic that calls goto('/auth') and leaving fetchFiles and the API user
fetch as-is.
Comment on lines
+229
to
+238
| <div | ||
| role="button" | ||
| tabindex="0" | ||
| class={`border-2 border-dashed rounded-lg p-8 text-center transition-colors ${ | ||
| dragOver ? 'border-blue-500 bg-blue-50' : 'border-gray-300' | ||
| }`} | ||
| ondragover={handleDragOver} | ||
| ondragleave={handleDragLeave} | ||
| ondrop={handleDrop} | ||
| > |
Contributor
There was a problem hiding this comment.
Add keyboard event handlers for accessibility.
The drag-and-drop area has role="button" and tabindex="0" but lacks keyboard event handlers. Users navigating with keyboards won't be able to trigger the file selection.
🔎 Proposed fix
<div
role="button"
tabindex="0"
class={`border-2 border-dashed rounded-lg p-8 text-center transition-colors ${
dragOver ? 'border-blue-500 bg-blue-50' : 'border-gray-300'
}`}
ondragover={handleDragOver}
ondragleave={handleDragLeave}
ondrop={handleDrop}
+ onkeydown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ document.getElementById('file-input')?.click();
+ }
+ }}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div | |
| role="button" | |
| tabindex="0" | |
| class={`border-2 border-dashed rounded-lg p-8 text-center transition-colors ${ | |
| dragOver ? 'border-blue-500 bg-blue-50' : 'border-gray-300' | |
| }`} | |
| ondragover={handleDragOver} | |
| ondragleave={handleDragLeave} | |
| ondrop={handleDrop} | |
| > | |
| <div | |
| role="button" | |
| tabindex="0" | |
| class={`border-2 border-dashed rounded-lg p-8 text-center transition-colors ${ | |
| dragOver ? 'border-blue-500 bg-blue-50' : 'border-gray-300' | |
| }`} | |
| ondragover={handleDragOver} | |
| ondragleave={handleDragLeave} | |
| ondrop={handleDrop} | |
| onkeydown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| document.getElementById('file-input')?.click(); | |
| } | |
| }} | |
| > |
🤖 Prompt for AI Agents
In @platforms/esigner/src/routes/(protected)/files/new/+page.svelte around lines
229-238, The drag-drop div (the element using dragOver, handleDragOver,
handleDragLeave, handleDrop and role="button") needs a keyboard handler so
keyboard users can open the file picker; add a keydown handler (e.g.,
handleKeyDown) on that same element that listens for Enter and Space and invokes
the same action the click/file-select uses (trigger the hidden file input or
call the existing file-selection function used elsewhere), and implement
handleKeyDown to preventDefault for Space and call that file-select trigger;
ensure the handler is wired up where handleDragOver/handleDragLeave/handleDrop
are defined.
Comment on lines
+344
to
+381
| <div class="flex justify-end mt-6"> | ||
| <button | ||
| onclick={async () => { | ||
| if (uploadedFile && !selectedFile) { | ||
| // Upload the file with name/description when proceeding | ||
| try { | ||
| await handleFileUpload(uploadedFile); | ||
| } catch (err) { | ||
| // Error already handled in handleFileUpload | ||
| return; | ||
| } | ||
| } else if (selectedFile && !uploadedFile) { | ||
| // Update existing file with new name/description if changed | ||
| try { | ||
| isLoading = true; | ||
| const response = await apiClient.patch(`/api/files/${selectedFile.id}`, { | ||
| displayName: displayName.trim() || selectedFile.name, | ||
| description: description.trim() || null | ||
| }); | ||
| selectedFile = response.data; | ||
| } catch (err) { | ||
| console.error('Failed to update file:', err); | ||
| alert('Failed to update signature container details'); | ||
| return; | ||
| } finally { | ||
| isLoading = false; | ||
| } | ||
| } | ||
| if (selectedFile || uploadedFile) { | ||
| currentStep = 2; | ||
| } | ||
| }} | ||
| disabled={(!selectedFile && !uploadedFile) || isLoading} | ||
| class="px-6 py-2 bg-blue-600 text-white rounded-lg font-semibold hover:bg-blue-700 transition-colors disabled:opacity-50 disabled:cursor-not-allowed" | ||
| > | ||
| {isLoading ? 'Saving...' : 'Next: Invite Signees'} | ||
| </button> | ||
| </div> |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract complex button logic into a separate function.
The "Next" button contains 30+ lines of inline logic handling multiple scenarios (upload new file, update existing file). This reduces readability and makes testing difficult.
🔎 Proposed refactor
Extract the logic into a dedicated function:
+ async function handleProceedToStep2() {
+ if (uploadedFile && !selectedFile) {
+ // Upload the file with name/description when proceeding
+ try {
+ await handleFileUpload(uploadedFile);
+ } catch (err) {
+ // Error already handled in handleFileUpload
+ return;
+ }
+ } else if (selectedFile && !uploadedFile) {
+ // Update existing file with new name/description if changed
+ try {
+ isLoading = true;
+ const response = await apiClient.patch(`/api/files/${selectedFile.id}`, {
+ displayName: displayName.trim() || selectedFile.name,
+ description: description.trim() || null
+ });
+ selectedFile = response.data;
+ } catch (err) {
+ console.error('Failed to update file:', err);
+ alert('Failed to update signature container details');
+ return;
+ } finally {
+ isLoading = false;
+ }
+ }
+ if (selectedFile || uploadedFile) {
+ currentStep = 2;
+ }
+ }
+
// ... in template:
<div class="flex justify-end mt-6">
<button
- onclick={async () => {
- if (uploadedFile && !selectedFile) {
- // Upload the file with name/description when proceeding
- try {
- await handleFileUpload(uploadedFile);
- } catch (err) {
- // Error already handled in handleFileUpload
- return;
- }
- } else if (selectedFile && !uploadedFile) {
- // Update existing file with new name/description if changed
- try {
- isLoading = true;
- const response = await apiClient.patch(`/api/files/${selectedFile.id}`, {
- displayName: displayName.trim() || selectedFile.name,
- description: description.trim() || null
- });
- selectedFile = response.data;
- } catch (err) {
- console.error('Failed to update file:', err);
- alert('Failed to update signature container details');
- return;
- } finally {
- isLoading = false;
- }
- }
- if (selectedFile || uploadedFile) {
- currentStep = 2;
- }
- }}
+ onclick={handleProceedToStep2}
disabled={(!selectedFile && !uploadedFile) || isLoading}
class="px-6 py-2 bg-blue-600 text-white rounded-lg font-semibold hover:bg-blue-700 transition-colors disabled:opacity-50 disabled:cursor-not-allowed"
>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @platforms/esigner/src/routes/(protected)/files/new/+page.svelte around lines
344-381, Extract the inline onclick logic from the Next button into a new async
function (e.g., handleNextStep) and call that from the button; move all branches
that reference uploadedFile, selectedFile, handleFileUpload, apiClient.patch,
displayName, description, isLoading and currentStep into this function,
preserving try/catch/finally behavior and ensuring isLoading is toggled
correctly, returned errors are handled the same way (console.error/alert) and
currentStep is set to 2 only after successful upload/update; update the button
to use onclick={handleNextStep} and keep the disabled binding and label logic
unchanged.
| type="text" | ||
| bind:value={searchQuery} | ||
| oninput={searchUsers} | ||
| placeholder="Search by name or ename..." |
Contributor
There was a problem hiding this comment.
Clarify or fix placeholder text: "ename".
The placeholder mentions "ename" which is non-standard terminology. This should likely be "email" or "username" for clarity.
🔎 Proposed fix
- placeholder="Search by name or ename..."
+ placeholder="Search by name or email..."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| placeholder="Search by name or ename..." | |
| placeholder="Search by name or email..." |
🤖 Prompt for AI Agents
In @platforms/esigner/src/routes/(protected)/files/new/+page.svelte around line
409, The placeholder text currently reads "Search by name or ename..." which is
unclear; update the input's placeholder attribute (placeholder="Search by name
or ename...") to use standard terminology such as "email" or include both
"email" and "username" for clarity — e.g., change to "Search by name or
email..." or "Search by name, email, or username..." so users understand what to
enter.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/check-code.yml (1)
29-31: The.env.exampleapproach is sound—file exists and contains the required variable.The change copies
.env.exampleto.envto providePUBLIC_ESIGNER_BASE_URLand other environment variables during the lint check. The file is properly committed and includes all necessary configuration, making this a straightforward solution to the build issue.Optional improvements to consider:
Error handling: Add a check to catch failures:
run: | cp .env.example .env || { echo "Failed to copy .env.example"; exit 1; } pnpm checkExplicit env variables: For CI clarity, you could set critical variables directly via GitHub Actions'
env:directive instead of relying on the example file:- name: Check Code env: PUBLIC_ESIGNER_BASE_URL: "http://localhost:3004" run: pnpm checkThe current implementation is functional as-is; these are refinements for robustness and clarity.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/check-code.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: lint
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description of change
Move eSigner to monorepo
Issue Number
Type of Change
How the change has been tested
Change checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.