Skip to content

Commit 7039582

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

441 files changed

Lines changed: 48511 additions & 13043 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.claude/AI-TIME-ESTIMATION.md

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
# AI Task Time Estimation — How It Works
2+
3+
This doc explains how AlianHub estimates the time needed to finish a task,
4+
in plain language.
5+
6+
---
7+
8+
## The Big Idea
9+
10+
When a task is created, we ask an AI model: **"If an AI coding agent
11+
(Claude Code) did this task end-to-end, how many minutes would it take?"**
12+
13+
The answer is saved on the task as `totalEstimatedTime` (in minutes).
14+
15+
That's it. No rules. No formulas. Just a smart guess based on what the
16+
task actually says.
17+
18+
---
19+
20+
## What the AI Looks At
21+
22+
The AI gets these pieces of info about the task:
23+
24+
1. **Task title** — the headline.
25+
2. **Task type** — bug, story, task, etc.
26+
3. **Priority** — LOW, MEDIUM, HIGH, URGENT.
27+
4. **Parent or subtask?** — subtasks are usually smaller.
28+
5. **Description** — the most important input. This is what the AI
29+
reads to understand the actual work.
30+
31+
If the description is empty, the AI is told so and will be more
32+
conservative (since it only has the title to go on).
33+
34+
---
35+
36+
## The 5 Things the AI Thinks About
37+
38+
The prompt walks the AI through these **in order**. Each one pushes
39+
the estimate up or down.
40+
41+
### 1. What is the deliverable?
42+
43+
What kind of work is this? Different shapes have different floors:
44+
45+
- **Copy or config change** → small.
46+
- **Bug fix** → medium, depends on how deep the bug is.
47+
- **New feature** → medium to big.
48+
- **Refactor** → big, because it touches many files.
49+
- **Migration** → big, because data is at risk.
50+
- **Integration** → big, because external systems bring surprises.
51+
52+
### 2. How much surface area is involved?
53+
54+
The AI counts the things the description mentions or implies:
55+
56+
- How many files?
57+
- How many modules?
58+
- Any APIs, schemas, or UI screens?
59+
- Any third-party services?
60+
- Are tests needed?
61+
62+
More things = more time.
63+
64+
### 3. How clear is the description?
65+
66+
This is a big one.
67+
68+
- **Clear description** → faster. The AI agent knows exactly what to do.
69+
- **Vague description** → slower. The AI agent has to figure things
70+
out as it goes, and that takes real time.
71+
72+
### 4. How will the work be verified?
73+
74+
Some work is "done" when tests pass. Other work has to be seen and
75+
clicked.
76+
77+
- **Internal refactor** → run tests, done. Fast.
78+
- **Anything touching UI, data, auth, or payments** → run the app,
79+
click around, watch what happens, fix issues. Slower.
80+
81+
### 5. Iteration loops
82+
83+
In real work, the first try is rarely the last. The agent writes
84+
code, runs it, sees what breaks, fixes it, runs again. The prompt
85+
tells the AI to bake in 1–2 iteration rounds for anything that isn't
86+
trivial.
87+
88+
---
89+
90+
## What the Estimate Does NOT Include
91+
92+
The number is the AI agent's **focused work time**. It does NOT count:
93+
94+
- Human review or approval time.
95+
- Waiting for CI, builds, or deploys.
96+
- Meetings, handoffs, status updates.
97+
- External system queues.
98+
99+
So an estimate of 60 minutes means *the agent works for 60 minutes*
100+
not that the task is done in 60 minutes of calendar time on a real team.
101+
102+
---
103+
104+
## Safety Bounds
105+
106+
We never trust the AI's number blindly.
107+
108+
- **Minimum: 5 minutes.** Even the smallest task has some overhead.
109+
- **Maximum: 10080 minutes (7 days).** If a task would genuinely take
110+
longer, it should be split into smaller tasks.
111+
112+
If the AI returns something outside these bounds (like 0 or 9999999),
113+
we clamp it back inside. If the AI returns total garbage we can't
114+
parse, the task just stays without an estimate — no broken data ever
115+
gets saved.
116+
117+
---
118+
119+
## The Single Biggest Driver
120+
121+
**The description content drives the number more than anything else.**
122+
123+
- The title and priority give the AI a starting feel.
124+
- The description tells the AI the actual scope.
125+
126+
Two tasks both called "Fix login bug" can land at 30 minutes or 8
127+
hours depending on what their descriptions say. The prompt is built
128+
to make sure the AI actually reads the description before answering,
129+
and to discourage lazy round-number guesses like 60 / 120 / 240.
130+
131+
---
132+
133+
## How It Runs in the App
134+
135+
- After a task is created, we fire the estimator in the background.
136+
- It does NOT block the create API — the response is returned first.
137+
- When the estimate comes back (usually a few seconds), we update
138+
the task and send a Socket.io event so connected screens refresh
139+
the value automatically.
140+
- If no AI provider is configured in `.env`, the estimator quietly
141+
does nothing and task creation works exactly like before.
142+
143+
---
144+
145+
## Where the Pieces Live
146+
147+
| What | File |
148+
|---|---|
149+
| The prompt the AI reads | `Modules/AIProjectGenerator/prompts/project-plan/task-time-estimate.md` |
150+
| The estimator code | `Modules/EstimatedTime/aiTaskEstimator.js` |
151+
| Single-task hook | `Modules/Tasks/helpers/taskMongo/create.js` |
152+
| AI Project Generator bulk hook | `Modules/AIProjectGenerator/orchestrator.js` |
153+
| DB field | `tasks.totalEstimatedTime` (Number, in minutes) |
154+
155+
---
156+
157+
## In One Sentence
158+
159+
> The AI reads the task description, judges how much real work the
160+
> description describes (size, clarity, verification needs, iteration),
161+
> and returns a minute estimate — which we clamp to safe bounds and
162+
> save on the task.

0 commit comments

Comments
 (0)