Conversation
* 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()`. Off-peak vs the
other midnight jobs so a heavy cleanup doesn't compound with the
bucket-size + AI-reset jobs on the same minute.
Frontend
========
- frontend/src/components/molecules/Setting/SettingScreenshotRetention.vue:
new card mounted on /settings/setting. Renders only for
`companyUser.roleType === 1`. Toggle + retention-window dropdown +
last-run telemetry. Enabling fires a SweetAlert confirmation that
shows the preview-count from the new GET preview endpoint so the
owner knows exactly what will be deleted on the next nightly run.
- frontend/src/views/Settings/Setting/Setting.vue: mount the new
component in the existing right-hand column. Self-hides for
non-owners.
- frontend/src/locales/en.js: new ScreenshotRetention.* keys (heading,
toggle/window labels, confirmation copy, last-run telemetry). Other
locales fall back to English via vue-i18n.
Out of scope (deferred)
=======================
- Translation backfill for non-English locales.
- A `RetentionAuditLog` collection for per-run history beyond
`lastRunStats`. The cron emits structured logs in the meantime.
- A "dry run" mode that lets the owner see what would be deleted
without enabling the policy. The preview endpoint already gives the
count; a per-record preview would need its own UI.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: comment out unused companyId injection in SettingScreenshotRetention component
* feat(setup): one-command developer setup via `npm run setup`
Adds an additive setup orchestrator that installs deps, builds the wizard,
bootstraps .env, starts backend + frontend, completes the installation wizard
headlessly, creates a default admin account, and opens the login page — all
from a single command. Non-technical contributors can go from `git clone` to
a working login screen with no manual steps.
New
- scripts/dev.js — orchestrator with HTTP-based wizard auto-completion,
MongoDB probe with retry, defensive .env patching, credentials banner.
- nodemon.json — explicit watch list (server-side dirs / .js only) so wizard
writes to installationSteps.json no longer restart the backend mid-request
(root cause of the "wizard reloads on MongoDB step" bug).
- package.json — `setup`, `dev`, `setup:reset` scripts + nodemon devDep.
Wizard improvements (back-compat preserving)
- Modules/CheckInstallStep/controller.js: `isDoItLater` support added for
Firebase (step 4) and SMTP (step 6), mirroring the existing AI (step 5)
skip pattern. Both steps remain mandatory unless the caller opts in.
- Defensive APIURL fallback at module-load (was crashing the backend on
fresh clones if .env hadn't loaded yet — TypeError on .substring of
undefined).
.env.example
- SERVICE_FILE corrected from "../firebase-adminsdk.json" to
"./firebase-adminsdk.json" (the prior default tripped the BUG-036
path-traversal guard and blocked the Firebase wizard step).
- Quick-start comment block at the top.
Nothing existing changes
- `npm start`, `npm run nodemon`, `npm run basic-install`, the interactive
wizard, and the documented manual setup paths are all untouched.
- New scripts never run automatically; users must invoke them explicitly.
- Fallback chain at every failure point (MongoDB unreachable / auto-setup
error / --manual flag) opens the interactive wizard UI so the user is
never left in an unrecoverable state.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(screenshot-retention): security + correctness pass on PR #157
Independent review of the original commit surfaced three blockers and
several correctness issues. This is a follow-up commit on the same
branch that addresses all of them.
BLOCKERS
========
1. **Routes had no JWT middleware.** Config/setMiddleware.js gates auth
via path allowlists. The new `/api/v1/screenshot-retention` and
`/api/v1/screenshot-retention/preview` paths were NOT listed in
either `verifyJWTTokenWithCRoute` or `verifyJWTToken`, so the
endpoints shipped completely unauthenticated — anyone on the
internet could toggle retention for any company. Added both paths
to `verifyJWTTokenWithCRoute` so the JWT middleware fires and
populates `req.uid` / `req.aud`.
2. **Thumbnail keys were generated in the wrong dimension order.** The
upload-time call at Modules/storage/wasabi/controller.js:298 passes
`(thu.height, thu.width)` into a function whose params are
`(width, height)`, so the stored filename is effectively
`<base>-<height>x<width>.<ext>`. The cleanup helper was building
`<base>-<width>x<height>.<ext>` and missing every thumbnail variant
on every retention run — leaving four orphan Wasabi objects per
trackshot, permanently. derivThumbnailKeys now mirrors the
upload-time swap with a comment explaining the pre-existing bug
in the upload path (out of scope to fix here).
3. **Cursor-based pagination replaces skip-based scan.** The previous
loop advanced `pagedSkip += docs.length` after each batch, but
the `$pull` inside the loop removes docs from the match set —
meaning the next page's skip lands past where it should. Most of
the DB was being missed on legacy-data tenants. Replaced with
`_id > lastSeenId` cursor that's immune to in-flight match-set
shrinkage.
SECURITY
========
4. **Controller now reads userId from `req.uid`** (set by
Config/jwt.js#checkToken from the verified token's `uid` claim),
not from `req.body.userId`. The old code let a non-owner pass the
owner's userId in the body and pass the role check. Status codes
updated: missing userId → 401 (authentication required) rather
than 400.
HIGH
====
5. **Legacy string-typed `screenShotTime` is now handled.** Multipart
form uploads coerce numbers to strings at write time, so the
`screenShotTime` field on legacy trackshots is stored as a string.
The previous `$lt: cutoffMs` (number) didn't match strings.
- `countOldTrackshots` now uses `$convert` with
`to: 'long', onError: null` inside the $filter so the comparison
works regardless of the field's actual storage type.
- `runRetentionForCompany`'s loose DB query + strict in-memory
filter coerces via `Number(t.screenShotTime)` and skips
non-finite values.
6. **`firstRunCompletedAt` is now stamped only when a real cleanup
completed.** Previous condition `deletedCount < cap` stamped the
marker on a zero-deletion run (e.g. a new company with no eligible
data), which lifted the first-run safety cap before any real bulk
data accumulated. New condition: scan exhausted (no cap hit, no
error) AND deletedCount > 0.
7. **Empty-image trackshots no longer inflate `deletedCount`.** The
scan loop now pre-filters them out (`oldShots` requires `t.image`),
and `deleteTrackshotObjects` returns `skipped: true` rather than
`mainDeleted: true` if it's ever called with a missing key.
Stats now track `skippedCount` separately.
MEDIUM
======
8. **Removed the no-op `updateCompanyPolicy(companyId, {}, {})`
call** that hit the empty-patch early-return at line 176 without
writing anything. Lock acquisition is a single `stampMasterField`
call, which is at least document-atomic. Cluster-aware leader
election is still out of scope (documented).
9. **`lastRunStats` now records partial-run errors and cap-hit state.**
New stats fields: `skippedCount`, `hitCap`, `error`. Ops can tell
from the master doc whether the run completed cleanly, hit the
cap, or threw mid-loop.
10. **Frontend now uses `watch(isOwner, …, {immediate: true})`** instead
of one-shot `onMounted`. The Vuex `companyUserDetail` store
sometimes hydrates after the component mounts on soft route
changes; the one-shot mount would never call `loadPolicy()` in
that case and the card stayed on hard-coded defaults forever.
LOW
===
11. **`extractKey` regex tightened** from `[a-f0-9]{20,}` to
`[a-f0-9]{24}` (exact ObjectId length) and now decodes URL
path segments so percent-encoded keys map back to their
canonical storage form.
12. **`getS3Client` doc + body cleaned up.** `s3Client` is not exported
from `wasabi/controller.js`, so the old "fall back if not
available" branch was always taken. Removed the misleading
primary path; we just build the client from `awsRef` and
memoise it.
Test plan addendum
==================
- Hit `GET /api/v1/screenshot-retention` without a JWT → 401.
- Hit `PUT /api/v1/screenshot-retention` with a forged body
`userId` of the owner from a non-owner JWT → 403 (controller
uses `req.uid`, not body).
- On a tenant with legacy string-typed `screenShotTime`:
- GET /preview returns a non-zero count for the configured window.
- A nightly run deletes those records + their (height×width-named)
thumbnail variants.
- Confirm thumbnail keys deleted in Wasabi by listing the bucket
after a run — there should be no orphans for the deleted main
keys.
- Confirm pagination handles a tenant whose every TimeSheet has at
least one expired trackshot (all docs drop from match set on
first $pull): the cursor variant should still terminate cleanly
and process every doc.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(security): P0 + partial P1 hardening, CI pipeline, multer v2
Closes the critical security and dependency gaps surfaced in the
codebase audit. All 20 unit tests pass; no behavior change for normal
request paths.
Backend security
- Sanitize regex search inputs across 7 controllers (AI,
AdvancedGlobalFilter, Comments, MediaFiles, Project filter,
notification-count, trackerDownload) via new utils/escapeRegex.js.
Blocks NoSQL regex injection / ReDoS / cross-tenant name enumeration.
- Add helmet + global express-rate-limit in index.js. CSP disabled to
preserve the Vue inline-script setup; secure / sameSite already
env-gated correctly. trust-proxy set to 'loopback' to silence the
ERR_ERL_UNEXPECTED_X_FORWARDED_FOR warning behind a same-host
reverse proxy.
- Harden all 4 multer call sites via new utils/uploadConfig.js
(DEFAULT_LIMITS, safeFileFilter blocking executable extensions,
safeRelativePath for traversal protection). bucket.helper.js
storage destination now validates path before writing.
- New requireCompanyAud middleware (Config/jwt.js) enforces the JWT
`aud` claim against any companyId in body / params / query /
header on the ~50 verifyJWTTokenV2-only routes. Non-ObjectId
values (e.g. USER_PROFILES global bucket) pass through.
Dependencies
- Remove aws-sdk v2 from package.json (EOL Sept 2025); only @aws-sdk
v3 was actually imported.
- Bump multer 1.4.5-lts.1 -> 2.1.1 to clear known 1.x CVEs.
Schema strictness (P1-SEC-11)
- utils/mongo-handler/createSchema.js: flipped 7 core schemas to
`strict: true` (tasks, comments, timesheet, history, adminDetail,
subscriptionPlan, globalCustomFields). Kept `strict: false` on 7
intentionally-dynamic schemas (notification counters, Chargebee
webhook mirrors, custom-field definitions, plan-feature maps) with
inline comments explaining why.
CI / DevOps
- .github/workflows/main.yml: added a `validate` job (npm test, npm
audit advisory, frontend install + build) that the deploy job now
`needs:`. Previously deploys ran with zero validation.
Diagnostics
- modules/storage/wasabi/controller.js: replaced 6 generic
`Error while upload file: ${error}` rejects with a
formatS3UploadError helper that logs the AWS error Code, HTTP
status, bucket, key, and requestId.
Documented for follow-up
- Auth/controller.js: TODO comments at both res.cookie sites
explaining why httpOnly stays false until the frontend stops
reading the cookie via js-cookie (P1-SEC-09 deferred).
Pre-existing fixes pulled in
- tests/utils/imageGuard.test.js: corrected env var names so the
suite goes from 19/20 to 20/20 passing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(safeServiceFile): reject Windows absolute paths on any host
The CI pipeline added in the same PR is the first thing to run
tests on Ubuntu. `path.isAbsolute('C:\Windows\...')` returns
false on Linux, so the safeServiceFile absolute-path guard fell
through and the request hit the `.json` extension check instead.
The test in `tests/utils/safeServiceFile.test.js:46` expects the
"relative" error in both shapes and was passing on Windows hosts
by accident.
Use `path.win32.isAbsolute(...)` alongside the platform-bound
`path.isAbsolute(...)` so both POSIX (`/etc/hosts`) and Windows
(`C:\Windows\...`) absolute paths are rejected regardless of
which OS the process runs on. This is also a defensive improvement
to BUG-036: a Linux deployment can no longer be coaxed into
re-resolving a Windows-shaped path inside the project root.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Update main.yml
* Update main.yml
* feat(ai-project-generator): one-shot AI project bootstrap with PDF brief + SSE progress
Adds a parallel "Create with AI" path to the existing manual project wizard.
The user describes their project in natural language (optionally attaches a
PDF/DOCX/MD/TXT brief and chooses public/private workspace + a target task
count), the LLM returns a plan (project metadata + folders → sprints →
tasks with rich descriptions), the user reviews/edits names, and on
"Create everything" the orchestrator commits the whole hierarchy via the
same write paths the manual flow uses — with SSE progress events and
rollback on partial failure.
== Backend ==
- New module Modules/AIProjectGenerator with:
* llmProvider/ — env-selected adapter (openai | anthropic). Reads
AI_API_KEY/AI_MODEL (existing) for OpenAI and ANTHROPIC_API_KEY/
ANTHROPIC_MODEL (new) for Anthropic, picked via LLM_PROVIDER.
* briefExtractor.js — multer v2 disk-buffer + pdf-parse + mammoth, 10MB
cap, mime allow-list, control-char strip, 100k char truncation.
* schemaValidator.js — zod PlanSchema with strict task/status/folder
rules; sanitizes member ids against the active company roster.
* promptTemplates.js — system + user + repair prompts. Forces 4-8
tasks per sprint, domain-specific status names, full lifecycle coverage.
* orchestrator.js — sequential project → folders → sprints → bulk tasks.
Reserves a taskKey range atomically, emits SSE per step, soft-rollbacks
in reverse on any failure (tasks → sprints → folders → project +
projectCount decrement).
* sseEmitter.js — heartbeat-equipped SSE channel keyed by random jobId.
* Endpoints: /api/v1/ai/project/{upload-brief,plan,clarify,execute}
+ unauthenticated /api/v1/ai-progress/:jobId (jobId is a bearer
capability — same pattern as /api/v1/generatePrompt/events).
- Config/setMiddleware.js: protected new auth paths via
verifyJWTTokenWithCRoute; SSE endpoint deliberately omitted.
- index.js: register the new module after Modules/AI.
- .env.example: LLM_PROVIDER, ANTHROPIC_API_KEY, ANTHROPIC_MODEL,
LLM_MAX_TOKENS_PLAN, LLM_MAX_TOKENS_CLARIFY.
- package.json: +@anthropic-ai/sdk, +pdf-parse, +mammoth, +zod.
== Frontend ==
- New AiProjectCreator.vue (organism) — 3-step sidebar:
1. Describe: textarea (20-char minimum), public/private workspace toggle
(mirrors manual ProjectWorkspace step), target-task-count slider,
PDF/DOCX/TXT/MD upload, inline clarification Q&A.
2. Review plan: project icon/code/description preview + inline-editable
names at every level (project / folder / sprint / task), expandable
task descriptions.
3. Execute: live SSE progress UI (project → folders → sprints → tasks)
with rollback-aware error state and "Open project" CTA on complete.
Close affordance is a close icon (was a Cancel button) — disabled
while uploading/loading; the sidebar can't be dismissed during the
execute phase to prevent orphaned in-flight jobs.
- New aiProjectGenerator.js composable wraps the four endpoints +
EventSource subscription.
- Projects.vue / ProjectListComponent.vue / ProjectListing.vue add the
"✨ Create with AI" button beside "+ New Project", gated on
currentCompany.planFeature.aiPermission.
- env.js: 5 new endpoint constants.
== Highlights from QA pass ==
- projectIcon is persisted in the canonical {type:'color', data:'#hex'}
shape Item.vue expects, so the AI-bootstrapped project's color/initial
pill renders in the sidebar (was an empty box).
- Multer rejection (LIMIT_FILE_SIZE, unsupported mime, etc.) is caught
before Express's default handler so users see "File is too large.
Maximum allowed size is 10 MB." instead of "Request failed with status
code 500".
- The user's explicit public/private choice in step 1 is forced onto the
plan server-side at /plan, /clarify and /execute — it always wins over
whatever the LLM emitted.
- Plan-feature quota (projectCount.*) is incremented like the manual
flow but the AI path intentionally skips checkProjectPlan's hard cap
so this feature remains usable on plans that limit project counts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* ci(main.yml): map VUE_APP_* secrets into the frontend build step
The CI frontend build was running with no .env present (gitignored, not
materialised on the runner) so every `process.env.VUE_APP_*` reference
was inlined as `undefined` in the bundle. Map the Firebase keys, storage
config, support-routing ids, and OAuth feature flags as repository
secrets in the Build frontend step so the validation build matches what
production actually ships.
Also bumps Node to 22, switches to npm ci, adds concurrency cancellation
for staging deploys, and tightens the workflow trigger list.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(env): update AI model to gpt-4o-mini in .env.example
* refactor(projects): split Projects.vue and Task.vue mega-components (#167)
Projects.vue: 2,737 -> 1,365 lines (50% reduction). Task.vue: 1,015 -> 540 lines (47% reduction).
- 9 composables extracted (useProjectCalendar, useProjectAvatar, useEmbedViews,
useProjectLifecycle, useProjectAssignee, useProjectTour, useProjectSearch,
useProjectRules, useProjectNameEdit)
- 5 sub-components extracted (ProjectActionsBar, ProjectFiltersToolbar,
ProjectSidebars, ProjectBottomModals, ProjectEmptyState)
- 7 view components (ListView, Comments, ActivityLog, WorkloadView, BoardView,
ProjectDetail, TableView, EmbedViewItem) converted to defineAsyncComponent
so each tab pulls its own chunk on demand
- 2 task composables (useTaskMutations, useTaskActions) and 1 sub-component
(TaskQuickMenu) extracted; duplicate mobile/desktop quick-menu markup deduped
Bundle (npm run build): main project chunk 2,621,285 -> 2,047,548 bytes
(-22%, -560 KB). New lazy chunks: project-list-view 381 KB, project-detail
117 KB, project-kanban 42 KB, project-table-view 28 KB, project-workload
17 KB, embed-view 3 KB, project-activity-log 0.5 KB.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(backend): split task/logtime/milestone/auth mega-files (#164)
Splits four oversized source files into focused sub-modules so no file in
the affected modules exceeds ~640 lines (issue target: ~800). Public API
is preserved: each original module path keeps re-exporting every symbol
it used to, so routes.js and downstream consumers are untouched.
- Modules/Tasks/helpers/task_class_Mongo.js (3,129 -> 27 lines)
Mixin pattern under taskMongo/: create, updateBasic, updateAssignment,
updateMe…
Manual release cut: #240 was squash-merged as a single chore commit, so release-please found no releasable commits on main. This mirrors what its Release PR would have produced - bumps package.json + the release-please manifest to 14.3.0 and adds the CHANGELOG section compiled from the PRs merged to staging since v14.2.0 (#228-#249). Co-authored-by: Claude Opus 4.8 (1M context) <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.
Back-merges
mainintostagingafter the v14.3.0 release so staging carries the version bump (package.json + .release-please-manifest.json → 14.3.0) and the CHANGELOG 14.3.0 section. Routine post-release sync. Merge as a merge commit (never squash).🤖 Generated with Claude Code