chore(release): promote staging to main#240
Merged
Merged
Conversation
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
`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
`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
`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
`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
…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
`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
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
`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
…(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
`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
…30 / #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
) `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
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>
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>
`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>
`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>
`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>
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>
`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>
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>
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>
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>
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>
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>
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>
`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>
`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>
`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>
Fifteen QA test-case files covering the recently shipped features (relations, reactions, recents, burndown, auto-archive, relation alerts, global search, epics, exports, pages, public sharing, Jira import, languages, toolbar more menu and the whats new page), written in the same table format as the existing files in .claude/test-cases. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…235) Adds GitLab as a third social-login provider alongside Google and GitHub: a token-exchange module, server-side identity verification against GitLab /user, a gitlab-signup endpoint, the gitlabId field on the auth record, and a GitLab button on the login and invitation pages. Also reconciles the existing Google/GitHub flows so verification actually runs: the client now forwards the Google id-token and the GitHub/GitLab access token, the backend accepts a single configured client id, and the GitHub/GitLab redirect handlers each ignore the other provider's code so they no longer collide. Manual test cases (with provider app-configuration steps and URLs) for all three providers are added under .claude/test-cases. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The deploy job was gated on `github.event_name == 'push'`, so a manual re-run only ran validate and skipped deploy. Allow workflow_dispatch too, so a frontend rebuild + redeploy can be triggered after a server-side env change without pushing an empty commit. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The login/signup buttons never appeared on staging because the frontend bundle is built on the CI runner, where the gitignored frontend/.env is absent, and the OAuth VUE_APP_* vars were not mapped from secrets like the Firebase ones. Map the three providers enable-flags and client ids from GitHub secrets, and pin the constant github.com / gitlab.com OAuth base URLs, so the shipped bundle actually shows and wires the buttons. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…AlianHub MCP server (#238) * feat(mcp): add server-side permission enforcement and token auth for the MCP server Adds Config/permissionGuard.js — a backend mirror of the frontend permission model — so API clients (the MCP server, scripts, curl) using a valid token obey the same roles and permissions as the web app instead of bypassing checks that previously ran only in Vue. Wires the requirePermission / requireRole guards onto the task, sprint, project, member, estimated-time and security-permission routes the MCP server calls, hardens the API-token auth, and adds token-management scripts (issue / toggle / clear / print-permissions). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(mcp): grant the MCP permission set folder-create and project-edit keys Adds project_folder_create and the project name/description/details/ assignee permission keys to MCP_PERMISSION_KEYS so the MCP server can perform those project operations under the server-side permission guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds backend and frontend support for tokens, OAuth, permissions, search, reactions, visits, project workflows, changelog access, and supporting schemas, scripts, tests, docs, and UI wiring. ChangesPlatform rollout
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Modules/Tasks/routes.js (2)
104-130:⚠️ Potential issue | 🟠 MajorAdd permission middleware to the task relations endpoint.
The
/api/v2/tasks/relationsendpoint allows adding/removing task relations without permission enforcement. Other endpoints in the same file userequireTaskActionPermission()middleware (e.g.,PATCH /api/v2/tasksat line 58), but this endpoint lacks it. The validation functions only verify data format and companyId, not user authorization. TheuserDataparameter is captured but only used for audit history and notifications, not for access control.Apply
requireTaskActionPermission()middleware to enforce permission checks before dispatching relation operations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Tasks/routes.js` around lines 104 - 130, The POST `/api/v2/tasks/relations` endpoint is missing permission enforcement. Add the `requireTaskActionPermission()` middleware to the route definition for the `/api/v2/tasks/relations` endpoint, applying it in the same manner as other endpoints in the file (such as PATCH /api/v2/tasks at line 58) to ensure user authorization is validated before dispatching relation operations like addTaskRelation, removeTaskRelation, or getTaskRelations.
74-98:⚠️ Potential issue | 🔴 CriticalAdd permission enforcement to bulk operations endpoint.
The
/api/v2/tasks/bulkendpoint dispatches to sensitive operations (bulkUpdateStatus, bulkDelete, bulkUpdateAssignee, bulkUpdatePriority, bulkUpdateDueDate, etc.) without permission checks. The individual PATCH/api/v2/tasksendpoint enforces role-based permissions viarequireTaskActionPermission()middleware for these same operations, but the bulk endpoint bypasses this entirely.A user lacking
task.task_statuspermission cannot callupdateStatusvia PATCH, but can callbulkUpdateStatusvia POST. Apply the same permission enforcement to the bulk endpoint by addingrequireTaskActionPermission()middleware or validating the bulk action againstTASK_ACTION_PERMISSIONbefore dispatch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Tasks/routes.js` around lines 74 - 98, The app.post('/api/v2/tasks/bulk', ...) route handler lacks permission enforcement for sensitive operations like bulkUpdateStatus, bulkDelete, bulkUpdateAssignee, bulkUpdatePriority, and bulkUpdateDueDate. Add permission validation before dispatching to the taskMongo[action] call by either applying the requireTaskActionPermission() middleware to this endpoint or by validating the action parameter against TASK_ACTION_PERMISSION mapping within the handler, consistent with how the individual PATCH endpoint enforces role-based access control. Extract the companyId and user permissions from the request, verify that the user has the necessary permission for the requested bulk action, and return an appropriate error response if permission is denied.
🟡 Minor comments (12)
Modules/Sprints/routes.js-12-13 (1)
12-13:⚠️ Potential issue | 🟡 MinorAdd permission guard to validate read access to sprint burndown data.
The
/api/v2/sprints/burndownendpoint returns sprint and task data but lacks an explicit permission check. While the function does scope queries tocompanyIdfrom request headers, it does not verify that the requesting user has read access to the specific sprint. Consider addingrequirePermission('project.project_sprint_create', { write: false })or a similar read-level permission guard to ensure consistent access control with the project's permission model, especially given multi-tenant requirements.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Sprints/routes.js` around lines 12 - 13, Add a permission validation middleware to the `/api/v2/sprints/burndown` endpoint that checks the user has read access to sprint data. Currently the burndown.getSprintBurndown route handler lacks an explicit permission guard, relying only on companyId scoping from request headers. Insert a permission check middleware (such as requirePermission with 'project.project_sprint_create' and write: false flag) between the route path and the burndown.getSprintBurndown handler to ensure the requesting user has appropriate read-level access to the specific sprint, maintaining consistency with the project's multi-tenant permission model.Config/permissionGuard.js-44-53 (1)
44-53:⚠️ Potential issue | 🟡 MinorAdd
updateTaskNameto theTASK_ACTION_PERMISSIONmap.
updateTaskNameexists as a callable task action but lacks a permission key mapping, causing it to bypass fine-grained permission checks at line 187. Map it totask.task_name_editfor consistency with frontend field gates.Note:
moveTaskand similar structural operations intentionally pass through unmapped (see comment on lines 41–43); this is documented design for operations that lack a single permission key.deleteTaskdoes not appear to exist as a directly callable action in the current implementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Config/permissionGuard.js` around lines 44 - 53, The TASK_ACTION_PERMISSION object is missing a mapping for the updateTaskName action, which allows it to bypass permission checks at line 187. Add updateTaskName as a key to the TASK_ACTION_PERMISSION object with the value task.task_name_edit to ensure it receives proper fine-grained permission validation consistent with the frontend field gates. Note that other unmapped actions like moveTask are intentionally excluded per the documented design comments.frontend/src/components/molecules/PublicShare/PublicShareModal.vue-170-173 (1)
170-173:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle Clipboard API rejection before showing a success toast.
navigator.clipboard.writeText(...)is async; the current code always shows success even if permission is denied or write fails.Suggested fix
-function copyLink() { - navigator.clipboard.writeText(shareUrl.value); - $toast.success(t('Toast.Link_is_Copied_to_clipboard'), { position: 'top-right' }); -} +async function copyLink() { + try { + await navigator.clipboard.writeText(shareUrl.value); + $toast.success(t('Toast.Link_is_Copied_to_clipboard'), { position: 'top-right' }); + } catch (error) { + $toast.error(t('Toast.something_went_wrong'), { position: 'top-right' }); + } +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/molecules/PublicShare/PublicShareModal.vue` around lines 170 - 173, The copyLink function shows a success toast immediately without waiting for the navigator.clipboard.writeText promise to resolve. Make the copyLink function async and await the clipboard write operation before displaying the success toast. Additionally, wrap the clipboard operation in a try-catch block to handle potential rejections (such as permission denied or write failures) and show appropriate error feedback instead of success when the operation fails.scripts/print-permissions.js-48-48 (1)
48-48:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
SCHEMA_TYPEfor the rules collection query.The rules fetch currently uses
dbCollections.RULES; this should use the schema enum for consistency with repository standards.Suggested change
-const { dbCollections } = require('../Config/collections'); const { SCHEMA_TYPE } = require('../Config/schemaType'); @@ - const rules = await MongoDbCrudOpration(companyId, { type: dbCollections.RULES, data: [] }, 'find'); + const rules = await MongoDbCrudOpration(companyId, { type: SCHEMA_TYPE.RULES, data: [] }, 'find');As per coding guidelines, “Use SCHEMA_TYPE enum for collection type references instead of hardcoding string values” for
**/*.{js,ts}.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/print-permissions.js` at line 48, Replace the use of dbCollections.RULES with the appropriate SCHEMA_TYPE enum value in the MongoDbCrudOpration call to maintain consistency with repository coding standards. Locate the type property in the MongoDbCrudOpration function call and swap dbCollections.RULES for the correct SCHEMA_TYPE enum reference that represents the rules collection.Source: Coding guidelines
Modules/GlobalSearch/helpers/searchRules.js-8-14 (1)
8-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject non-string queries and trim
companyIdbefore validating.
validateSearchInputcurrently coerces any query type to string, so objects/arrays can pass length checks as unintended text. Tighten validation to enforce string input and normalized company scope keys.Suggested fix
const validateSearchInput = ({ companyId, query }) => { - if (!companyId) { + const normalizedCompanyId = typeof companyId === 'string' ? companyId.trim() : ''; + if (!normalizedCompanyId) { return { valid: false, reason: 'companyId is required.' }; } - const trimmed = String(query || '').trim(); + if (typeof query !== 'string') { + return { valid: false, reason: 'query must be a string.' }; + } + const trimmed = query.trim(); if (trimmed.length < MIN_QUERY_LENGTH) { return { valid: false, reason: `Query must be at least ${MIN_QUERY_LENGTH} characters.` }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/GlobalSearch/helpers/searchRules.js` around lines 8 - 14, The validateSearchInput function currently coerces query to a string without validating its type first, allowing objects and arrays to pass unintended validation, and it does not trim the companyId parameter before checking its existence. Add a type check to ensure query is a string before the trimming operation, rejecting non-string queries early in the function. Additionally, trim the companyId value before validating whether it exists to normalize company scope keys and prevent whitespace-related validation issues.Modules/PublicShares/helpers/shareRules.js-41-42 (1)
41-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEmail length limit should be distinct from name limit.
Line 41 validates email against
MAX_NAME_LENGTH(120 chars), but emails can legitimately exceed that. Consider using the same limit as description (MAX_TEXT_LENGTHat 5000) or a separate email-specific constant (typically 254 for RFC 5321).🔧 Proposed fix
Either:
- Use
MAX_TEXT_LENGTHfor email (permissive):- if (email !== undefined && String(email).length > MAX_NAME_LENGTH) { + if (email !== undefined && String(email).length > MAX_TEXT_LENGTH) { return { valid: false, reason: 'Email is too long.' }; }
- Or define a dedicated constant:
+const MAX_EMAIL_LENGTH = 254; - if (email !== undefined && String(email).length > MAX_NAME_LENGTH) { + if (email !== undefined && String(email).length > MAX_EMAIL_LENGTH) { return { valid: false, reason: 'Email is too long.' }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/PublicShares/helpers/shareRules.js` around lines 41 - 42, The email length validation on line 41 in the if condition checking String(email).length against MAX_NAME_LENGTH is using an inappropriately restrictive limit designed for names. Replace MAX_NAME_LENGTH with either MAX_TEXT_LENGTH (which allows 5000 characters) or define and use a dedicated email-specific constant following RFC 5321 standards (typically 254 characters). Choose the constant that best fits your email validation requirements and update the comparison in the condition accordingly..claude/test-cases/RecentTasks.md-21-21 (1)
21-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix summary grammar in totals line.
All statusshould beAll statusesfor clarity in user-facing QA docs.Suggested patch
-**Total:** 5 test cases · **All status:** ⏳ Pending +**Total:** 5 test cases · **All statuses:** ⏳ Pending🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/test-cases/RecentTasks.md at line 21, In the totals line on line 21, change the text "All status" to "All statuses" to correct the grammar. The line should read "**Total:** 5 test cases · **All statuses:** ⏳ Pending" for proper plural form in the user-facing QA documentation.Config/jwt.js-533-537 (1)
533-537:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep auth error payload consistent in PAT branch.
This 401 response omits
isJwtErrorwhile adjacent auth failures include it. Keep the field for downstream error handling consistency.Suggested patch
return res.status(401).json({ status: false, error: 'A valid companyid header is required for API token authentication', statusText: 'Unauthorized', + isJwtError: true, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Config/jwt.js` around lines 533 - 537, The 401 response payload for the missing companyid header validation (lines 533-537 in Config/jwt.js) is missing the isJwtError field that is included in other auth failure responses in the same file. Add the isJwtError field to the JSON response object returned in this block to maintain consistent error payload structure across all authentication failures, ensuring downstream error handling works correctly.frontend/src/components/molecules/TaskFilter/style.css-295-300 (1)
295-300:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe new shrink-to-content behavior is still overridden at smaller breakpoints.
width: autohere is negated by the media rule settingwidth: 43%, so the button-group crowding issue can still reproduce on common viewport sizes.💡 Suggested CSS adjustment
`@media`(max-width: 1800px){ - .task-filtersearch {max-width: 43%;width: 43%;} + .task-filtersearch {max-width: 43%;width: auto;} }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/molecules/TaskFilter/style.css` around lines 295 - 300, The issue is that the `width: auto` in the `.task-filtersearch` rule is being overridden by a media query rule that sets `width: 43%`, which causes the shrink-to-content behavior to fail at smaller breakpoints and recreates the button-group crowding problem. Locate the media query rule that targets the `.task-filtersearch` class and remove or adjust the `width: 43%` override to use `width: auto` instead, ensuring the shrink-to-content behavior persists across all viewport sizes.frontend/src/components/molecules/Burndown/BurndownModal.vue-68-76 (1)
68-76:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInitialize burndown loading when the modal is initially mounted open.
If
modelValueis alreadytrueat mount time, this watcher won’t run, soselectedSprintId/data bootstrap can be skipped.Suggested fix
-watch(() => props.modelValue, (open) => { +watch(() => props.modelValue, (open) => { if (open) { if (!selectedSprintId.value && sprintOptions.value.length) { selectedSprintId.value = sprintOptions.value[0].id; } else if (selectedSprintId.value) { fetchBurndown(); } } -}); +}, { immediate: true });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/molecules/Burndown/BurndownModal.vue` around lines 68 - 76, The watcher on props.modelValue in the BurndownModal.vue component only triggers on value changes, not on initial mount. If modelValue is already true when the component mounts, the watcher callback will never run, leaving selectedSprintId uninitialized and burndown data unfetched. Add the immediate option to the watcher configuration to ensure the callback executes immediately on mount with the current modelValue, or extract the initialization logic to a separate function and call it from both the watcher and a mounted lifecycle hook.Modules/Changelog/controller.js-122-124 (1)
122-124:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDon’t reuse prior release-time cache for a different changelog revision during backoff.
When
cacheKeychanges and retry backoff is active, returning the previousbyVersioncan leak stale timestamps into a newly parsed changelog payload.Suggested fix
- if (Date.now() < githubTimes.retryAtMs) return githubTimes.byVersion || {}; + if (Date.now() < githubTimes.retryAtMs) { + return githubTimes.key === cacheKey ? (githubTimes.byVersion || {}) : {}; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Changelog/controller.js` around lines 122 - 124, The retry backoff check on line 123 returns the cached githubTimes.byVersion without validating that the cacheKey matches, which causes stale timestamps from the old cache key to leak into a newly parsed changelog when the cacheKey changes. Add the cacheKey validation check (githubTimes.key === cacheKey) to the retry backoff condition on line 123 before returning githubTimes.byVersion, ensuring cached data is only reused when both the retry period is active AND the cache key still matches.frontend/src/components/molecules/TaskDetailTab/TaskDetailTab.vue-199-204 (1)
199-204:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a null guard before reading
user.idin visit tracking.Line 199 can return
undefinedwhile user cache is still hydrating; Line 203 then throws onuser.id.Suggested patch
function recordRecentVisit() { if (!props.task?._id) return; const user = getUser(userId.value); + if (!user?.id) return; apiRequest('post', '/api/v2/recent-visits', { entityType: 'task', entityId: props.task._id, userData: { id: user.id }, }).catch((error) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/molecules/TaskDetailTab/TaskDetailTab.vue` around lines 199 - 204, The getUser(userId.value) call can return undefined while the user cache is still hydrating, causing an error when attempting to access user.id in the apiRequest call. Add a null guard check to verify that the user object is defined before passing user.id to the userData object in the apiRequest. If user is undefined, either skip the API call or provide a fallback value for the user id.
🧹 Nitpick comments (12)
frontend/vue.config.js (1)
16-18: ⚡ Quick winUse an environment variable for the new
/shareproxy target.The new proxy target is hardcoded, which makes environment switching brittle and violates the repo config rule.
As per coding guidelines, "Use environment variables for all configuration and secrets, never hardcode sensitive values."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/vue.config.js` around lines 16 - 18, The `/share` proxy target in the devServer.proxy configuration is hardcoded as 'http://localhost:4000'. Replace this hardcoded string with an environment variable (e.g., process.env.SHARE_PROXY_TARGET or similar, depending on your naming convention) to allow configuration across different environments. Check how other proxy targets in the file handle environment variables to maintain consistency with existing patterns in the codebase.Source: Coding guidelines
Config/permissionGuard.js (1)
171-176: ⚖️ Poor tradeoffFail-open on internal errors may degrade security guarantees.
When
evaluatePermissionthrows (e.g., transient MongoDB/network failure), the middleware logs an error and callsnext(), allowing the request through. While the comment explains this is intentional to avoid blocking the whole app, it creates a window where permission checks are bypassed during outages.Consider:
- A circuit breaker that fails-closed after repeated errors
- Rate limiting requests during degraded permission state
- At minimum, emitting a metric/alert when this fallback activates so ops can investigate
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Config/permissionGuard.js` around lines 171 - 176, The catch block in the requirePermission middleware currently implements fail-open behavior by calling next() when evaluatePermission throws, which bypasses permission checks during transient failures and creates a security vulnerability. Implement at least one of the suggested mitigations: add a circuit breaker pattern that fails-closed after repeated errors, implement rate limiting for requests during degraded permission state, or emit a metric/alert when the error fallback activates. At minimum, add instrumentation (metrics/alerts) to the error handler so that operations can detect and respond to permission check failures, ensuring visibility into when this security-critical fallback is being triggered.Modules/ApiTokens/controller.js (1)
40-46: ⚡ Quick winError responses return HTTP 200 status code.
Error conditions (missing companyId, invalid input, etc.) use
res.send()which returns HTTP 200. This makes it harder for clients to detect failures without parsing the response body. Consider usingres.status(400).send()for validation errors andres.status(404).send()for not-found cases.As per coding guidelines, endpoints should use try-catch error handling, but returning 200 for errors may confuse API clients expecting standard HTTP semantics.
Example fix for createToken
if (!companyId || !userId) { - return res.send({ status: false, statusText: 'companyId and userId are required.' }); + return res.status(400).send({ status: false, statusText: 'companyId and userId are required.' }); } const check = validateCreateInput({ name, scopes, expiresInDays }); if (!check.valid) { - return res.send({ status: false, statusText: check.reason }); + return res.status(400).send({ status: false, statusText: check.reason }); }Also applies to: 75-77, 97-99, 130-132, 157-159
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/ApiTokens/controller.js` around lines 40 - 46, Error responses in the createToken function and related validation checks are returning HTTP 200 status code instead of appropriate error codes. Replace all `res.send()` calls that return error responses with `res.status(400).send()` for validation errors like missing companyId, userId, and invalid input from validateCreateInput, and use `res.status(404).send()` for not-found cases. This should be applied consistently across all validation error response locations in the controller to comply with HTTP semantics expectations.Source: Coding guidelines
tests/auto-archive-rules.test.js (1)
27-30: ⚡ Quick winAdd coverage for
afterDays: nulland empty-string edge cases.Current tests miss coercion inputs that can silently change archival timing behavior; add explicit expectations for
null/''.Suggested test additions
test('missing or junk afterDays falls back to the default', () => { expect(normaliseRule({ enabled: true }).afterDays).toBe(DEFAULT_AFTER_DAYS); expect(normaliseRule({ enabled: true, afterDays: 'soon' }).afterDays).toBe(DEFAULT_AFTER_DAYS); + expect(normaliseRule({ enabled: true, afterDays: null }).afterDays).toBe(DEFAULT_AFTER_DAYS); + expect(normaliseRule({ enabled: true, afterDays: '' }).afterDays).toBe(DEFAULT_AFTER_DAYS); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/auto-archive-rules.test.js` around lines 27 - 30, The test function 'missing or junk afterDays falls back to the default' currently only tests missing afterDays and an invalid string value ('soon'), but lacks coverage for null and empty string edge cases that could silently affect archival timing. Add two additional expect statements to this test function to verify that normaliseRule correctly coerces both afterDays: null and afterDays: '' (empty string) back to DEFAULT_AFTER_DAYS, ensuring these edge cases are explicitly handled.Modules/projectSetting/routes.js (1)
104-105: 🏗️ Heavy liftNormalize the new endpoint paths to kebab-case.
The newly added paths use camelCase segments. Prefer kebab-case route segments for consistency with the route format convention.
As per coding guidelines, “Format all API endpoints using kebab-case with version prefix (e.g., /api/v2/tasks/create)” for
**/routes.{js,ts}.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/projectSetting/routes.js` around lines 104 - 105, The route paths for the autoArchive endpoints use camelCase segments (projectSetting and autoArchive) but should use kebab-case to match the API endpoint convention. In the Modules/projectSetting/routes.js file, convert the path segments in both the getAutoArchive and setAutoArchive route definitions from camelCase to kebab-case: change projectSetting to project-setting and autoArchive to auto-archive. The method handler references (autoArchive.getAutoArchive and autoArchive.setAutoArchive) should remain unchanged.Source: Coding guidelines
.claude/test-cases/RelationAlerts.md (1)
11-15: 💤 Low valueConsider varying sentence structure in test steps.
The static analysis tool flagged repeated sentence beginnings (e.g., "1. As user 1...", "2. As user 2...") in steps RA_001 through RA_003. Rewriting to vary the sentence structure (e.g., "User 1 links task A to task B" instead of "As user 1 link...") would improve readability without changing the test's clarity.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/test-cases/RelationAlerts.md around lines 11 - 15, The test steps in RA_001, RA_002, and RA_003 have repetitive sentence structures with consistent beginnings like "As user 1" and "As user 2" that reduce readability. Rewrite the steps column for each of these test cases to vary the sentence structure while maintaining clarity, such as converting "As user 1 link..." to "User 1 links..." and mixing different grammatical patterns throughout the steps to improve overall readability without changing the test's intent or clarity..claude/test-cases/Languages.md (1)
11-17: 💤 Low valueConsider varying sentence structure in test steps.
The static analysis tool flagged repeated sentence beginnings ("As user 1...", "As user 2...") in the test steps. While functionally clear, varying the sentence structure would improve readability without changing meaning.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/test-cases/Languages.md around lines 11 - 17, Review the Steps column across all test cases (LG_001 through LG_005) in the Languages.md test case table and vary the sentence structure used to describe each step. Instead of using repetitive phrasing patterns, diversify how the instructions are written—some steps could use imperative verbs, others could use different grammatical structures—while maintaining the exact same functional meaning and clarity. For example, vary between action-oriented beginnings, conditional statements, and different syntactic patterns to improve readability without altering what the tester needs to do.frontend/src/components/molecules/Epics/EpicsPanel.vue (2)
43-43: 💤 Low valueUnnecessary import of compiler macros.
In Vue 3.3+,
definePropsanddefineEmitsare compiler macros that don't need to be explicitly imported in<script setup>. The import works but is redundant.♻️ Suggested fix
-import { defineProps, inject, ref, watch } from "vue"; +import { inject, ref, watch } from "vue";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/molecules/Epics/EpicsPanel.vue` at line 43, Remove the redundant `defineProps` import from the import statement in EpicsPanel.vue since Vue 3.3+ treats `defineProps` as a compiler macro that is automatically available in `<script setup>` blocks without explicit importing. Keep the other imports (`inject`, `ref`, `watch`) as they are still needed and must be explicitly imported.
111-116: ⚡ Quick winDelete operation lacks confirmation and error feedback.
The
deleteEpicfunction deletes immediately without user confirmation and silently fails (only console.error). Consider adding a confirmation dialog and a toast on failure for better UX.♻️ Suggested improvement
function deleteEpic(epic) { + if (!confirm($t('Projects.confirm_delete_epic'))) return; apiRequest('delete', `/api/v2/epics/${epic._id}`) .then((response) => { if (response.data?.status) fetchEpics(); - }).catch((error) => console.error('ERROR in delete epic: ', error)); + else $toast.error(response.data?.statusText || 'Delete failed', { position: 'top-right' }); + }).catch((error) => { + console.error('ERROR in delete epic: ', error); + $toast.error('Failed to delete epic', { position: 'top-right' }); + }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/molecules/Epics/EpicsPanel.vue` around lines 111 - 116, The deleteEpic function currently deletes immediately without user confirmation and only logs errors to the console. Add a confirmation dialog (using your UI framework's dialog/modal component) before the API request is made, and replace the console.error in the catch block with a user-facing toast notification to inform the user when the deletion fails. This improves UX by preventing accidental deletions and providing clear feedback on operation outcomes.Modules/Auth/controller/authHelpers.js (1)
320-328: 💤 Low valueInline require inside function.
require('axios')inside the function works but is evaluated on every call. Consider moving to the top of the file for consistency with other imports, though this matches the pattern used forhttpsinverifyGithubAccessToken.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Auth/controller/authHelpers.js` around lines 320 - 328, Move the axios require statement from inside the verifyGitlabAccessToken function to the top of the file with the other module imports. Remove the line const axios = require('axios'); from within the function body, and add it as a module-level import at the top of authHelpers.js to avoid re-requiring the dependency on every function invocation and maintain consistency with the file's import pattern.Modules/Pages/controller.js (1)
121-134: ⚖️ Poor tradeoffVersion snapshot failure is silently ignored.
The version snapshot is fire-and-forget (no
await). If the snapshot fails, the update proceeds and the previous version is lost with only a log message. Consider awaiting the snapshot or at least warning the caller.If version history is critical, consider awaiting the snapshot:
// Snapshot the outgoing body (only when the body actually changes). if (contentHtml !== undefined) { - await MongoDbCrudOpration(companyId, { + try { + await MongoDbCrudOpration(companyId, { type: SCHEMA_TYPE.PAGE_VERSIONS, data: { pageId: pageObjId, title: existing.title, content: existing.content || { html: '' }, rawText: existing.rawText || '', savedBy: existing.updatedBy || '', }, - }, 'save').catch((error) => { + }, 'save'); + } catch (error) { logger.error(`ERROR in page version snapshot: ${error.message}`); - }); + // Optionally: return res.send({ status: false, statusText: 'Failed to save version history.' }); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Pages/controller.js` around lines 121 - 134, The MongoDbCrudOpration call for saving page versions (SCHEMA_TYPE.PAGE_VERSIONS) is not being awaited, making it fire-and-forget. If the snapshot save fails, the main update proceeds and version history is silently lost. Add await to the MongoDbCrudOpration call so that version snapshot failures are properly handled and the operation waits for the snapshot to complete before continuing, ensuring the caller is aware of any failures in the version history save.utils/mongo-handler/createSchema.js (1)
91-92: ⚡ Quick winAlign recent-visits read index with actual query shape.
The list query filters by
userId+entityTypeand sorts byvisitedAt; addingentityTypeto the sort index improves selectivity and avoids extra filtering work.Suggested patch
-recentVisitsSchema.index({ userId: 1, visitedAt: -1 }); +recentVisitsSchema.index({ userId: 1, entityType: 1, visitedAt: -1 });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@utils/mongo-handler/createSchema.js` around lines 91 - 92, The first index definition on recentVisitsSchema does not include entityType, but the list query filters by both userId and entityType before sorting by visitedAt. To improve index selectivity and match the actual query shape, modify the first recentVisitsSchema.index() call to include entityType between userId and visitedAt in the index key specification so the index can efficiently handle the filter-then-sort operation without additional filtering work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5df46af6-2ba6-40ec-91a0-eb2a41dec98e
⛔ Files ignored due to path filters (2)
frontend/src/assets/images/svg/gitlab_icon.svgis excluded by!**/*.svgpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (146)
.claude/test-cases/AutoArchive.md.claude/test-cases/BurndownChart.md.claude/test-cases/CommentReactions.md.claude/test-cases/Epics.md.claude/test-cases/GlobalSearch.md.claude/test-cases/JiraImporter.md.claude/test-cases/Languages.md.claude/test-cases/OAuthGithubLogin.md.claude/test-cases/OAuthGitlabLogin.md.claude/test-cases/OAuthGoogleLogin.md.claude/test-cases/PagesWiki.md.claude/test-cases/PublicSharing.md.claude/test-cases/RecentTasks.md.claude/test-cases/RelationAlerts.md.claude/test-cases/TaskExports.md.claude/test-cases/TaskRelations.md.claude/test-cases/ToolbarMoreMenu.md.claude/test-cases/WhatsNew.md.env.example.github/workflows/main.ymlConfig/collections.jsConfig/jwt.jsConfig/permissionGuard.jsConfig/schemaType.jsConfig/setMiddleware.jsModules/ApiTokens/controller.jsModules/ApiTokens/helpers/apiTokenRules.jsModules/ApiTokens/init.jsModules/ApiTokens/publicApi.jsModules/ApiTokens/routes.jsModules/Auth/controller/authHelpers.jsModules/Auth/controller/createUser.jsModules/Auth/routes2.jsModules/Changelog/controller.jsModules/Changelog/init.jsModules/Changelog/routes.jsModules/Epics/controller.jsModules/Epics/helpers/epicRules.jsModules/Epics/init.jsModules/Epics/routes.jsModules/EstimatedTime/controller.jsModules/ExportJobs/controller.jsModules/ExportJobs/helpers/exportRules.jsModules/ExportJobs/init.jsModules/ExportJobs/routes.jsModules/GlobalSearch/controller.jsModules/GlobalSearch/helpers/searchRules.jsModules/GlobalSearch/init.jsModules/GlobalSearch/routes.jsModules/Importers/controller.jsModules/Importers/helpers/jiraRules.jsModules/Importers/init.jsModules/Importers/routes.jsModules/OAuth/controller.jsModules/Pages/controller.jsModules/Pages/helpers/pageRules.jsModules/Pages/init.jsModules/Pages/routes.jsModules/Project/controller/getProjectList.jsModules/PublicShares/controller.jsModules/PublicShares/helpers/shareRules.jsModules/PublicShares/init.jsModules/PublicShares/publicRenderer.jsModules/PublicShares/routes.jsModules/Reactions/controller.jsModules/Reactions/helpers/reactionRules.jsModules/Reactions/init.jsModules/Reactions/routes.jsModules/RecentVisits/controller.jsModules/RecentVisits/init.jsModules/RecentVisits/routes.jsModules/SaasAdmin/instanceAdmin.jsModules/SaasAdmin/routes.jsModules/Sprints/burndown.jsModules/Sprints/routes.jsModules/Tasks/helpers/taskMongo/relations.jsModules/Tasks/routes.jsModules/Webhooks/controller.jsModules/Webhooks/dispatcher.jsModules/Webhooks/helpers/webhookRules.jsModules/Webhooks/init.jsModules/Webhooks/routes.jsModules/createProject/routes.jsModules/gitlabOAuth/controller.jsModules/gitlabOAuth/init.jsModules/gitlabOAuth/routes.jsModules/projectSetting/autoArchive.jsModules/projectSetting/autoArchiveRules.jsModules/projectSetting/routes.jsModules/settings/Members/controller.jsModules/settings/Members/routes.jsModules/settings/securityPermissions/routes.jscron.jsfrontend/src/components/atom/ReactionBar/ReactionBar.vuefrontend/src/components/molecules/AutoArchive/AutoArchiveModal.vuefrontend/src/components/molecules/Burndown/BurndownModal.vuefrontend/src/components/molecules/Epics/EpicPicker.vuefrontend/src/components/molecules/Epics/EpicsPanel.vuefrontend/src/components/molecules/ExportTasks/ExportTasksDropdown.vuefrontend/src/components/molecules/GlobalSearch/GlobalSearchModal.vuefrontend/src/components/molecules/ImportJira/ImportJiraModal.vuefrontend/src/components/molecules/Pages/PagesPanel.vuefrontend/src/components/molecules/PublicShare/PublicShareModal.vuefrontend/src/components/molecules/RecentVisits/RecentVisitsDropdown.vuefrontend/src/components/molecules/TaskDetailTab/TaskDetailTab.vuefrontend/src/components/molecules/TaskFilter/style.cssfrontend/src/components/organisms/Comment/Comment.vuefrontend/src/components/organisms/Header/Header.vuefrontend/src/components/organisms/Header/style.cssfrontend/src/composable/commonFunction.jsfrontend/src/config/env.jsfrontend/src/locales/en.jsfrontend/src/locales/ja.jsfrontend/src/locales/ko.jsfrontend/src/locales/main.jsfrontend/src/locales/ptBr.jsfrontend/src/plugins/oauth/github/GithubAuth.vuefrontend/src/plugins/oauth/gitlab/GitlabAuth.vuefrontend/src/plugins/oauth/google/GoogleAuth.vuefrontend/src/plugins/oauth/oAuthPlugin.jsfrontend/src/plugins/oauth/oAuthProviders.vuefrontend/src/router/index.jsfrontend/src/utils/TaskOperations/index.jsfrontend/src/views/Changelog/Changelog.vuefrontend/src/views/Projects/components/ProjectFiltersToolbar.vuefrontend/vue.config.jsindex.jspackage.jsonscripts/clear-api-tokens.jsscripts/issue-api-token.jsscripts/print-permissions.jsscripts/toggle-api-token.jsserver.jstests/api-token-rules.test.jstests/auto-archive-rules.test.jstests/epic-rules.test.jstests/export-rules.test.jstests/jira-import-rules.test.jstests/page-rules.test.jstests/reaction-rules.test.jstests/search-rules.test.jstests/share-rules.test.jstests/webhook-rules.test.jsutils/mongo-handler/createSchema.jsutils/mongo-handler/mongoQueries.jsutils/mongo-handler/schema.js
Comment on lines
+450
to
+466
| if (assignCompany) { | ||
| const query = { | ||
| type: SCHEMA_TYPE.COMPANY_USERS, | ||
| data: [ | ||
| { | ||
| _id: new mongoose.Types.ObjectId(companyUserDocID) | ||
| }, | ||
| { | ||
| $set: { | ||
| status: 2, | ||
| userId: authRes._id | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| await mongoRef.MongoDbCrudOpration(assignCompany, query, 'findOneAndUpdate'); | ||
|
|
There was a problem hiding this comment.
Verify invitation ownership before linking userId to COMPANY_USERS.
The update matches only companyUserDocID; an attacker with another pending invite document ID in the same company could bind their OAuth account to it.
Suggested fix
if (assignCompany) {
+ if (!mongoose.Types.ObjectId.isValid(companyUserDocID)) {
+ return res.status(400).json({ status: false, message: "Invalid companyUserDocID" });
+ }
+
+ const inviteLookup = {
+ type: SCHEMA_TYPE.COMPANY_USERS,
+ data: [{
+ _id: new mongoose.Types.ObjectId(companyUserDocID),
+ // adapt field name to your schema for invited email:
+ email: email,
+ status: 1 // pending invite
+ }]
+ };
+ const inviteDoc = await mongoRef.MongoDbCrudOpration(assignCompany, inviteLookup, "findOne");
+ if (!inviteDoc) {
+ return res.status(403).json({ status: false, message: "Invitation mismatch or not pending" });
+ }
+
const query = {
type: SCHEMA_TYPE.COMPANY_USERS,
data: [
{
_id: new mongoose.Types.ObjectId(companyUserDocID)
},🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Modules/Auth/controller/createUser.js` around lines 450 - 466, The security
vulnerability in the createUser function's COMPANY_USERS update operation is
that the query only filters by companyUserDocID without verifying that the
current user is authorized to claim this invitation. Add an additional condition
to the query filter (in the first element of the data array) to verify
ownership, such as checking that the invitation's email or intended recipient
matches the authenticated user's email from authRes. This ensures an attacker
cannot bind their OAuth account to pending invitations intended for other users.
Comment on lines
+71
to
+73
| const { format, projectId, sprintId, projectName, userData } = req.body || {}; | ||
| const userId = userData && (userData.id || userData._id) ? String(userData.id || userData._id) : ''; | ||
| const check = validateExportInput({ companyId, format, projectId, sprintId, userId }); |
There was a problem hiding this comment.
Authorization is based on client-provided user identifiers across endpoints. The shared root cause is trusting request payload/query user ids instead of authenticated middleware identity, enabling cross-user access within a company scope.
Modules/ExportJobs/controller.js#L71-L73: deriveuserIdfrom authenticated request context for job creation ownership.Modules/RecentVisits/controller.js#L20-L21: removeuserDatatrust and bindrecordVisitto authenticated user identity.Modules/RecentVisits/controller.js#L54-L57: ignoreuidquery for reads; use authenticated identity.Modules/ExportJobs/controller.js#L105-L107: remove caller-supplieduidfrom list filtering and use authenticated identity.Modules/ExportJobs/controller.js#L124-L132: enforce download authorization via authenticated user identity only.
📍 Affects 2 files
Modules/ExportJobs/controller.js#L71-L73(this comment)Modules/RecentVisits/controller.js#L20-L21Modules/RecentVisits/controller.js#L54-L57Modules/ExportJobs/controller.js#L105-L107Modules/ExportJobs/controller.js#L124-L132
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Modules/ExportJobs/controller.js` around lines 71 - 73, Fix authorization
bypass by replacing all client-supplied user identifiers with authenticated user
identity from middleware. In Modules/ExportJobs/controller.js lines 71-73,
replace userData-based userId extraction with authenticated user identity for
job creation ownership. In Modules/RecentVisits/controller.js lines 20-21,
remove userData parameter trust and use authenticated user identity for
recordVisit calls. In Modules/RecentVisits/controller.js lines 54-57, ignore the
uid query parameter and use authenticated user identity for read operations. In
Modules/ExportJobs/controller.js lines 105-107, remove uid query parameter from
list filtering and use authenticated user identity instead. In
Modules/ExportJobs/controller.js lines 124-132, replace uid-based download
authorization checks with authenticated user identity verification only.
Comment on lines
+20
to
+22
| const { rows, projectId, sprintId, sprintName, folderId, folderName, userData } = req.body || {}; | ||
| const userId = userData && (userData.id || userData._id) ? String(userData.id || userData._id) : ''; | ||
| const check = validateImportInput({ companyId, projectId, sprintId, rows, userId }); |
There was a problem hiding this comment.
Do not trust caller-provided identity fields for user scoping.
Line 21 derives userId from req.body.userData, and Line 118 derives it from req.query.uid; both are client-controlled. This allows user impersonation and cross-user history access unless upstream replaces these values. Derive identity from authenticated middleware context only (e.g., req.userData/token claims), and ignore body/query identity inputs.
Suggested fix sketch
- const { rows, projectId, sprintId, sprintName, folderId, folderName, userData } = req.body || {};
- const userId = userData && (userData.id || userData._id) ? String(userData.id || userData._id) : '';
+ const { rows, projectId, sprintId, sprintName, folderId, folderName } = req.body || {};
+ const authUser = req.userData || {};
+ const userId = authUser && (authUser.id || authUser._id) ? String(authUser.id || authUser._id) : '';
...
- userData: { id: userId, Employee_Name: userData.Employee_Name || '', companyOwnerId: userData.companyOwnerId || '' },
+ userData: { id: userId, Employee_Name: authUser.Employee_Name || '', companyOwnerId: authUser.companyOwnerId || '' },
- const userId = String(req.query?.uid || '');
+ const userId = String((req.userData && (req.userData.id || req.userData._id)) || '');Also applies to: 118-124
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Modules/Importers/controller.js` around lines 20 - 22, The code derives
userId from untrusted client-controlled sources (req.body.userData at line 21
and req.query.uid at line 118), enabling user impersonation and cross-user
history access. Replace both derivations to obtain userId exclusively from the
authenticated middleware context (such as req.userData or token claims) instead
of body or query parameters. Remove the lines extracting userId from
req.body.userData and req.query.uid, and substitute them with retrieval from the
authenticated context where the identity has already been validated by upstream
authentication middleware.
Comment on lines
+22
to
+24
| const { targetType, targetId, emoji, userData, isProjectComment = false } = req.body || {}; | ||
| const userId = userData && (userData.id || userData._id) ? String(userData.id || userData._id) : ''; | ||
|
|
There was a problem hiding this comment.
Do not trust userData from the request body for actor identity.
This allows reaction spoofing by sending another user’s ID. Derive actor identity from authenticated server context (token/session), and ignore client-supplied user identity fields.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Modules/Reactions/controller.js` around lines 22 - 24, The controller is
extracting userId from userData in the request body, which allows an attacker to
spoof reactions as another user. Instead of deriving the user identity from
client-supplied userData, extract the userId from the authenticated server
context (such as req.user, a decoded JWT token, or session data). Remove
userData from the destructuring of req.body and replace the userId extraction
logic to use the authenticated context, while still accepting targetType,
targetId, emoji, and isProjectComment from the request body as those are
legitimate data payload fields.
Comment on lines
+21
to
+25
| const isValidUrl = (value) => { | ||
| try { | ||
| const url = new URL(String(value || '')); | ||
| return url.protocol === 'http:' || url.protocol === 'https:'; | ||
| } catch (e) { |
There was a problem hiding this comment.
Harden webhook URL validation against SSRF targets.
Protocol-only checks (http/https) still permit internal destinations (e.g., localhost, metadata IPs, private CIDRs). Since the server actively POSTs to saved URLs, this is an SSRF vector.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Modules/Webhooks/helpers/webhookRules.js` around lines 21 - 25, The
isValidUrl function in webhookRules.js currently only validates protocol
(http/https) but does not check if the URL hostname resolves to internal or
private destinations, creating an SSRF vulnerability since the server actively
POSTs to these URLs. Enhance the isValidUrl function to reject URLs with
hostnames that are localhost, loopback addresses (127.0.0.1, ::1), metadata IPs
(169.254.169.254), or private CIDR ranges (10.0.0.0/8, 172.16.0.0/12,
192.168.0.0/16). Extract the hostname from the parsed URL object and validate it
against these internal destination patterns before returning true, ensuring only
external/public addresses are accepted.
| "ua-parser-js": "2.0.0", | ||
| "winston": "3.17.0", | ||
| "winston-daily-rotate-file": "^5.0.0", | ||
| "xlsx": "^0.18.5", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the latest GitHub and OSV advisories for npm package "xlsx", and do they list a patched version available on npm?
💡 Result:
The npm package "xlsx" is no longer maintained on the npm registry [1][2][3]. Because the package is unmaintained and deprecated on npm, there are no patched versions available through the npm registry to address the vulnerabilities identified in recent GitHub and OSV advisories [2][3][4][5]. Key security vulnerabilities associated with the package include: - Regular Expression Denial of Service (ReDoS): Tracked as CVE-2024-22363 and GHSA-5pgg-2g8v-p4x9 [4][6]. - Prototype Pollution: Tracked as CVE-2023-30533 and GHSA-4r6h-8v6p-xvw6 [2][5]. - Various Denial of Service (DoS) vulnerabilities: Including CVE-2021-32012, CVE-2021-32013, and CVE-2021-32014 [7][8][9]. The maintainers of the project have shifted away from npm and GitHub [1]. To obtain versions that include fixes for these vulnerabilities (such as version 0.19.3 for the prototype pollution issue or 0.20.2 for the ReDoS issue), users must download them directly from the official SheetJS CDN [2][3][4][5]. You can install these versions using your package manager by referencing the specific tarball URL, for example: npm install https://cdn.sheetjs.com/xlsx-0.20.2/xlsx-0.20.2.tgz [1][3][4]
Citations:
- 1: [Security] Prototype Pollution in sheetJS SheetJS/sheetjs#2822
- 2: https://advisories.gitlab.com/npm/xlsx/CVE-2023-30533/
- 3: https://advisories.gitlab.com/pkg/npm/xlsx/
- 4: https://osv.dev/vulnerability/GHSA-5pgg-2g8v-p4x9
- 5: https://osv.dev/GHSA-4r6h-8v6p-xvw6
- 6: https://security.snyk.io/vuln/SNYK-JS-XLSX-6252523
- 7: https://osv.dev/vulnerability/GHSA-g973-978j-2c3p
- 8: https://osv.dev/vulnerability/GHSA-3x9f-74h4-2fqr
- 9: https://intel.aikido.dev/packages/npm/xlsx
Do not use xlsx from npm—package is unmaintained and contains unpatched critical vulnerabilities.
The xlsx package on npm is deprecated, completely unmaintained, and has multiple unpatched high-severity vulnerabilities (CVE-2024-22363, CVE-2023-30533, CVE-2021-32012, CVE-2021-32014, and others). There are no patched versions available through the npm registry.
To proceed with xlsx, you must either:
- Migrate to an alternative package (e.g.,
exceljs,xlsx-populate), or - Install the patched version directly from SheetJS CDN (e.g.,
npm install https://cdn.sheetjs.com/xlsx-0.20.2/xlsx-0.20.2.tgz), acknowledging that this bypasses npm's standard package management and maintenance practices.
Shipping with xlsx@^0.18.5 from npm exposes the application to critical security risks. This must be resolved before release.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` at line 82, The xlsx package dependency at version ^0.18.5 in
package.json is unmaintained and contains multiple unpatched critical
vulnerabilities (CVE-2024-22363, CVE-2023-30533, CVE-2021-32012, CVE-2021-32014,
and others). Remove the current xlsx dependency and either replace it with a
maintained alternative package such as exceljs or xlsx-populate, or if xlsx
functionality is specifically required, replace the npm registry reference with
the patched version from SheetJS CDN by using the URL
https://cdn.sheetjs.com/xlsx-0.20.2/xlsx-0.20.2.tgz. This must be resolved
before release to eliminate the critical security risk.
#241) Sharpens the value proposition (ClickUp/Jira alternative with built-in time tracking + AI planning), adds a feature-status table that marks Gantt/timeline as roadmap rather than implying it ships today, adds a how-AlianHub-compares table of capabilities that are free here but paid or absent in other open-source PM tools, adds a deploy-and-customize Aliansoftware services section, and repoints the dead ROADMAP.md links to Discussions (the ROADMAP file was removed earlier). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Restores ROADMAP.md (removed earlier) with an explicit, public free/paid boundary: everything that works today stays free under AGPL-3.0, and any future paid features are new enterprise add-ons (SAML/SCIM/LDAP, audit-log API, managed cloud, SLA support) rather than take-backs. Adds a Now/Next/Later direction section and how to suggest features. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 410-417: The phrase "Priority support & SLAs" in the README.md
file contradicts the explicit statement in SUPPORT.md that there is no SLA on
fixes or feature implementation. Either clarify the language in README.md to
specify that SLAs refer only to response-time targets (not fix or feature
implementation timelines), or revise the wording to remove the SLA claim
entirely and align it with the actual support guarantees documented in
SUPPORT.md. Ensure consistency between these two documents so the support
promise is not overstated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
A webhook can now set format=slack or format=discord (default json) so it can target a Slack or Discord incoming webhook directly — the dispatcher renders each task event into Slack Block Kit or a Discord embed instead of raw JSON, no transform service needed. format is validated on create/update, declared on the webhooks schema, and the transformer is a pure function covered by unit tests. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
When DEMO_MODE=true on the server, the public brand-settings endpoint returns demoMode plus the shared demo credentials, and a thin top banner appears app-wide (and on the login page) showing the demo notice, the demo login, a Star-on-GitHub link, and a Deploy-your-own link. Fully off by default - with DEMO_MODE unset the flag is false, the banner renders nothing, and no credentials are exposed, so it is inert in real deployments. Closes the code portion of the S0-03 demo-instance task. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Modules/Admin/common/controller.js`:
- Around line 108-110: The response handler is attempting to parse data as JSON
when the makeDefaultBrandSettings() function already returns a parsed object
rather than a JSON string. In the .then() block that handles the data parameter,
remove the JSON.parse() call and pass data directly to withDemo(), since data is
already an object when makeDefaultBrandSettings() resolves on the first-run
bootstrap path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7cd327e7-fa24-476e-a59b-e4a13c27ab7f
📒 Files selected for processing (6)
.env.exampleModules/Admin/common/controller.jsfrontend/src/App.vuefrontend/src/components/atom/DemoBanner/DemoBanner.vuefrontend/src/locales/en.jsfrontend/src/main.js
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/locales/en.js
A per-user quick-capture scratchpad opened from a header icon: a slide-over panel of colored notes with a bold title and body, debounced auto-save, a per-note ... menu (pin, expand to a large view, color, delete), and drag-to-reorder. Notes are private to the user and scoped to the company database. Backend: Modules/Stickies (CRUD + reorder) with a pure stickyRules layer and unit tests; the stickies collection is wired through all five schema registration points; every query is scoped by company + userId with content/title length caps and a per-user count cap. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
frontend/src/components/molecules/Stickies/StickiesPanel.vue (1)
144-149: ⚡ Quick winRename
colorsto UPPER_SNAKE_CASE to match repository convention.As per coding guidelines, “Use UPPER_SNAKE_CASE for constant values.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/molecules/Stickies/StickiesPanel.vue` around lines 144 - 149, The constant array `colors` should be renamed to UPPER_SNAKE_CASE to match the repository coding convention, consistent with the other constants like `COLOR_BG` and `SAVE_DEBOUNCE_MS` in the same block. Rename the `colors` variable to `COLORS` and update all references to this variable throughout the StickiesPanel.vue file to use the new uppercase name.Source: Coding guidelines
tests/sticky-rules.test.js (1)
68-77: ⚡ Quick winAdd a regression test for string boolean input on
isPinned.Current tests don’t cover the
"false"string edge case, so the coercion bug can regress silently.[simulate with:
validateStickyPayload({ isPinned: "false" }, { partial: true })should be invalid]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/sticky-rules.test.js` around lines 68 - 77, Add a regression test for the string boolean edge case on the isPinned field. Create a new test case that calls validateStickyPayload with isPinned set to the string "false" (along with the partial: true option) and verify that the result's valid property is false, ensuring that string boolean values like "false" are properly rejected and cannot be coerced to valid boolean inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/molecules/Stickies/StickiesPanel.vue`:
- Around line 260-272: In the remove function, before making the DELETE API
request to delete a note, you need to clear any pending debounce timer that may
be scheduled to save that note. Identify the debounce timer variable (likely a
map or object storing timers by note ID) and call clearTimeout or similar to
cancel any active timer for the note being deleted using note._id as the key.
This prevents a stale PUT request from firing after the note has been deleted on
the server.
In `@frontend/src/locales/en.js`:
- Around line 2658-2671: The Stickies object literal in the locales file is
missing its closing brace before the Demo object begins, causing a parse error.
Add a closing brace and comma after the drag_hint property (the last property of
Stickies) to properly close the Stickies object and separate it from the Demo
object that follows.
In `@Modules/Stickies/controller.js`:
- Around line 17-22: The getUserId function currently extracts user identity
from untrusted request body and query parameters, which allows callers to
impersonate other users. Replace this implementation to resolve the
authenticated user identity from the request's authenticated context (such as
req.user) rather than accepting user-provided input from body.userId,
body.userData, or query parameters. This same security issue appears in multiple
locations throughout the controller that call or rely on getUserId, so ensure
all user identity resolution throughout the controller uses authenticated server
context instead of trusting client-provided user identifiers.
- Around line 162-170: The loop that iterates through check.ids and calls
MongoDbCrudOpration with 'updateOne' for each item performs non-atomic updates,
meaning if any update fails mid-loop, the sortIndex values will be inconsistent
across the items. Replace the loop with a single bulk operation that updates all
items at once, or use a database transaction that ensures either all sortIndex
updates succeed together or the entire request fails. This way, the reorder
operation maintains data consistency even if failures occur.
- Around line 68-80: After each successful database mutation in the
MongoDbCrudOpration calls for sticky create/update/delete/reorder operations,
emit socket.io events to notify connected clients of the changes and call
removeCache() to invalidate stale cached data. Specifically, add socket event
emission and cache clearing at lines 68-80 (after successful sticky creation),
lines 102-115 (after update), lines 131-140 (after delete), and lines 162-173
(after reorder) in the controller.js file. Each write operation should follow
the pattern of emitting an appropriate socket event (e.g., for creation, update,
deletion, or reordering) with the relevant data and then invalidating the cache
to ensure real-time synchronization and prevent stale data from being served to
clients.
In `@Modules/Stickies/helpers/stickyRules.js`:
- Around line 45-47: The Boolean() coercion in the stickyRules.js file at the
isPinned assignment is incorrectly converting string values like "false" to true
because non-empty strings are truthy in JavaScript. Instead of using
Boolean(payload.isPinned), implement a strict boolean check that properly
handles string representations. Compare the payload.isPinned value explicitly
(for example, checking if it strictly equals true, or if it's a string, checking
if the lowercase string value equals "true") to ensure only actual boolean true
values result in isPinned being set to true. This prevents string values like
"false" from being incorrectly persisted as true.
---
Nitpick comments:
In `@frontend/src/components/molecules/Stickies/StickiesPanel.vue`:
- Around line 144-149: The constant array `colors` should be renamed to
UPPER_SNAKE_CASE to match the repository coding convention, consistent with the
other constants like `COLOR_BG` and `SAVE_DEBOUNCE_MS` in the same block. Rename
the `colors` variable to `COLORS` and update all references to this variable
throughout the StickiesPanel.vue file to use the new uppercase name.
In `@tests/sticky-rules.test.js`:
- Around line 68-77: Add a regression test for the string boolean edge case on
the isPinned field. Create a new test case that calls validateStickyPayload with
isPinned set to the string "false" (along with the partial: true option) and
verify that the result's valid property is false, ensuring that string boolean
values like "false" are properly rejected and cannot be coerced to valid boolean
inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 590a07ff-b55c-460e-ba8c-88f1599d6d2f
⛔ Files ignored due to path filters (1)
frontend/src/assets/images/svg/sticky_note_icon.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
Config/collections.jsConfig/schemaType.jsModules/Stickies/controller.jsModules/Stickies/helpers/stickyRules.jsModules/Stickies/init.jsModules/Stickies/routes.jsfrontend/src/components/molecules/Stickies/StickiesPanel.vuefrontend/src/components/organisms/Header/Header.vuefrontend/src/locales/en.jsindex.jstests/sticky-rules.test.jsutils/mongo-handler/createSchema.jsutils/mongo-handler/mongoQueries.jsutils/mongo-handler/schema.js
✅ Files skipped from review due to trivial changes (1)
- Modules/Stickies/init.js
🚧 Files skipped from review as they are similar to previous changes (7)
- Config/schemaType.js
- index.js
- utils/mongo-handler/mongoQueries.js
- utils/mongo-handler/createSchema.js
- Config/collections.js
- frontend/src/components/organisms/Header/Header.vue
- utils/mongo-handler/schema.js
Comment on lines
+17
to
+22
| // Resolve the acting user from the various shapes the clients send. | ||
| const getUserId = (req) => { | ||
| const fromBody = req.body && (req.body.userId || (req.body.userData && (req.body.userData.id || req.body.userData._id))); | ||
| const fromQuery = req.query && req.query.uid; | ||
| return String(fromBody || fromQuery || ''); | ||
| }; |
There was a problem hiding this comment.
Do not trust userId from request body/query for authorization.
This allows a caller to provide another user’s id and read/mutate that user’s stickies. Resolve actor identity from authenticated server context (e.g., req.user) and treat body/query user ids as untrusted input.
🔧 Directional fix
-const getUserId = (req) => {
- const fromBody = req.body && (req.body.userId || (req.body.userData && (req.body.userData.id || req.body.userData._id)));
- const fromQuery = req.query && req.query.uid;
- return String(fromBody || fromQuery || '');
-};
+const getUserId = (req) => String(req.user?._id || req.user?.id || '');
// in handlers:
-if (!companyId || !userId) {
+if (!companyId || !userId) {
return res.send({ status: false, statusText: 'Unauthorized.' });
}Also applies to: 25-31, 45-53, 87-95, 121-129, 146-153
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Modules/Stickies/controller.js` around lines 17 - 22, The getUserId function
currently extracts user identity from untrusted request body and query
parameters, which allows callers to impersonate other users. Replace this
implementation to resolve the authenticated user identity from the request's
authenticated context (such as req.user) rather than accepting user-provided
input from body.userId, body.userData, or query parameters. This same security
issue appears in multiple locations throughout the controller that call or rely
on getUserId, so ensure all user identity resolution throughout the controller
uses authenticated server context instead of trusting client-provided user
identifiers.
Comment on lines
+68
to
+80
| const created = await MongoDbCrudOpration(companyId, { | ||
| type: SCHEMA_TYPE.STICKIES, | ||
| data: { | ||
| userId, | ||
| title: check.data.title, | ||
| content: check.data.content, | ||
| color: check.data.color, | ||
| sortIndex: 0, | ||
| isPinned: false, | ||
| }, | ||
| }, 'save'); | ||
|
|
||
| return res.send({ status: true, statusText: 'Sticky created.', data: created }); |
There was a problem hiding this comment.
Emit realtime updates and clear cache after successful sticky mutations.
Create/update/delete/reorder write paths return success but do not publish socket events or invalidate cache, so connected clients can stay stale.
As per coding guidelines, “Emit Socket.io events after database mutations to ensure real-time synchronization across all connected clients” and “Clear cache entries after data mutations using removeCache() to prevent stale data from being served to clients.”
Also applies to: 102-115, 131-140, 162-173
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Modules/Stickies/controller.js` around lines 68 - 80, After each successful
database mutation in the MongoDbCrudOpration calls for sticky
create/update/delete/reorder operations, emit socket.io events to notify
connected clients of the changes and call removeCache() to invalidate stale
cached data. Specifically, add socket event emission and cache clearing at lines
68-80 (after successful sticky creation), lines 102-115 (after update), lines
131-140 (after delete), and lines 162-173 (after reorder) in the controller.js
file. Each write operation should follow the pattern of emitting an appropriate
socket event (e.g., for creation, update, deletion, or reordering) with the
relevant data and then invalidating the cache to ensure real-time
synchronization and prevent stale data from being served to clients.
Source: Coding guidelines
The Stickies block from PR #239 was missing its closing brace, nesting Demo inside Stickies and leaving the locale object one brace short, which broke the frontend build (Unexpected token, expected comma). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds an X on each Recently Added Projects row in the Employee Workload Edit Card. Clicking it dismisses the suggestion (without adding the project) and persists the dismissal in localStorage per user, so it stays hidden after the card is closed/reopened or the page is reloaded. Dismissals are pruned to the 2-day new-project window so the stored list stays bounded. The tick-to-add path and Save are untouched. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-count cap (#248) permissionGuard.js: the permission/role guards now run only for PAT (MCP / API-token) requests (req.apiToken). JWT/web-app requests pass straight through, so existing frontend/backend behaviour is byte-for-byte unchanged - fixes false denials on shared web routes (e.g. assigning a task). Also adds user_timesheet + workload_timesheet to MCP_PERMISSION_KEYS. createProject/controller.js: checkProjectPlan no longer enforces a plan project-count cap; creation is gated only by the project.project_create permission. Always resolves { status: true }. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…249) * fix: address safe CodeRabbit findings from the promotion (#240) review Six low-risk, verified fixes: - Webhooks/dispatcher.js: guard against an undefined pending entry in the debounce callback (entry.doc could throw). - gitlabOAuth/controller.js: add a 15s timeout to the token-exchange axios call (was unbounded). - Admin/common/controller.js: drop an erroneous JSON.parse in the missing-file branch (makeDefaultBrandSettings resolves an object) - fixes a first-run 404 on getBrandSettingsData. - Stickies/helpers/stickyRules.js: strict boolean for isPinned (the string 'false' no longer coerces to true). - StickiesPanel.vue: clear the pending debounced save in remove() so it cannot fire a stale PUT after delete. - README.md: 'Priority support & SLAs' -> 'response-time targets' to match SUPPORT.md. Verified: node --check (backend), sticky-rules jest 16/16, StickiesPanel SFC compile. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: scope updateMember role-promotion guard to PAT requests Companion to #248: the owner-only role-promotion check in updateMember now runs only for PAT/MCP requests (req.apiToken), so the web app's role management behaves exactly as before. Same 2026-06-15 MCP-isolation hardening as permissionGuard.js. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
parth0025
added a commit
that referenced
this pull request
Jun 16, 2026
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>
This was referenced Jun 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotion: staging → main
Promotes all work merged into
stagingsince the last release. 148 files changed (+9,356 / −56).Headline features included
NOT included (still open against staging)
Merge guidance
main, cut the release tag and back-mergemain→stagingto keep them aligned.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes