chore(release): back-merge main into staging after v14.1.0#223
Merged
Conversation
* fix: Update all backend require statements to use renamed PascalCase module paths
* Merge code
Merge code
* fix(security): reject regex audience in JWT verify (BUG-001 / #55)
`verifyJWTTokenWithC` and `verifyJWTTokenWithCV2` were passing
`new RegExp(companyId)` β where `companyId` comes straight from the
`companyid` request header β as the `audience` option to `jwt.verify`.
A header value like `.*` matched any company in the token audience,
allowing cross-tenant access; the same input doubled as a ReDoS vector.
Replace the regex with two explicit checks:
1. Reject any `companyid` that isn't a 24-char Mongo ObjectId
(defense-in-depth against control chars / regex metacharacters).
2. Verify the JWT without an `audience` option, then run a strict
membership check (`isCompanyInAudience`) against the comma-joined
`aud` claim β exact match per entry, trim-tolerant.
Response shapes and HTTP codes are unchanged, so existing clients see
no behavioural diff on legitimate requests. Cross-tenant headers now
return 401 instead of leaking data.
Closes #55
* fix(security): replace wildcard CORS with env-driven allow-list (BUG-002 / #56)
`app.use(cors({origin: '*'}))` let every browser origin call every API
route β combined with credentialed cookies and JS-readable tokens this
opened a CSRF / token-theft path from any third-party origin.
Introduce `utils/cors.js` with three helpers:
- `buildCorsAllowList(env)` β derives the allow-list from WEBURL,
APIURL, and an optional CORS_ORIGINS comma-list.
- `isOriginAllowed(origin, list)` β exact-match check, trim/trailing-
slash tolerant, and permissive for no-Origin / `null` / `file://`
cases so curl, native mobile clients, and the Electron desktop
build keep working.
- `corsOriginDelegate` β drop-in `origin:` callback for the `cors`
middleware.
`index.js` now wires that delegate into `app.use(cors({...}))`, and
`.env.example` documents the new optional `CORS_ORIGINS` variable for
multi-domain deployments.
The helper is intentionally generic so the Socket.io fix (BUG-003)
can reuse it.
Closes #56
* fix(security): replace wildcard Socket.io CORS with env allow-list (BUG-003 / #57)
The Socket.io server was instantiated with `cors: {origin: '*', credentials: true}`,
the configuration browsers reject when sane β but engine.io accepts it. Any
malicious origin could open an authenticated websocket on behalf of a
logged-in victim, receive their realtime task/comment stream, and emit
events as them.
Reuse the shared CORS allow-list helper at `utils/cors.js` (same module
introduced for the Express HTTP fix in BUG-002 / #56) and wire it into
Socket.io's `cors.origin` callback. `credentials: true` is kept β it is
safe now that origins are explicitly checked.
`utils/cors.js` and the `CORS_ORIGINS` entry in `.env.example` are
duplicated with the BUG-002 PR (#103) on purpose: each fix branches from
`staging` independently per the rollout plan, and git merges identical
blobs as a no-op so order of landing does not matter.
Closes #57
* fix(security): move Socket.io admin UI creds to env, default-off (BUG-004 / #58)
The Socket.io admin UI was instantiated with hardcoded credentials:
instrument(io, {
auth: { type: "basic", username: "alian", password: "$2a$12$HHe..." },
namespaceName: "/admin",
mode: "development"
});
Anyone with read access to the repo could:
- reuse the bcrypt hash, or compute the cleartext offline,
- reach the deployed `/admin` namespace,
- read connected sockets / rooms / events and emit events as any user.
`mode: "development"` further disabled the TLS-only enforcement on the
admin namespace in production deployments.
Replace with a small env-driven helper `getAdminUiConfig(env)`:
- Requires BOTH `SOCKETIO_ADMIN_USERNAME` and `SOCKETIO_ADMIN_PASSWORD_HASH`
to be set (non-empty, non-whitespace). If either is missing the
helper returns null and `instrument()` is skipped β the admin UI is
disabled entirely. This is the new default for fresh installs and
for any deployment that doesn't explicitly opt in.
- `mode` follows `NODE_ENV`: `production` enables TLS-only protection
on the admin namespace; everything else (including unrecognised
values) stays `development` to fail closed.
`socket/socketinit.js` exports the helper so the regression suite at
`.claude/tests/test-bug-004.js` can exercise it directly. `.env.example`
documents the new variables and how to generate a bcrypt hash.
Closes #58
* fix(security): generate password-reset token with crypto.randomBytes (BUG-005 / #59)
The `/api/v2/sendForgotPasswordEmail` flow built reset tokens as
let temp = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
let token = '';
for (let i = 0; i < 8; i++) {
token += temp.charAt(Math.floor(Math.random() * temp.length));
}
β 8 chars from a 62-char alphabet (~48 bits) drawn from `Math.random()`,
which is neither cryptographically secure nor large enough to resist
brute force against the reset endpoint.
Replace with a single helper:
exports.generateResetToken = () =>
crypto.randomBytes(32).toString('hex');
That's 32 bytes (256 bits) of CSPRNG output, hex-encoded to 64 URL-safe
characters. The reset link format, DB shape, and call sites are
unchanged β only the token-generation expression is swapped.
The other forgot-password route (`/api/v2/auth/forgot-password` β
`exports.sendForgotPassword`) already used a JWT and is unaffected.
Closes #59
* fix(security): remove unauthenticated file-disclosure endpoint (BUG-007 / #61)
`GET /api/v1/checkAvaibility` was registered in `Modules/auth/routes.js:9`
and handled by `exports.checkAvaibility` in `Modules/auth/controller.js:109`.
The handler did a `fs.readFileSync('./utils/licensesValidate.js')` and
returned the file's source in the JSON body β with no authentication and
no rate limit. The file does not even exist in the repository, and `git
grep` shows zero callers under `Modules/`, `utils/`, or `frontend/src/`,
so this is pure dead code that doubled as a source-disclosure / LFI
attack surface (today returns ENOENT; on any deployment that adds the
file it would dump licensing logic verbatim).
Delete the route, the controller function, and the now-unused
`const fs = require("fs")` import. No backward-compat concerns.
Closes #61
* fix(stability): respond instead of ReferenceError in createProjectFun (BUG-009 / #63)
`Modules/createProject/controller.js:104` had `reject(error)` in the
outer `catch` block of `createProjectFun`. The function is declared as
`async (req, res) => { ... }`, not a `new Promise` constructor body, so
`reject` was never defined in that scope.
Any synchronous throw inside the surrounding `try { β¦ }` (most easily
triggered by a malformed request body that breaks `checkProjectPlan`'s
synchronous prelude) hit that catch, threw a `ReferenceError`, masked
the original error, and left the response hanging until the client or
upstream proxy timed out β usually 30β60s.
Replace `reject(error)` with the same response shape the rest of the
function already uses, and log the underlying error via Winston so the
real cause is captured:
logger.error(`createProjectFun error: ${error.message || error}`);
res.status(400).send({
status: false,
statusText: error.message || error,
});
Out of scope here: BUG-010 (#64) β the `if (data.status) { β¦ }` branch
without an `else` that causes the same handler to hang when
`checkProjectPlan` resolves with `{status: false}`. Tracked separately.
Closes #63
* fix(stability): add missing else in createProjectFun (BUG-010 / #64)
`Modules/createProject/controller.js:83-93` had an `if (data.status) { β¦ }`
on the resolution of `exports.checkProjectPlan(req)` with no `else`
branch. Today every failure path of `checkProjectPlan` happens to go
through `reject`, so the missing branch isn't currently reachable β but
the code is one refactor away from a silent hang:
- If anyone ever moves a failure path from `reject({status:false})`
to `resolve({status:false})` (a common Promise refactor), the
handler falls through with no response and the request hangs until
the upstream proxy times out.
- `checkProjectPlan` increments `projectCount.projectCount` (and one
of `publicCount`/`privateCount`) on the company document *before*
it validates plan limits. Any future failure path that lands here
via `resolve` would leave that speculative increment in place.
Add a defensive `else` branch that mirrors the existing `.catch`
rollback logic:
} else {
exports.removeProjectCount(req.body.CompanyId, req.body.isPrivateSpace);
res.status(400).send({
status: false,
statusText: (data && data.statusText) || 'Project plan check did not pass.',
});
}
The branch is reachable today only via test-time stubs (the regression
test exercises it) but closes the latent hang for any future change to
the validation contract.
Sibling fix BUG-009 (#63) β `reject(error)` in the outer `catch` β is
its own PR.
Closes #64
* fix(security): stop overwriting req.body, allow-list invite keys (BUG-011 / #65)
`Modules/auth/controller/verifyInvitation.js:26-42` decoded the base64
invitation blob from `req.body.id`, parsed it `&`/`=`-style, and then:
req.body = finalObj;
That replaced `req.body` with whatever key/value pairs the attacker
chose to encode β no allow-list, no shape validation, and any later
code reading `req.body.<anything>` would see attacker input as if it
had been validated. Combined with predictable invitation tokens (the
broader BUG-005 / #59 family) it was a parameter-injection primitive.
Introduce a small exported pure helper:
exports.parseInviteBlob = (encoded) => { β¦ }
It accepts only the keys actually consumed downstream β `userId`,
`companyId`, `linkId`, `docId` β and returns either a fresh local
object containing just those keys or `null` for invalid input. It
also:
- Validates the input alphabet with a strict base64 regex before
invoking `atob` (the npm `atob@2.1.2` package is permissive and
happily returns garbage on malformed input).
- Splits `key=value` on the FIRST `=` only, so values that legally
contain `=` aren't truncated.
- Skips parts without `=`.
- Returns `null` if the blob contains none of the allow-listed keys.
`exports.checkPermission` now reads from a local `invite` object
returned by `parseInviteBlob` and `req.body` is left untouched.
Closes #65
* fix(security): tighten + env-configure auth rate limit (BUG-012 / #66)
`Modules/auth/helper.js:180-194` hard-coded the auth rate-limit numbers
inside `manageResetAttempt`:
- 9 failed attempts allowed inside a window,
- window labelled `fiveMinutes` but actually 10 min,
- 10-minute block once tripped.
That left every login / forgot-password / reset-password endpoint at
roughly 9 attempts per IP per 10 minutes β permissive enough that a
patient distributed-credential-stuffing attacker could grind through
indefinitely. The variable name `fiveMinutes` also disagreed with its
value, which is a maintenance trap.
Tighten the defaults and lift the knobs out to env:
AUTH_RATE_LIMIT_MAX_ATTEMPTS default 5 (was 9)
AUTH_RATE_LIMIT_WINDOW_MS default 15 min
AUTH_RATE_LIMIT_BLOCK_MS default 30 min (was 10)
A new exported helper `getRateLimitConfig()` reads from `process.env`,
falls back to safe defaults on missing / non-numeric / zero input, and
clamps to sane minimums (>=1 attempt, >=1s window/block). It's read at
request time so an operator can adjust without restarting.
`.env.example` documents the three new variables; operators who need
laxer behaviour for staging / load testing can raise them.
The bad `fiveMinutes` variable name is gone; the threshold check now
uses `attempts >= MAX_ATTEMPTS` so the configured limit means what it
says.
Closes #66
* fix(security): re-validate company membership per request, cached (BUG-013 / #67)
The JWT audience claim is frozen at login and lasts JWT_EXP (24h
default). A user removed from a company between login and token
expiry kept access until the token expired β for up to a day on
defaults.
Add a live membership re-check that runs after the existing audience
verification inside `verifyJWTTokenWithC` and `verifyJWTTokenWithCV2`:
verifyCompanyMembership(uid, companyId)
1. Validate uid/companyId look like ObjectIds (cheap rejection
of garbage so we don't waste a Mongo round-trip).
2. Check the in-memory cache (`node-cache`) β separate positive
and negative entries.
3. Fall back to `users.findOne({_id, AssignCompany})` and cache
the boolean for `MEMBERSHIP_CACHE_TTL_SECONDS` (default 60s).
invalidateMembershipCache(uid, companyId?)
- Exported so the membership-change flow (add / remove member)
can wipe the cache and surface changes immediately.
When the check fails, the middleware now returns HTTP 403 with
`isLogout: true` so the frontend logs the user out rather than
silently retrying with a token that the server no longer trusts.
Both middlewares are now `async`. The membership cost is one Mongo
lookup per (user, company) per minute by default β negligible next
to the existing JWT verify on the same path.
Doc: `MEMBERSHIP_CACHE_TTL_SECONDS` added to `.env.example` so
operators can dial it down (faster removal propagation) or up
(less Mongo traffic).
Closes #67
* fix(security): tighten verifyEmail against loose-equality bypass (BUG-014 / #68)
`Modules/auth/controller/verifyEmail.js` had three bugs that combined
into an exploitable verification bypass:
1. Input validation used `!req.body.token`, which lets falsy non-empty
non-strings through. `![]` is `false`, so `{ token: [] }` passed.
2. The success branch used `==` (loose equality):
response.verificationToken == req.body.token
`"" == []` is `true` under JavaScript's loose-equality rules. The
codebase stores `verificationToken: ""` both after a successful
verification and as a fresh-account default, so any account in
that state could be re-verified by sending `{ uid, token: [] }`.
3. There was no fallback `else`, so unmatched states (e.g. stored
token was `null` or `undefined`) silently fell off the end and
left the response hanging until the proxy timed out.
Also: the expiry check called
new Date(verificationTokenTime).setMinutes(...)
on potentially-missing fields. `new Date(null)` is the epoch and
`new Date(undefined)` is `Invalid Date`. Either way the resulting
`ValidTime < new Date()` comparison silently bypassed the expiry for
documents missing `verificationTokenTime`.
The rewrite:
- Validates `uid` and `token` with `typeof x === 'string' && x.length`
so arrays / objects / numbers / null are rejected up front.
- Validates `response.verificationToken` is a non-empty string before
comparing β empty / null / undefined stored token β "expired".
- Treats missing or invalid `verificationTokenTime` as expired rather
than silently bypassing the 10-minute window.
- Uses strict equality `===` for the token comparison.
- Sends exactly one response in every branch so the request never
hangs.
Closes #68
* fix(stability): stop calling reject after resolve in HandleTask (BUG-016 / #70)
`Modules/tasks/helpers/mongo_helper.js` had two sites where `.catch`
handlers called `reject(error)` AFTER the outer Promise had already
been settled by an earlier `resolve(...)`. Calling `reject` on an
already-settled Promise is a no-op, so the underlying history /
inner-update failures were silently swallowed.
Site 1 (the explicit bug-report site, lines 80-99 of `HandleTask`):
MongoDbCrudOpration(...).then((response) => {
socketEmitter.emit(...);
resolve({status: true, ...}); // β settles here
exports.HandleHistory('task', ...)
.catch((error) => { reject(error); }); // β no-op, log lost
exports.HandleHistory('project', ...)
.catch((error) => { reject(error); }); // β no-op, log lost
})
Refactor to:
MongoDbCrudOpration(...).then(async (response) => {
socketEmitter.emit(...);
await Promise.allSettled([
exports.HandleHistory('task', ...).catch(log),
exports.HandleHistory('project', ...).catch(log),
]);
resolve({status: true, ...});
})
`Promise.allSettled` makes a single history failure non-fatal (the
batch continues) and each per-task `.catch` logs via `logger.error`
so the failure is now visible.
Site 2 (the sibling instance at `convertToSubTaskFunction:335-337`):
MongoDbCrudOpration(...).then((result) => {
...lots of code, including resolve(...)...
}).catch((error) => { reject(error); }) // β also no-op
The inner `.catch` here is reachable only after the inner `resolve(...)`
has already settled the outer Promise (Mongo settles the inner before
the .catch fires, and resolve happens inside the .then). Replace
`reject(error)` with `logger.error(...)` so the failure is logged
instead of silently dropped.
Closes #70
* fix(stability): replace forEach(async) fire-and-forget pattern (BUG-017 / #71)
`array.forEach(async x => { β¦ })` doesn't wait for the callbacks and
silently swallows any rejection. The audit flagged ~14 sites across the
codebase. After reading each in context they split into two categories:
CATEGORY B β real concurrency bug (the callback actually had an `await`
inside, so the loop "completed" before the awaited work did):
- Modules/AI/controller.js:387 β `await limitCountUpdate(...)` was
fire-and-forget, so per-chunk quota updates raced the outer resolve.
Replace with `forβ¦of` + `await` (serial; at most one chunk has
`usage` per response so serial is the natural shape).
- Modules/storage/server/helpers/bucket.helper.js:195 β `await
copyFile(...)` was fire-and-forget, so `resolve()` ran before any
file had finished copying. Replace with
`await Promise.allSettled(imageArray.map(async ...))` so copies
run in parallel and a single failure doesn't abort the batch.
Also fix a latent secondary bug uncovered during the test:
`copyFile` itself did `await fs.cp(source, dest, callback)`. `fs.cp`
is callback-style and returns `undefined`, so `await` resolved
immediately and the function returned before the copy started β the
surrounding `Promise.allSettled` fix would have been toothless
without this. Promisify the callback so awaits up the chain actually
wait.
CATEGORY A β the `async` keyword was unused (no `await` inside, callback
was sync). The keyword was misleading but harmless. Drop it so the
pattern is consistent everywhere:
- Modules/AI/controller.js:395 (string concat)
- Modules/projectSetting/controller.js:57, 154, 158 (uses .then chain)
- frontend/src/store/ProjectData/actions.js:206, 261 (Vuex commit)
- frontend/src/components/molecules/TaskAudioFiles/TaskAudioFiles.vue
:351, :419, :424 (object construction)
- frontend/src/components/organisms/HourlyMilestone/helper.js:194
- frontend/src/components/templates/CreateProject/TaskStatusForm.vue
:317
- frontend/src/components/templates/CreateProject/ProjectTaskTypeForm.vue
:266
Frontend build verified clean (`npm run build`). No remaining
`forEach(async ...)` patterns in executable code (only in fix-comments
that reference the old pattern).
Closes #71
* fix(stability): surface allSettled rejection reasons (BUG-018 / #72)
The audit description for BUG-018 β `Promise.allSettled rejection
branch never responds, outer Promise stays pending` β turned out to be
inaccurate in two ways once verified against the source:
1. The `else` branch on `if (rejected.length === 0)` already existed
and called `reject({status: false, statusText: 'error in
createProject'})`. The outer Promise did NOT hang.
2. `Promise.allSettled` never rejects in the first place β it always
resolves with the per-promise outcomes. So even without an else,
the surrounding `.then` would still fire.
What WAS broken (and is fixed here) is the loss of debugging info: the
existing else body was a generic statusText, with every per-query
`reason` in the `rejected` array discarded. The caller and the log
learnt nothing about WHICH prerequisite query failed or WHY.
Replace the else's reject body with a structured payload that includes:
- `rejectedCount` / `totalCount`
- `reasons`: array of `{index, reason}` carrying the actual error
message for each failed prerequisite query.
β¦and log the same array via `logger.error` so the failure is also in
the server log for ops.
Closes #72
* fix(stability): declare setChat with const (BUG-019 / #73)
`Modules/MainChats/controller.js:71` did:
setChat = await MongoDbCrudOpration(req.headers['companyid'], β¦);
with no `const`/`let`/`var`. That makes `setChat` an implicit global on
the Node process. Two concurrent requests share the same slot and can
clobber each other β the second response can include the first
request's data. (Under strict mode the assignment throws
`ReferenceError`; the file isn't strict today, so the live behaviour is
the silent cross-request leak.)
Trivial fix: add `const`. The variable is only used on the very next
line, so block scope is the right shape.
Closes #73
* fix(stability): align cache set/remove key in MainChats (BUG-020 / #74)
`Modules/MainChats/controller.js` had a cache-key mismatch between
writer and invalidator:
- `getChats` set the cache at
`mainChat:${req.headers['companyid']}`
- `updateMainChat` tried to invalidate at
`mainChat:${JSON.parse(JSON.stringify(result))?._id}`
β i.e. the updated document's `_id`, NOT the companyId.
The two keys never matched. `removeCache(...)` was a silent no-op and
stale chats stayed cached until the 3600s TTL expired.
Centralise the key in a small helper so both sites use the same shape:
const mainChatCacheKey = (companyId) => `mainChat:${companyId}`;
β¦and use it for both `myCache.set` and `removeCache`. The helper is
exported so the regression test can pin the shape.
Closes #74
* perf: add MongoDB indexes for hot query paths (BUG-021 / #75)
`utils/mongo-handler/createSchema.js` declared exactly one index β the
`sessions` TTL β and left every other collection un-indexed. Every
multi-field filter (tasks by project/sprint, history by task, users
by email, etc.) was a collection scan, scaling linearly with data
size.
Add compound + single-field indexes for the hottest filter paths the
codebase actually uses. ESR (Equality β Sort β Range) order where
applicable. Mongoose creates indexes at startup via `ensureIndexes`;
existing collections will see one-time background builds, which is
fine for any non-trivial dataset.
Per-company collections:
- tasks: (ProjectID, sprintId, deletedStatusKey)
(sprintId, deletedStatusKey)
(AssigneeUserId), (ParentTaskId), (TaskKey)
- comments: ('objId.taskId', deletedStatusKey)
('objId.sprintId'), ('objId.projectId')
- history: (TaskId, createdAt: -1), (ProjectId, createdAt: -1)
- timesheet: (TicketID), (userId, ProjectId)
- userId: (userId)
- sprints: (ProjectID, deletedStatusKey)
- folders: (ProjectID)
- projects: (deletedStatusKey)
Global DB collections:
- users: (Employee_Email), (AssignCompany)
The AssignCompany index is specifically required by the
BUG-013 per-request membership re-check that landed in
PR #114.
- userAuth: (email)
- companyUsers: (userId)
- sessions: (refreshToken), (userId) (TTL on createdAt already exists)
- resetAttempt: (ip)
Note on multi-tenancy: each company has its own MongoDB database, so
the document-level `companyId` field is redundant with the database
name and not indexed here. The audit's "missing companyId index"
framing was inaccurate; what's needed are the per-collection filter
indexes added above.
Closes #75
* chore(deps): upgrade sharp 0.32.6 β ^0.34.0 (BUG-022 / #76)
`sharp@0.32.6` carries CVE-2024-28219 (heap-buffer overflow via crafted
SVG). The fix was released in 0.33.2; bumping to the current major
(^0.34.0) β installed as 0.34.5 here β picks up that fix and several
subsequent security/perf releases.
The two existing callers in `Modules/storage/...` use the long-stable
`sharp(input).resize().withMetadata().toFile(...)` shape, which is
unchanged across 0.32 β 0.34, so no code changes are required.
A regression test at `.claude/tests/test-bug-022.js` (local, not
committed) confirms package.json + the installed module + a runtime
resize/metadata round-trip.
Closes #76
* fix(security): guard sharp() inputs against pixel-bomb DoS (BUG-023 / #77)
`Modules/storage/wasabi/controller.js` and
`Modules/storage/server/helpers/bucket.helper.js` invoke `sharp(buffer)`
and `sharp(file.path)` with no validation on either the input file size
or the image's pixel dimensions. An authenticated user could upload a
30000x30000 PNG and OOM the worker β `bodyParser` only limits the
request envelope, not what libvips materialises in memory.
New shared helper at `utils/imageGuard.js`:
- `guardFile(filePath)` β fs.statSync size check + metadata() check
- `guardBuffer(buffer)` β buffer-length check + metadata() check
- `getLimits()` reads `MAX_IMAGE_FILE_BYTES` and `MAX_IMAGE_PIXELS`
from env (defaults 25 MB and 50 megapixels). Both clamp to sane
minimums so a misconfigured 0 doesn't accidentally disable the
guard.
- On rejection, throws `ImageGuardError` with `statusCode: 413` and
a stable `code` (`IMAGE_TOO_LARGE` / `IMAGE_TOO_MANY_PIXELS`) for
the calling controller to surface to the client.
Wired into all five sharp() callsites:
- wasabi/controller.js: uploadThumbnailFile (file), uploadThumbnailFileFromBase64 (buffer)
- bucket.helper.js: uploadStorageThumbnailFile (file), uploadStorageThumbnailFilev2 (file/buffer)
The metadata-only sharp().metadata() call inside the guard does NOT
materialise the full image β it parses the header only, so the guard
itself cannot be DoS'd by the same input it's meant to reject.
`.env.example` documents the two new variables.
Closes #77
* perf: switch readFileSync in request handlers to async (BUG-024 / #78)
`Modules/storage/wasabi/controller.js` had five `fs.readFileSync` calls
inside request-handler flows (file uploads to Wasabi):
- updateLocalWasabiFiles (line 80)
- uploadThumbnailFile (line 619)
- uploadFileWasabiPromise (lines 695, 758)
- uploadPublicAssetsToWasabi (line 1010)
β¦and `Modules/notification/notification-middleware/controllerV2.js`
had a sixth `readFileSync` on every push-notification request to read
`brandSettings.json`.
Every one of these blocks the Node event loop for the duration of the
disk read. Under concurrent load, p99 latency on completely unrelated
routes spiked because they were stuck waiting on the event loop. The
sharp() resizing path was particularly bad because it reads the
resulting thumbnail back synchronously after writing it.
Switch each site to `await fs.promises.readFile(...)` and adjust the
surrounding Promise/callback structure:
- Promise constructors become `async (resolve, reject) => { β¦ }` so
`await` is legal inside.
- The sharp `.toFile(outputFile, async (err) => { β¦ })` callback
becomes async too.
- Each read is wrapped in try/catch that calls `reject` with the
underlying message instead of bubbling an uncaught exception.
- `uploadPublicAssetsToWasabi` was already `async`, so the change
there is a one-liner.
The notification handler additionally wraps the file-exists check in
try/catch so a permissions error doesn't unhandled-promise-reject and
crash the worker.
Closes #78
* fix(observability): route console.* through Winston (BUG-025 / #79)
Auth + MainChats had several `console.log` / `console.error` calls
that printed PII and error stacks to raw stdout, which in hosted
deployments lands in shared aggregators searchable by anyone with
log access.
Replace each call in:
- Modules/auth/controller/sendInvitation.js (5 sites)
- Modules/MainChats/controller.js (3 sites)
β¦with `logger.info` / `logger.error` from the existing Winston
config. Messages now go through the same redaction-able structured
pipeline as the rest of the app.
This PR covers the two files explicitly named in the audit. A
broader sweep across the codebase is possible but kept out of
scope to keep the diff reviewable.
Closes #79
* fix(stability): strict equality on isEmailVerified check (BUG-026 / #80)
`Modules/Users/controller.js:74` used `response.isEmailVerified ==
false`, which also matches `0`, `""`, `null`, `undefined`, `NaN`. A
user document missing the field entirely (legacy data, fresh records
before the field was added) would be classified as "Email Not
Verified" even though it was never explicitly false.
Swap to `=== false`. Documents with the field missing now fall
through to the next branch, which is the intended behaviour.
Audit-accuracy note: the audit referenced `Modules/usersModule/...`,
which doesn't exist after the naming-conventions refactor β the real
site is `Modules/Users/controller.js`. The audit mentioned the same
pattern was "widespread" but a fresh codebase grep finds this is the
only `== false` against `isEmailVerified`.
Closes #80
* fix(stability): guard checkUserAndCompany against duplicate res.send (BUG-027 / #81)
`Modules/Users/controller.js` `checkUserAndCompany` has six different
`res.send` sites across three nested .then / .catch chains. Today the
early-return pattern keeps them mutually exclusive in normal traffic,
but the structure is fragile:
- If `res.send` throws synchronously inside the inner `.then`
(e.g. ERR_HTTP_HEADERS_SENT triggered by upstream middleware), the
sync throw is caught by the inner `.catch`, which then calls
`res.send` AGAIN β a guaranteed cascade.
- Any future refactor that adds another branch without an explicit
`return` re-introduces the race.
Wrap every `res.send` through a local `sendOnce(payload)` helper backed
by a boolean flag. Duplicate writes are suppressed with a `logger.warn`
instead of throwing, and the request only ever produces one response.
(Also picks up a stray `== false` from BUG-026 in the same function
that was already fixed on another branch, harmless if both merge.)
Closes #81
* fix: correct copy-paste error in companyId validation message (BUG-028 / #82)
`Modules/logTime/controllerV2.js:77` returned "ProjectId is required"
when the missing field was actually companyId. One-character fix
(copy-paste from the adjacent projectId check at line 70).
Closes #82
* fix: validate timeDuration shape before .split(':') (BUG-029 / #83)
`Modules/logTime/controllerV2.js:162` did
`req.body.timeDuration.split(':')` after only a falsy-existence check
at line 53. That left two crash / data-corruption paths open:
- Non-string values that pass `!req.body.timeDuration` (e.g. an array
`[]`, an object `{}`, a number `42`) reach `.split` and either
crash with `TypeError: timeDuration.split is not a function` or
produce surprising arrays.
- A string without a colon (e.g. "1") returns `["1"]` from .split;
`diffArr[1]` is undefined, `+undefined` is NaN, and `diffMin` is
NaN. That NaN then writes straight into Mongo as the log duration.
Add an explicit shape check before the split: `typeof === 'string'`
AND matches `/^\d+:\d+$/`. Any other value gets a clean 400 with
"timeDuration must be a string in HH:MM format".
Closes #83
* fix(stability): guard against missing response.data in sprints (BUG-030 / #84)
`Modules/sprints/controller.js:45` did
const data = JSON.parse(JSON.stringify(response.data));
then read `data.projectCount.privateChannels` /
`data.planFeature.maxPrivateChannels`. If `response.data` was
undefined (race with a newly-created company, malformed write, etc.)
the surrounding Promise crashed with TypeError on the first nested
field access.
Guard up front:
- If `response` or `response.data` is null/undefined, log a warning
and resolve(false) (the "not allowed" outcome) instead of throwing.
- Default `projectCount` and `planFeature` to `{}` so individual
missing sub-fields evaluate to `undefined` instead of crashing.
The audit framed this as "JSON.parse throws on undefined" β actually
JSON.parse(JSON.stringify(undefined)) returns undefined silently. The
real crash was the subsequent property chain on the now-undefined
`data`.
Closes #84
* fix(observability): log silently-swallowed fetch failures (BUG-031 / #85)
`Modules/tasks/helpers/handleNotification.js:22-30` and the sibling at
line 33-43 had
.catch(error => { return null })
β silently turning every DB failure (fetchProjectDetailsSingle /
fetchTaskDetails) into "no data" downstream. The notification flow
continued with projectData=null / taskData=null and emitted
notifications with missing context.
Keep best-effort semantics (notifications shouldn't fail user
operations), but log via Winston so the failure is visible in ops.
Closes #85
* fix(BUG-032): consistent soft-delete filtering on list/count endpoints
Three list-style endpoints either lacked a soft-delete filter or used
an over-narrow form that excluded legacy documents (where the flag was
unset):
* Modules/Comments/controller.js β `getPaginatedMessages` previously
returned soft-deleted comments because it never matched on
`isDeleted`. Added `{ isDeleted: { $ne: true } }` to the aggregation,
matching `searchComments`. Also changed
`searchMessageFromMainChat`'s `isDeleted: false` to
`isDeleted: { $ne: true }` so legacy comments (no flag at all) keep
showing up.
* Modules/Project/controller/getSprintFolder.js β the `count` branch
matched on `projectId` only, so deleted sprints/folders inflated the
sidebar counters. Aligned with the listing branches
(`deletedStatusKey: { $nin: [1] }`).
* Modules/tasks/controller/getTabSyncTasks.js β `getTabSynctTaskWithTable`
used `{ deletedStatusKey: { $in: [0] } }`, which dropped any task
whose flag was unset. Sibling helpers (`getTaskCount`,
`getTabSynctTaskWithoutTable` via plain `0`) need the legacy/undefined
case to pass. Switched to `{ $in: [0, undefined] }` to bring it back
in line.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(BUG-033): reconcile sprint task counts when secondary writes fail
The multi-step task flows in `Modules/tasks/helpers/mongo_helper.js`
(`moveTaskFunction`, `convertToListSubTask`, the merge-task path) all
follow the same shape: a primary findOneAndUpdate on a task, then one
or more independent `$inc` calls to keep the sprint's denormalised
`tasks` counter in sync. Each `$inc` is fire-and-forget β if it fails,
the count drifts away from the live tasks-collection state and the
sidebar shows the wrong number until someone notices.
A full MongoDB transaction would be the textbook fix, but
`MongoDbCrudOpration` does not thread a session through, and Mongo
transactions require a replica-set deployment that the self-hosted
footprint cannot guarantee. A self-healing reconcile is safer and
works in every deployment mode.
Changes:
- **Modules/tasks/helpers/reconcileTaskCount.js (new)** β
`computeLiveTaskCount(companyId, sprintId)` recomputes the canonical
count from the tasks collection (with the BUG-032 filter shape
`deletedStatusKey β {0, undefined}`).
`reconcileSprintTaskCount(...)` writes the recomputed value back.
`scheduleReconciliation(...)` runs it off the event loop via
`setImmediate` so callers don't have to chain another promise.
Helper is idempotent and best-effort: invalid input β null, errors
are logged but never rethrown.
- **Modules/tasks/helpers/mongo_helper.js** β
Wire `scheduleReconciliation` into every `updateSprintFun(...).catch`
in `moveTaskFunction`, `convertToListSubTask`, and the merge-task
flow, plus the outer `findOneAndUpdate` catch in `moveTaskFunction`
(where the source task is already soft-deleted before the destination
insert fails). On failure, the count is recomputed from source-of-
truth instead of being left drifted.
The audit's framing was slightly off β `HandleTask` itself only does a
save + history hooks, not the count updates. The actual drift risk
sits in the move/convert/merge flows downstream, which this commit
addresses.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(BUG-034): rotate Winston log files
`Config/loggerConfig.js` wired three plain `transports.File` instances,
so `log/track.log`, `log/error.log`, and `log/combined.log` grew without
bound. On long-running deployments this eventually fills the disk and
crashes the process.
Replace each with a `winston-daily-rotate-file` transport that:
* Splits files by date (legacy basename preserved as a prefix:
`track-YYYY-MM-DD.log`, `error-β¦`, `combined-β¦`), so any existing
tail/grep / log-aggregator wiring keeps working.
* Caps each file at 20MB (`maxSize`) β env-tunable via LOG_MAX_SIZE.
* Keeps the last 14 days of rotated files (`maxFiles`) β env-tunable
via LOG_MAX_FILES.
* Gzips rotated files (`zippedArchive: true`) so old logs are cheap to
retain.
Also adds `winston-daily-rotate-file ^5.0.0` to dependencies.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(BUG-035): pin cron jobs to a known timezone
`schedule.scheduleJob('0 0 * * *', β¦)` resolves the schedule in the
server's local timezone, so DST transitions or a container-host tz
change silently move the wall-clock firing time. Daily-midnight jobs
suddenly fire at 23:00 or 01:00 with no log trail.
Switch every `scheduleJob` call to the `{ rule, tz }` object form. The
tz defaults to UTC (the only zone that's stable everywhere), but
operators can override via the `CRON_TZ` env var when they need
calendar-day cuts in a specific region for accounting/billing.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(BUG-036): path-traversal-safe loader for SERVICE_FILE
`Modules/checkinstallstep/controller.js` did
require("../" + process.env.SERVICE_FILE)
and `Config/firebaseConfig.js` did
require(path.resolve(__dirname, "..", config.SERVICE_FILE)).
Either lets an attacker (or a careless installer-wizard input) load
arbitrary JS/JSON inside the repo by stuffing `../`-style paths into
`SERVICE_FILE`. A `.js` file under the repo root would even be
executed with full process privileges.
Add `utils/safeServiceFile.js` exposing `resolveServiceFile()` which:
* rejects absolute paths,
* resolves the supplied path against the project root and verifies
the result is still inside it (no `..` escape),
* requires a `.json` extension (the Firebase service-account file
is always JSON; this blocks the arbitrary-`.js` execution path),
* verifies the file exists.
Wire the helper into both call sites so the same allow-list applies
whether SERVICE_FILE arrives via env at boot (`firebaseConfig.js`) or
during the installation wizard (`checkinstallstep/controller.js`).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(BUG-037): tighten global bodyParser limit
Every endpoint accepted a 50MB JSON / url-encoded / raw body via
the global `bodyParser` middlewares. Any unauthenticated POST could
force the server to buffer the full 50MB before validation ran β
trivial memory DoS.
Drop the global default to 2MB (well above typical JSON payloads:
comments, settings, project data β but blocks the multi-MB DoS).
The body-parser cap is independent of multer, so file-upload routes
keep working under their own multer limits.
Operators with bulk-import use cases that legitimately need bigger
JSON can raise the cap via the `BODY_LIMIT` env var without code
changes.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(BUG-038): drop S3 request handler timeout from 5 min to 30 s
`Config/config.js` built `requestHandler` with both `connectionTimeout`
and `socketTimeout` set to 300_000 ms (5 minutes). A hung Wasabi call
pinned the request worker for the full five minutes β a brief upstream
outage was enough to saturate the worker pool and degrade unrelated
routes.
Drop both defaults to 30_000 ms (30 s). That covers normal multi-MB
uploads on slow links but gives up quickly on truly stuck connections.
Env-tunable via `S3_CONNECTION_TIMEOUT_MS` / `S3_SOCKET_TIMEOUT_MS`
for operators who need a different ceiling (e.g. very large file
uploads from poorly-connected mobile clients).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(BUG-039): server-side identity verification for social login
The frontend completes the OAuth dance client-side and POSTs
`{email, googleId}` or `{email, githubId}` to `/loginAuth`. The
backend trusted those values blind, so anyone who knew a victim's
email plus their numeric provider id could authenticate as them. The
audit's framing ("missing state parameter") was off for this
architecture β there is no backend-initiated OAuth redirect to attach
state to β but the underlying threat is real.
Add server-side verification gated behind env config so existing
deployments don't break on upgrade.
GitHub:
* `verifyGithubAccessToken(accessToken)` round-trips
`GET https://api.github.com/user` (HTTPS, 8 s timeout, structured
User-Agent + Accept headers).
* `verifyGithubAuth` rejects login unless GitHub's response carries
the same id and email as the client-supplied claim.
* Strict by default when an accessToken is present. Operators on
legacy clients can keep the old behaviour by setting
`GITHUB_OAUTH_REQUIRED=false` while they ship a frontend update.
Google:
* `verifyGoogleIdToken(idToken)` uses google-auth-library (already
a transitive dep) to validate signature, audience, and expiry,
then pulls the canonical `sub` and `email` from the verified
payload.
* `verifyGoogleAuth` rejects login on sub or email mismatch.
* Gated on `GOOGLE_OAUTH_CLIENT_ID` (the audience). If unset we log a
one-line warning and fall through to the legacy path so single-host
upgrades stay safe; `GOOGLE_OAUTH_REQUIRED=false` keeps the
legacy id-only behaviour when the client-id IS set but the client
hasn't been updated yet.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(BUG-040): drop deprecated `atob` / `btoa` npm shims
Both packages have been runtime globals in Node since v16, so the npm
shims (`atob 2.1.2`, `btoa 1.2.1`) are dead weight in the lockfile and
haven't been maintained in years.
Replace the two call sites with a one-line `Buffer.from(...)`
expression that round-trips identically to what the shims did:
* Modules/auth/controller/verifyInvitation.js β decode base64 β
binary string for the invitation-blob parser.
* Modules/auth/controller/sendInvitation.js β encode binary β
base64 for the same blob construction.
`npm uninstall atob btoa` removes both packages from
package.json/package-lock.json.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(BUG-041): drop aws-sdk v2; consolidate on @aws-sdk/* v3
Both `aws-sdk` (v2, maintenance-only) and `@aws-sdk/*` (v3) were
installed and used in the same codebase. The only live consumer was
SES email sending in `Modules/servicewithAWS.js` (via
`Config/aws.js`'s exported `ses` client). The other v2 clients we
exported (`sesv2`, `ssmClient`, `s3`, `sesWithAttachment`) had zero
call sites β dead weight.
Migrate to @aws-sdk/client-ses (v3) and remove `aws-sdk` from
package.json.
Config/aws.js:
* Build the SES client via `new SESClient({...})` from
`@aws-sdk/client-ses`.
* Expose `awsRef.ses` as a v2-shaped shim: `sendEmail(params, callback)`
internally invokes `client.send(new SendEmailCommand(params))` and
fires the callback with `(err, data)` so the single existing caller
in servicewithAWS.js needs no rewrite.
* Also expose the raw v3 client as `awsRef.sesClient` for any future
code that wants to use Commands directly.
* Drop the unused exports (sesv2, ssmClient, s3, sesWithAttachment).
Modules/servicewithAWS.js:
* Remove the duplicate `aws-sdk` import and `AWS.config.update()` call
(v2-only).
* Switch the nodemailer `SES` transport to the v3-compatible
`{ SES: { ses, aws: require('@aws-sdk/client-ses') } }` shape for
the `sendAttachMail` path.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(BUG-042): drop `moment` from backend; standardise on luxon
CLAUDE.md says the project standardised on Luxon. The backend still
imported `moment` in five files for trivial date formatting; this PR
removes those usages and drops `moment` from the root package.json.
Frontend `moment` usage (hundreds of call sites across Timesheet /
TimeLog / composables) is NOT touched here β that's a much larger
migration scoped separately. The frontend's own `package.json` keeps
its `moment` entry so the Vue build doesn't break.
New helper:
* `utils/dateHelpers.js` exposes
- `formatDate(input, fmt)` β accepts JS Date, millis, ISO string,
or `{seconds}` shape, and translates moment-style format tokens
(YYYY/MM/DD/HH/mm/ss/A/MMM/ddd) to luxon-style under the hood
so existing callers don't have to change their format strings.
- `formatNotificationDate(input)` β exact replacement for the two
`moment.calendar()` sites in `Modules/notification/sendEmail/`,
which both used the same format string for every branch (so the
relative-time wrapper was a no-op). Preserves the original
output, including the pre-existing `HH:MM` token quirk.
Migrations:
* `Modules/logTime/controllerV2.js` β single `.format("YYYY-MM-DD")`
call β `formatDate(date, 'yyyy-LL-dd')`.
* `Modules/notification/sendEmail/controller.js` and
`Modules/notification/sendEmail/controllerV2.js` β
`moment().calendar(...)` β `formatNotificationDate(...)`.
* `Modules/tasks/helpers/helper.js` and
`Modules/tasks/helpers/mongo_helper.js` β `changeDateFormat`
delegates to `formatDate` (which keeps the moment-style API its
callers pass).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(BUG-043): wrap clickable <span>/<img> affordances in real <button>
Two affordances dispatched click handlers on non-semantic elements
(no role, no aria-label, no keyboard focus, no screen-reader
announcement):
* `frontend/src/components/atom/Modal/Modal.vue` β the modal close
icon was a bare `<img @click="closeModal()">`. Wrap in
`<button type="button" :aria-label="$t('Projects.close') || 'Close'">`
with the icon inside as a presentational `<img alt="">`.
* `frontend/src/components/atom/Attachments/Attachments.vue` β the
"Download All" affordance was a `<span @click="downloadAllImages()">`.
Replace with `<button type="button" class="download-all-btn ...">`.
Both components' stylesheets gain a tiny rule to strip native button
chrome (so the visual result matches the old elements) but preserve
`:focus-visible` so keyboard users see the focused state.
Note: a couple of nearby affordances (Attachments' "See All" `<div>`
and the help-icon popover) have the same anti-pattern but are out of
scope here β the audit specifically flagged the `<span>` and `<img>`
cases above.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(BUG-044): give Modal role=dialog, aria-modal, and focus trap
The Modal atom rendered a bare `<div class="modal">` β no role,
no aria-modal, no aria-labelledby. Screen readers couldn't announce
the modal as a dialog and focus could leave the modal via Tab and
reach background controls.
Changes to `frontend/src/components/atom/Modal/Modal.vue`:
* Template:
- Root `<div class="modal">` gains `role="dialog"`,
`aria-modal="true"`, `:aria-labelledby="titleId"`, and
`tabindex="-1"` (so focus can land on the dialog itself when no
children are focusable).
- The title `<span>` now carries the `:id="titleId"` that
aria-labelledby points at.
- `@keydown.tab="handleTabKeydown"` traps Tab; `@keydown.esc.stop`
closes the modal on Escape.
* Script:
- `modalRef` template ref + `titleId` computed.
- `handleTabKeydown` cycles focus between the first and last
focusable descendants (queries the standard focusable selector
list, skipping aria-hidden and offscreen elements).
- `activateFocusTrap` captures the previously-focused element and
moves focus to the first focusable inside the modal (or the
dialog itself if there are none). Called on mount-if-open and
when `modelValue` flips to true.
- `deactivateFocusTrap` restores focus to the captured element on
close / unmount.
No new dependency: the trap is ~40 lines of inline logic; we don't
need `focus-trap` for one component.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(BUG-045): wire up the jest test suite
`npm test` was a placeholder (`echo "Error: no test specified" && exit 1`)
even though jest was already installed as a devDependency. This PR
wires it up as a working bootstrap suite that future fixes can grow
into.
Changes:
* `package.json` β
- `test` now runs `jest`.
- new `test:watch` (development) and `test:naming` (the pre-existing
structural-audit test, kept off the default run because it flags
legacy naming inconsistencies tracked separately).
* `jest.config.js` β points jest at `tests/`, ignores
node_modules/frontend/installation/time-tracker-app/.claude and the
naming-conventions audit, and runs in `node` environment.
* `tests/smoke.test.js` β minimal guarantee the harness is alive
(always runs).
* `tests/utils/dateHelpers.test.js`, `tests/utils/safeServiceFile.test.js`,
`tests/utils/imageGuard.test.js` β behaviour tests for the helpers
introduced earlier in this audit (BUG-042, BUG-036, BUG-023). Each
suite is wrapped in a graceful skip when its target module isn't on
the current branch, so this PR lands cleanly today and the tests
light up automatically once the corresponding helper PRs merge to
staging.
`npm test` exits 0 β 3 passed (smoke) + 16 skipped (helpers pending
merge). After BUG-023/036/042 merge, all 19 should pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(BUG-046): let Mongoose manage history timestamps
`Modules/tasks/helpers/{helper,mongo_helper}.js`'s `HandleHistory`
wrote `createdAt` / `updatedAt` manually using `DateTime.utc().ts`,
which is a numeric millisecond value β not a BSON Date. Existing
documents ended up with inconsistent shapes (number vs Date) depending
on which code path created them.
Every other schema in `utils/mongo-handler/createSchema.js` is built
with `{ timestamps: true }` so Mongoose owns the timestamps. The
history schema alone was missing that option.
Changes:
* `utils/mongo-handler/createSchema.js` β add
`{ strict: false, timestamps: true }` to `historySchema`.
* `Modules/tasks/helpers/mongo_helper.js` (HandleHistory) and
`Modules/tasks/helpers/helper.js` (HandleHistory) β drop the manual
`createdAt: utcDateTime.ts, updatedAt: utcDateTime.ts` lines.
Mongoose populates both as BSON Dates on `.save()`.
* `utils/mongo-handler/schema.js` β keep `createdAt`/`updatedAt`
declared on the history shape so callers that read them still work,
but drop `required: true`. Mongoose sets them on `.save()`; making
them required on updates would reject legacy docs that never had
them.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(BUG-047): ARIA semantics + keyboard nav for CustomDropDown
`frontend/src/components/molecules/DropDown/CustomDropDown.vue` is the
shared custom dropdown atom used across the app. Pre-fix:
* The trigger `<div @click="buttonClick()">` had no role,
no aria-haspopup, no aria-expanded, no tabindex β keyboard users
couldn't reach or open it.
* The floating panel had no role="listbox" β screen readers couldn't
identify it as a menu.
* The mobile-view close affordance was a bare clickable `<img>`.
Changes:
* Trigger gains role="button", tabindex="0", :aria-haspopup="'listbox'",
:aria-expanded (bound to open state), :aria-controls (panel id).
* Trigger handles keyboard:
- Enter / Space β open/close (mirrors click).
- Escape β close.
- ArrowDown β open AND move focus to the first focusable child
inside the options slot. (We can't enforce role="option" on
user-provided slot content, but moving focus there gets keyboard
users navigating immediately.)
* Floating panel gains role="listbox" and an @keydown.esc handler.
* Mobile close `<img>` wrapped in a real `<button type="button">`
with aria-label="Close" and a style rule (`.dropdown-close-btn`)
that strips native chrome but preserves :focus-visible.
* New helpers: `closeDropdown()` and `openAndFocusFirstOption()`.
Note: the sibling `DropDown.vue` and `MobileDropDown.vue` follow the
same anti-pattern but were not flagged by the audit; they'd benefit
from a similar sweep in a follow-up.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Done - Test Case
Done - Test Case
* Add Gitigonre
Add Gitigonre
* Rename the folder Auth
Rename the folder Auth
* Rename Folder
Rename Folder
* Chnage the name
Chnage the name
* Folder Rename
Folder Rename
* remove dist
remove dist
* Create yml
Create yml
* Update main.yml
* Update main.yml
* Update main.yml
* Basic Setup
Basic Setup
* fix: repair broken advisory URL in SECURITY.md (#44)
The advisory link was split across two lines and wrapped in a code span,
rendering as malformed text rather than a clickable link. Joined the URL
and used proper markdown link syntax. Pairs with enabling Private
Vulnerability Reporting in repo settings so external reporters can
actually submit advisories.
Closes #44
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: refresh correct lastRequest field on connection reuse (#42)
updateConnectionRecord was writing to lastRequset (typo) while the
idle-cleanup loop in startInterval reads lastRequest. Active company
databases therefore kept the timestamp frozen at createdAt and were
eligible for termination after the 30-minute window despite continuous
use. Fixed the property name so the update path and the cleanup path
reference the same field.
Closes #42
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: remove duplicate CheckInstallStep mount and .env hot-reload (#41)
Two related sources of duplicate-handler accumulation in the bootstrap:
1. CheckInstallStep was mounted twice: once inside
initializeControllers() and once unconditionally at the top level.
The top-level mount has to stay because the install wizard must be
reachable before MONGODB_URL is configured. Removed the inner mount
so each startup registers it exactly once.
2. fs.watchFile on .env called initializeControllers() on every save,
which re-ran .init(app) for ~60 route modules onto the same Express
app instance (Express has no clean route un-registration) and spun
another setInterval inside startInterval(). Removed the watcher;
nodemon already restarts on file changes in dev, and production
env changes require a process restart anyway.
After this change initializeControllers() is only invoked once, from
the MONGODB_URL startup gate.
Closes #41
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: preserve dataType and options across JWT refresh retry (#38)
apiRequest and apiRequestWithoutCompnay reconstruct the call after a
token refresh using only (type, endPoint, data), dropping dataType
and options. Any retry of a multipart upload then runs through the
JSON axios instance instead of the form-data instance, and any
caller-supplied abort signal or per-request option is lost. Forwarded
dataType and options into both retry calls so the replay matches the
original.
Closes #38
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: clean up close_click IPC listener after each notification (#39)
sendNotification() registered a new ipcMain.on('close_click', ...)
handler on every screenshot capture and never removed it, so the
process accumulated one global IPC listener per screenshot and a
single close click eventually fired N stale handlers, each trying to
close an already-destroyed BrowserWindow reference.
Switched the registration to ipcMain.once so the happy path
(user clicks close) auto-removes the listener. For the auto-timeout
path (window closes after 10s without a click) the handler stays
registered and would later steal a future notification's close
event, so also remove it explicitly when the window emits 'closed'.
The timeout id is now tracked and cleared on the same hook so the
timer can't fire against a destroyed window. The window reference
is captured in a local so each listener targets its own window even
if a subsequent sendNotification() reassigns the module-level
screenshotNotificationWindow before this listener fires.
Closes #39
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat: allow editing the "Created by" user on a task
The task sidebar Details panel previously showed Created by (the
Task_Leader field) as a read-only user β set once at creation and
never changeable. Add an inline picker mirroring the existing
Assignee field so the creator can be reassigned.
Changes:
- Backend: new updateTaskLeader action on task_class_Mongo. Validates
the new Task_Leader, writes via $set, emits the same
socketEmitter.emit('update', { module: 'task', updatedFields }) used
by updateAssignee, and logs a TaskLeader_Changed history entry via
HandleHistory. Includes the isUpdateTask:false side-effects-only
branch for parity with updateStatus / updatePriority.
- Frontend store (TaskOperations): new updateTaskLeader action that
optimistically commits the new Task_Leader into the projectData
Vuex store, then PATCHes /api/v2/tasks with action 'updateTaskLeader'.
- TaskDetailRightSide.vue: replaced the read-only Created-by block
with an Assignee picker (single-select) for users holding the
task.task_assignee + task.task_list permissions, preserving the
original read-only display as the fallback for users without those
permissions. Wired @selected to a new updateTaskLeader() handler
with the same toast / error pattern as updateAssignee.
- i18n: added Toast.Created_by_updated_successfully and
Toast.Created_by_not_updated to the English locale. The other 10
locales fall back to English via vue-i18n until translators
backfill β flagged as a follow-up.
Permission policy: reuses task.task_assignee. Anyone allowed to
change the Assignee can also change Created by. No new permission
key, no role-permission migration. Backend has no per-field gate,
consistent with the existing Assignee / Status / Priority update
paths which all trust the frontend permission gate.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat: split header notifications into Unread and Archive views
Previously the notification bell fetched every notification the user
had ever received (read or unread) in a single combined list, making
it hard to focus on actionable items. This change splits the dropdown
into two views β Unread (default on every open) and Archive β gated
by a single toggle button in the dropdown head.
Changes:
- Backend: GET /api/v1/app-notification/notification accepts a new
optional `filter` query param ('unread' | 'archived'). 'unread' adds
`notSeen: { $in: [userId] }` to the aggregation match; 'archived'
adds `notSeen: { $nin: [userId] }`. Default is 'unread' to keep the
bell focused on actionable items. Refactored the match clauses into
a `baseMatch` array so the filter is appended cleanly; query shape
and pagination are otherwise unchanged.
- Header.vue: added a `notificationFilter` ref ('unread' by default),
a "View Archive" / "View Unread" toggle in the dropdown head, a
`switchNotificationFilter()` handler that resets paging state and
refetches, and an `openNotificationsDropdown()` helper that the
bell click handlers now use so each fresh dropdown open lands in
Unread. `markRead()` and `markAllRead()` clear the read items from
the local list while the Unread filter is active so the dropdown
reflects the filter without an extra refetch. The "Mark all as
read" button visibility now keys off `notifications.length` (the
list is already filtered to unread on this view) rather than the
server-side `totalNotification` counter, which can lag and falsely
hide the button.
- i18n: added Header.View_Archive / Header.View_Unread to the English
locale; other locales fall back to English via vue-i18n until
translators backfill (flagged as a follow-up).
Out of scope (deferred per user instruction): goals 3 and 4 from the
original spec β creator-prefs fire policy verification and email
preference parity audit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: honour project-level Ignore for task creator and company owner
Closes goals 3 & 4 of the notification refactor. Two parallel bypasses
in handleNotification.HandleBothNotification let task creators and
company owners through the project-level watcher filter (the Ignore /
All Activity / Participating setting in the List of Watcher panel):
- Task branch (taskId path): Task_Leader was Set-union'd into the
recipient list AFTER `projectData.watchers` had already filtered out
any user set to "ignore". So a creator who had set the project to
Ignore still ended up in `assigneeUsers`, and the downstream
per-event preference check (NOTIFICATIONS_SETTINGS β a separate
preference layer that defaults email=true) emitted both an in-app
and an email notification. Now Task_Leader is included only if their
project-watcher setting is not "ignore".
- Project branch (type === 'project'): same pattern β companyOwnerId
was union'd in unconditionally, bypassing the watcher filter.
Honours "ignore" too.
This explains the goal-4 symptom of "I'm getting emails for events I
disabled": the disablement was set at the project-watcher layer, but
the bypass routed the recipient straight to the per-event layer where
email was still on.
Other recipient paths are untouched. Default behaviour for users who
have not chosen "ignore" is unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat: per-company screenshot retention policy + nightly cleanup cron
Adds an opt-in retention policy on the time-tracker screenshot pipeline.
Visible on /settings/setting to the company owner (roleType === 1)
only. When enabled, a nightly cron permanently deletes trackshots
older than the configured window (3 / 6 / 12 / 24 months) from both
the per-tenant TimeSheet.trackShots subdoc array and from Wasabi
(main object + every thumbnail variant).
Backend
=======
- utils/mongo-handler/schema.js: new `screenshotRetention` Map field
on the global companies schema. Stores `enabled`, `maxAgeMonths`,
`enabledAt`, `enabledBy`, `lastRunAt`, `lastRunStats`,
`firstRunCompletedAt`, `runningSince`. Backward-compatible β legacy
companies without the field read as `enabled: false`.
- Modules/ScreenshotRetention/: new module.
helper.js β policy read/write, preview counter, per-company
cleanup workflow, and the cron entry point.
Production-ready guarantees:
* Wasabi delete BEFORE db $pull so transient
Wasabi failures leave the db record intact and
the next nightly run retries (no permanent
orphans).
* Per-trackshot main + 4 thumbnail keys deleted
(sizes hard-coded from thumbnail.json).
* Filters by trackshot.screenShotTime (epoch ms),
not parent TimeSheet timestamp.
* Advisory `runningSince` lock prevents
double-runs; stale locks (>4h) are reclaimed.
* First-run safety cap (50k deletions) for the
initial cleanup on legacy data; lifted once
`firstRunCompletedAt` is stamped.
* Bounded company concurrency (5 in parallel)
via Promise.allSettled.
controller.js β three endpoints with owner role check:
GET /api/v1/screenshot-retention
GET /api/v1/screenshot-retention/preview
PUT /api/v1/screenshot-retention
Owner check looks up the per-tenant
`company_users` doc for the caller and confirms
`roleType === 1`. Returns 403 on mismatch.
routes.js β endpoint registration.
init.js β module bootstrap (matches existing convention).
- index.js: register the new module beside the rest of
`initializeControllers()`.
- cron.js: removed the broken `cleanUpTrackshot()` call (was
referencing an unimported binding and never executed). Added a new
schedule at 00:30 UTC that invokes
`screenshotRetention.runRetentionForAllCompanies()`. Ofβ¦
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Reconnects the branch histories severed by squash promotions so the v14.2.0 promotion PR (#222) resolves cleanly. Conflict resolution: release-please artifacts (package.json version, CHANGELOG, .release-please-manifest.json) taken from main; Modules/Tasks code (task relations feature) kept from staging. Full test suite 45/45. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the βοΈ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
β¨ Finishing Touchesπ§ͺ Generate unit tests (beta)
Comment |
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Back-merges
mainintostagingto resolve the conflicts blocking the v14.2.0 promotion PR #222.Why this is needed
Promotions to
mainare squash-merged (linear-history requirement), which severs the shared history between the branches. Since the last promotion, release-please committed version bumps that exist only on main (package.json14.1.0,CHANGELOG.md,.release-please-manifest.json), whileModules/Tasksevolved only on staging (task relations, #221). With an ancient merge-base, Git flags all five files as conflicts on #222.This PR carries one merge commit that reconnects the histories and resolves every conflict:
package.json(version),CHANGELOG.md,.release-please-manifest.json,package-lock.jsonModules/Tasks/routes.js,Modules/Tasks/helpers/task_class_Mongo.jsVerified after resolution: version is 14.1.0, the relations feature is intact, and the full test suite passes 45/45.
Merge this PR with "Create a merge commit" β NOT squash. Squashing would flatten the merge commit and discard the reconnected parentage, leaving #222 conflicted. (This is also why #222 conflicts existed in the first place.)
After this merges
π€ Generated with Claude Code