* test(core): pin what the tenant-isolation guards actually do
TenantBaseGuard and TenantPermissionGuard gate most controllers
(@UseGuards(TenantPermissionGuard, PermissionGuard)), yet no spec exercised
them: every other suite mocks or overrides them. This adds 39 behavioural
tests covering the tenant from the request context, the tenant-id header
path, the GET/DELETE query and data.findInput paths, the POST/PUT/PATCH body
paths, @Public, the super-admin switch, handler-over-controller permission
metadata, de-duplication, and the 5-minute permission cache (hit, miss,
per-role key).
Two current behaviours are pinned deliberately and documented in the spec,
not changed: a matching tenant-id header is the whole check (a body or
query tenantId naming another tenant passes the guard, so services must take
the tenant from context), and a permission verdict is cached for 5 minutes.
Mutation-checked: making the header or body comparison always pass,
ignoring the base guard's verdict, dropping the allowSuperAdminRole switch,
or changing the cache TTL each turns the suite red.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test(core): follow the house let/const rule and drop the lint findings in the guard spec
Review follow-up (Greptile). The context helper now declares the request
headers with let and rebuilds them instead of mutating a const, per the
repository's rule. Also clears the ESLint error (empty stub function) and
the nine no-explicit-any warnings the new spec added: typed casts through
unknown, and one typed handle on env.allowSuperAdminRole. Still 39/39, and
all five guard mutations are still caught.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* fix(auth): make email verification work end to end and stop register dead-ends
Verification link and notice
- /auth/confirm-email no longer sits behind NoAuthGuard. Registering signs the
user in, so the emailed link was opened by a signed-in user, redirected to the
dashboard before the resolver ran, and never verified anything. The token is
still required and still single use (the API clears it on success).
- A refused or expired link now shows its message instead of an endless spinner,
and a successful one marks the signed-in user verified in the store.
- New "verify your email" notice with Resend (POST /auth/email/verify/resend-link,
already throttled 3/min) in the main layout and on tenant onboarding. It shows
only when GET /auth/email/verify/status (new, same feature flag, so 404 where
verification is off) confirms the user is unverified.
- Settings > Billing: an unverified admin with no linked subscription is told a
paid plan connects once the address is verified.
Email sending
- Templates fall back to English when the recipient's locale has none, instead
of rendering an empty email (verification exists only in en/bg/he/ru).
- Send failures are logged with the provider code, SMTP reply code and command,
every address masked; verification sends are now recorded in email_sent with
status SENT/FAILED (subject only, never the link). A transport that fails
verification throws instead of returning undefined.
- resend-link answers 503 when the provider refused the message, not "OK".
- The verification link encodes the address (plus-addressing survived as a space).
- A caller-supplied appEmailConfirmationUrl is honoured only on an origin this
deployment serves (CLIENT_BASE_URL, the configured links, EMAIL_LINK_ALLOWED_ORIGINS;
"*" disables the check).
- Auth emails carry X-PM-TrackLinks: None so Postmark stops storing tokens as clicks.
- {{appLink}} falls back to CLIENT_BASE_URL when APP_LINK is empty: every
hosted deployment has APP_LINK empty, so welcome emails linked to localhost:4200.
Register
- A refused sign-up shows the API's 4xx message (e.g. "A subscription is
required...") instead of "Something went wrong", and the 403 checkoutUrl is
offered as a "Continue to checkout" button.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(auth): confirm-email trusts the API, not the link, for who was verified
Review follow-ups on the confirm-email page:
- After a successful confirmation, re-read GET /auth/email/verify/status
instead of comparing the link's email parameter with the signed-in user.
The token decides which account was confirmed; the email parameter is not
bound to it.
- A request that got no HTTP answer (status 0) is shown as a connection
problem, not as an invalid link.
- ActivatedRoute, Store and AuthService come from inject().
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(auth): apply a verification status only to the user it was asked for
If a different user signs in while the status or resend request is in flight,
the answer no longer updates the new user's verification state.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(ui-core): drop a resend answer once another user has signed in
The verification notice now tracks the user it speaks for. A different user
signing in cancels the pending resend and resets its state, and a late answer
for the previous user changes nothing.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(ui-core): a dismissed verification notice stays dismissed only for that user
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Product-scoped (hosted plan only) Stripe webhook linking, signup paywall, lazy tenant link and /billing routes; checkout-session proof at register/onboarding; BILLING_PRODUCT, BILLING_SIGNUP_PAYWALL and BILLING_WEBHOOK_LINKING (default off); 402 payment_method_required on a paid upgrade without a card. See the PR description for details.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The develop push of #10324 (9a6f8787) failed "Check Spelling and Typos
with cspell" (run 36399734369): 13 issues in 5 files. PRs into develop
run no cspell, so this surfaced only after the merge.
- "Nx's" (4x) and "callables": reworded in the comments.
- "internmap" (a d3 dependency, npm package name): added to .cspell.json.
- "avascript" / "msdt" in the safe-url spec: file-level cspell:ignore,
as the repo does elsewhere; they are the tail of the entity-encoded
"javascript:" payloads and the ms-msdt: scheme the tests feed in.
Comment, dictionary and directive changes only; no code or config
behaviour changes. Local cspell 6.31.3 on the five files: 0 issues
(the pre-fix safe-url spec, as a control: 6 issues).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A synchronous call cannot be interrupted (Jest's timeout included), so a
grossly slow isAllowedUrl is now reported after ONE call on the smaller
hostile input - the same single cold call the old 100/200 ms asserts
timed - instead of after the batches and the fourfold input have
multiplied the wait (CodeRabbit). Running each measurement in a child
process, as also suggested, is not done: a call that never returns
already ends in a failure (the job timeout), never a pass, and spawn/IPC
time would be billed to the measurements.
Known-dirty control (local): an injected O(n^2/200) scan in
isAllowedUrl makes the data: linearity test fail with a growth factor of
16.8 against the < 10 bound; the restored code passes (docs-ui 541/541).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- docs-ui safe-url linearity tests: time each input size in batches of
calls (doubled until a batch runs >= 50 ms, median of three) and take
the growth ratio as measured, with no 1 ms floor that could understate
growth for sub-millisecond samples (CodeRabbit). Keep a generous
absolute ceiling (3 s per rejection of the smaller input, ~9x the
slowest CI reading) so a uniformly slow validator cannot pass either.
- role-permission demo-mode suite: state its scope precisely. The reload
only adds rows, so it never removes a deletion grant a tenant already
holds; the seed and RolePermissionService keep demo tenants free of it,
and PermissionGuard has no demo-mode rule (a product question, out of
scope for this test-harness PR).
- Sonar: String.raw for the transformIgnorePatterns regex (pattern
verified byte-identical), startsWith in jest.resolver.js, and no
blanket eslint-disable in the new activepieces jest config (its
template, integration-make-com, has none; eslint clean).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Local runs after this commit: ui-core 45/45 suites, ui-auth 4/4, gauzy
29/29, desktop-ui-lib 41/41, gauzy-server 2/2, and every plugin UI project
that was red (integration-ai/github/hubstaff/upwork-ui, job-employee/
matching/proposal/search-ui, jobs-ui) green.
- jest.interop.js now patches every callable CommonJS package the code
imports both ways (moment, randomcolor) with `default` + `__esModule`, so
both import styles resolve to the function under either esModuleInterop.
apps/gauzy's spec tsconfig turns esModuleInterop on (as ui-core, docs-ui
and videos-ui do): ui-core's line chart default-imports
chartjs-plugin-annotation and Chart.register received undefined.
- jest.preset.js transforms d3-* / internmap (ESM-only, reached through
@swimlane/ngx-charts).
- desktop-ui-lib specs: section forms get the FormGroup their parent passes,
cell renderers their rowData, dialogs their data; Settings/Setup/
ScreenCapture get the library's GAUZY_ENV and the time tracker the
AuthStrategy/AuthService the desktop apps bootstrap; the task table a
signed-in session. Two test-side overrides are documented in place:
LanguageSelectorComponent binds [(ngModel)] without FormsModule (a dead
binding in the app too), and the plugin source forms use <nb-hint> as a
plain styled element (Nebular has no such component).
- apps/gauzy: page specs get the date-picker config their route sets,
the timesheet layout / job pages their route data, the task dialog a
selected organization (its unguarded async read crashed the Jest worker)
and a stand-in for docs-ui's links panel (docs-ui's entry point loads a
PDF viewer that uses import.meta, which CommonJS Jest cannot parse).
- integration-upwork-ui: two hand-written specs used jasmine spies (jest
has none) and partial Router/TranslateService mocks the template could
not render with; they use jest.fn() and the real services now.
- gauzy-server's AppComponent spec was the 2021 CLI scaffold (title
'desktop-web-ui', a "Welcome" h1) and declared a standalone component; it
now imports it and asserts what the shell does (router outlet, language
bridge started on init), with the library entry mocked to its one token.
- jobs-ui: the layout spec has asserted a main landmark since it was
written; the layout now renders <main role="main"> around its outlet (the
app shell defines no other main landmark).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Once the Angular suites loaded, most failures were CLI-generated
`should create` stubs that had never run, failing on the first injection
(TranslateService, NbToastrService, NbDialogRef, ActivatedRoute, the
translate pipe, the default icon pack).
- jest.angular-defaults.ts (setupFilesAfterEnv, after each project's
test-setup): a top-level beforeEach adds what the app's root module
provides - TranslateModule.forRoot, the Nebular forRoot modules,
NgxPermissionsModule.forRoot, the Tabler/eva icon pack, HttpClient
testing, router, noop animations, a NbDialogRef stub, GAUZY_ENV - and a
matchMedia stand-in (jsdom has none). configureTestingModule merges, so a
spec's own providers still win. It imports no ui-core/core: that would
cache real modules a spec later jest.mock()s.
- jest.preset.js maps dayjs/esm (imported by ngx-daterangepicker-material's
.mjs bundle) to dayjs's CommonJS build.
- Specs of non-standalone components import the NgModule that declares
them, provide what the host feature module provides (EmployeesService,
AuthService, NbAuthModule.forRoot, PipesModule), and set the inputs /
route data / date-picker config the app always supplies before render.
- Two stubs imported classes their files do not export
(GauzyRangePickerComponent, ViewComponent) and declared `undefined`.
- desktop-ui-lib's test-setup installs an inert `window.electronAPI`
preload bridge of the shape apps/agent exposes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Iteration 1 of the hermetic run (36330457608, 83 pass / 14 fail) moved the
Angular suites past TestBed init and into two load-time failures:
- "(0, moment_1.default) is not a function" in ~50 suites across 11
projects. ui-core imports `moment` as a default import, most other code
as a namespace import; the bundlers accept both, the TypeScript CJS emit
Jest runs cannot under a single esModuleInterop setting. jest.interop.js
(a preset setupFiles entry, concatenated with any project's own) gives
the loaded module `default` and `__esModule`, so both styles resolve to
the moment function itself, as they do in the app build.
- "Cannot find module '@gauzy/ui-config'" in 23 ui-core suites: ui-core's
tsconfig.json maps it to the BUILT dist copy, which a test run does not
have, and Nx's Jest resolver falls back to the spec tsconfig's paths. The
spec tsconfig now carries the same source mappings as tsconfig.lib.json.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
docker/build-push-action uploads a build record (*.dockerbuild) for every image
build. The record stores every build-arg value in plain text, and on a public
repo any signed-in GitHub user can download it. Set DOCKER_BUILD_RECORD_UPLOAD
and DOCKER_BUILD_SUMMARY to false at workflow level in every workflow that uses
the action. BuildKit secrets are never recorded, so nothing else changes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(security): prove GitHub installation ownership before binding it (GHSA-4rwq)
POST /integration/github/install bound whatever installation_id the request
body carried, as long as the state nonce belonged to the caller's tenant. The
nonce proves which tenant started the flow, not which GitHub installation that
tenant may bind — and binding hands the tenant the App's token for every
repository in the installation. A tenant could bind another organization's
installation and read its private repositories.
The GitHub App now issues an OAuth code with the post-install redirect ("Request
user authorization (OAuth) during installation"). POST /install — the
authenticated request — exchanges it and binds only an installation the
authorizing GitHub user is entitled to in full:
- a personal installation: the user must be that account;
- an organization installation: the user must already reach every repository
it covers (their count equals the App's own count, read before and after to
close a create/delete race).
The code is signed with the nonce it arrived with, so a code leaked from a
post-install URL cannot be replayed against another tenant's nonce, and the
user token is revoked once checked. Without a code the install is refused with
a message naming the GitHub App setting to enable.
Also closes a sibling on the same surface: GithubMiddleware loaded an
integration's settings before authentication using ?tenantId= ahead of the
Tenant-Id header, while the tenant guard validates only the header when one is
sent. Own tenant in the header plus a victim's in the query let the repository,
metadata, issue and sync routes act on the victim's installation. The
middleware now records who owns the settings, and GithubIntegrationTenantGuard
refuses a mismatch after authentication.
The install popup now shows why a connection was refused instead of closing
after two seconds, and handles the update/request redirects GitHub sends.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(security): spend the install code at the callback, compare repository ids
Two P1 review findings on the first version of this fix, both correct:
- The post-install callback is public and signed ANY code with ANY live nonce,
so a leaked, unused victim code plus a nonce from the attacker's own tenant
still bound the victim's installation — the code binding proved nothing. The
callback now exchanges the code the moment it arrives (spending it, so it
cannot be replayed from a URL, history or log), checks entitlement there, and
hands the browser only a signed, 10-minute proof bound to this flow's nonce
and installation. The code never reaches the browser, and POST /install binds
nothing without a valid proof. When there is no proof, install_check tells
the web app why (no code / not entitled / unverifiable).
- Comparing repository COUNTS could be balanced by a member who creates
throwaway repositories and deletes them between the reads while the
repositories hidden from them remain. Entitlement for an organization
installation is now a set comparison: every repository id the App can reach
must be one the user can read (user ids read first, App ids after).
Also: installation ids beyond Number.MAX_SAFE_INTEGER are refused (Octokit
takes a number and would check a different installation than the one stored),
member install requests (setup_action=request) no longer hit a raw 400, and the
popup text is translatable.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The Unit Tests workflow has never been green (24 of 97 projects red at
develop eac7136562, run 36319732347). Causes addressed here:
- Env leak: Nx loads the committed .env.local (DEMO=true,
WORKER_QUEUE_ENABLED=false, NODE_ENV=development) into every task.
The test step now sets NX_LOAD_DOT_ENV_FILES=false, and the two specs
that depend on a mode pin it themselves (role-permission counts pin
environment.demo=false and gain a demo-mode suite asserting 209 per
role; the worker spec clears the queue env before the constants load).
.env.local is unchanged.
- Angular harness: nohoist gives each workspace its own @angular copy,
so setupZoneTestEnv() initialised a different TestBed from the one the
spec used. A workspace resolver (jest.resolver.js, wrapping Nx's) makes
every @angular/* import in a Jest project resolve to one copy. The
Angular projects now inherit the preset's transformIgnorePatterns,
which gains the .mjs exception plus @datorama, @ngneat and lodash-es.
- ui-config's environment.ts is generated and gitignored; the workflow
runs `yarn config:dev` before the tests, as the Playwright workflow does.
- Misconfigured targets: gauzy (jest.config.js -> .ts, Angular transform,
setupFile moved into the config), integration-sim-ui (.ts -> .cts),
integration-activepieces (config file added), mcp-auth (ran
`node build/main.js --test`; now the jest executor like apps/mcp).
- Real failures: the openai "silence" case used a body with no `text`,
which the shared helper rejects by contract; the toolbar spec read the
tabIndex property, which is 0 on any button; the docs-ui linearity
tests asserted wall-clock bounds, now a 4x-input growth ratio.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Render blueprints declare SENTRY_DSN with sync: false, so Render prompts
the person creating the Blueprint for their own DSN instead of shipping one.
The desktop, desktop-timer, server, server-api and agent apps logged the full
DSN at startup; they now only say that Sentry is enabled.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
.env.compose (loaded by docker-compose.yml by default), .env.demo.compose,
.env.docker, render.yaml, .render/render.demo.yaml and both fly.toml files
carried the Ever Gauzy project's DSN, so every self-hosted install reported its
errors and log lines into Ever's Sentry project and spent its quota. They are
now empty, with a note to set your own DSN. Existing installs keep the value
they were created with; only rotating that key stops them.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SENTRY_LOG_LEVELS accepted 'fatal', but SentryService had no fatal override, so
Nest's ConsoleLogger.fatal printed the message and Sentry never saw it (also
before this branch). A fatal log now becomes an event whenever fatal or error is
captured, and is a breadcrumb otherwise. The level list is a Set.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Static Checks / lint (x64-4 lane): every cache hit since the job moved to
this lane ran out of space. The 2.47 GB archive plus the tree it unpacks to
does not fit the 16Gi RAM-backed workspace, so each hit fell back to a
1.5-4 h cold install (16 of 16 runs cold since #10303). The Restore step
now moves the archive onto the disk-backed package-cache volume (the
parent of YARN_CACHE_FOLDER) before extracting, so only the tree lives in
RAM. Where YARN_CACHE_FOLDER is unset, or the move fails, it extracts in
place exactly as before. Two report-only df lines (after the restore and
at the top of Summarize) record workspace headroom and can never fail a
step.
Triggers: apply the owner decision of 2026-09-21 (already on draft #10254,
same text) directly to develop. A pull request INTO develop no longer
starts static-checks, typos (cspell), snyk-analysis (trigger removed,
restore lines kept in a comment), or build, secrets-analysis and the
external-uptime-monitor self-test (branches-ignore: develop, so PRs into
stage/master still run them). Every push trigger is unchanged, so all of
them still run when code lands on develop. develop has no required status
checks, so no PR is blocked by the missing runs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The whole ever-co Sentry organisation (5,000 errors a month on the current
plan) has accepted no error after the first hours of each monthly reset:
2026-07-09, 08-09 and 09-09 are the only days with accepted errors in 90 days.
About 97% of what was accepted were info-level log lines, not errors.
- SentryService (the API's Nest logger) turned every log/warn/debug/verbose
call into its own Sentry event and ignored the `logLevels: ['error']` the API
passes. It now honours `logLevels`: listed levels become events, the rest
become breadcrumbs on the next event. An unset or empty list keeps the old
capture-everything behaviour. The API reads the list from SENTRY_LOG_LEVELS
(default `error`), so capturing warnings or logs again is a config change.
- RequestContextMiddleware logged the start and end of every request,
including the Kubernetes readiness probe on /api/health every 10 s on every
pod: 2 events per probe, ~2,900 events an hour from the four Ever Teams API
pods alone. /api/health and /api/health/* are no longer logged. This is
decided by path only, since a client-set User-Agent must not be able to hide
other requests.
- apps/api/src/sentry.ts printed the DSN at startup and ran the SDK in debug
mode in production (`environment.production` is false in the published
image), writing several SDK lines per request. Debug is now opt-in with
SENTRY_DEBUG=true, and the DSN is no longer printed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review follow-up (Greptile). core's and integration-zapier's
tsconfig.spec.json comments, and the idempotency README, still said allowJs
was required for transformIgnorePatterns to take effect. With the pinned
ts-jest (>= 29.3.2) that is no longer true for files under node_modules,
which is all the list covers. Reword the three comments; the allowJs
settings themselves are unchanged (identical parsed JSON).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Six service/controller specs in three plugins failed to load, so they ran
zero tests. Two defects were stacked, and the first hid the second.
1. Their testing modules load @gauzy/core, which imports uuid. The installed
uuid (14.0.0) is ESM-only. The allow-list of ESM packages Jest must
transform lived in packages/core/jest.config.ts, with a copy in
integration-zapier, and not in the shared preset. Move it into
jest.preset.js so inheriting projects get it, and let core inherit it
(jest --showConfig resolves to the byte-identical pattern). zapier keeps
its own copy, which replaces the preset's, and now points at the preset.
No allowJs is needed: ts-jest >= 29.3.2 compiles node_modules .js to
CommonJS regardless. All six specs pass with allowJs unset.
2. The specs are Nest CLI "should be defined" scaffolding whose testing
modules never resolved the dependency graph of the class under test.
Add stub providers for exactly what each class injects, and override
the @UseGuards guards in the controller specs. Nest builds those in the
module that owns the controller, but they are not constructor
dependencies. The spec changes are additive only: no it/expect changed
and nothing declared was removed.
Verified locally, before vs after on 1b2278d329:
- the three plugins go from 0 tests run to green (videos 3/3, 8 tests;
job-proposal 2/2; wakatime 2/2)
- the other 31 plugins with specs are unchanged, suite for suite
- the 11 non-plugin projects that inherit the preset and have specs are
unchanged
- core passes 179/179 suites, 2174 tests
- a mutation check (drop one repository stub) turns the spec red
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The "new version available, download it?" dialog was registered with
autoUpdater.once(), so it could appear only once per app run. Since every
check now looks up the newest release again (#10305), a release published
later in the same run got no dialog. The once() listener was also used up
when the first version was found while automatic updates were off, so no
dialog appeared in that run even after they were turned back on. The
dialog now appears once for each new version. A version the user has
already answered in this run is not offered again, including the repeat
event that follows choosing Upgrade.
Only one update dialog is shown at a time. The dialog does not open while
it or the "ready to install" prompt is still showing; the next check
offers that version again. A "ready to install" prompt for a download
that finishes while the dialog is open waits until the dialog is
answered. An error showing either dialog is now logged instead of being
left as an unhandled rejection. The OS notification and the settings
page events are unchanged.
The settings page sends automatic_update_setting as { isEnabled, delay },
but the main process read automaticUpdateDelay, and AutomaticUpdate's
delay getter threw away any delay passed in and used the stored setting.
It worked only because the settings page happens to save the setting
first. The handler now accepts both names, handled on the receiving side
so existing settings pages keep working. The loop uses the delay it was
sent, falling back to the stored delay and then 1 hour. Delays that
setInterval cannot hold are ignored, since they would make it run every
millisecond.
Turning on the prerelease channel filtered the GitHub releases down to
prereleases only, so those users were never offered a newer stable
release. On the real release list they were held at v111.44.15, or even
v102.0.1, while v111.44.48 was out. The prerelease channel now means the
newest release of either kind, still preferring the newest one that
already has this platform's update file. Stable-only users are unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every Gauzy app snap (desktop, desktop-timer, server, api-server, agent,
mcp-server; amd64 and arm64) went to the Snap Store 'edge' channel only,
from both the stage lane (stage-apps) and the prod lane (apps). The
electron-builder snap target publishes with its own snapStore config.
When the build config has no snapStore entry, that config has no
channels, and the Snap Store publisher then defaults to 'edge'.
Prod builds now release to 'stable' and stage builds to 'edge'. Each
Linux build step appends
-c.snap.publish.provider=snapStore -c.snap.publish.channels=<channel>
to its yarn command. yarn adds extra arguments to the end of the script,
which is the electron-builder call. The override is in the workflows
because the root build scripts are shared by both lanes. Only
snap.publish is overridden: a -c.publish override would be merged into
the GitHub publish entry. GitHub releases, update channels and every
other target are unchanged. Stage names 'edge' explicitly so each
prod/stage pair still differs only in the channel.
The generated snaps already use grade stable and strict confinement,
and they need no store-approved plugs, so the store accepts them on
stable.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Nothing stopped a role from holding the same permission twice. The
*RolePermissionsReload* migrations read which permissions a role lacks
and then insert them, so two API processes running them against one
database at the same time both inserted the same rows. Any install that
ran two instances through those migrations can hold millions of such
duplicate role_permission rows.
Add migration AddRolePermissionUniqueIndex1790000017000, which creates
the unique index IDX_role_permission_unique on (tenantId, roleId,
permission) for Postgres, MySQL and SQLite:
- It builds the index first. With no duplicates that build is the only
pass over the table and nothing is rewritten. If the build fails on a
duplicate key, the duplicates are deleted in one statement and the
index is built again. Each group keeps the row that actually grants
the permission (enabled, active, not archived, not soft-deleted),
otherwise a row the app can still see, then an enabled row, then the
oldest one, then the smallest id. So no role loses a permission it
has. Rows with a NULL tenantId are left alone.
- On Postgres it runs inside the migration's transaction under a
SHARE ROW EXCLUSIVE lock with a 5 s lock_timeout. Reads keep working,
writers wait for the build (about 5 s per 2M rows), and a second
process starting at the same time waits, then finds the index done.
The build does not use CONCURRENTLY: a failed CONCURRENTLY build
leaves an INVALID index, while this one just rolls back and runs
again on the next start. An INVALID index already using this name is
refused with instructions instead of being accepted as done.
- On MySQL the index is looked up by name before and after the build,
so a retry after a crash or a lost race with another process does
not fail.
- down() drops only the index.
Declare the same index on the RolePermission entity so synchronize and
migration:generate produce the same schema. Make the batched reload
insert skip rows another process inserted first (ON CONFLICT DO NOTHING
on Postgres and SQLite, a no-op ON DUPLICATE KEY UPDATE on MySQL).
Without that, the unique violation would be caught and logged, and
every tenant after the failing one would miss its new permissions.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The GitHub update feed is github.com/<owner>/<repo>/releases/download/<tag>.
CdnUpdate.tagName() looks up <tag> through the unauthenticated releases API.
- Any failed lookup (offline, or the 60 requests/hour/IP rate limit, whose
403 body is an object rather than a list) returned app.getVersion()
without the "v" that release tags carry (111.44.45 instead of v111.44.45).
That URL 404s until the app restarts. A failed lookup now keeps the last
resolved tag and falls back to v<version>. A non-array response counts as
a failure. The fallback is null-safe even when the prerelease setting
itself cannot be read.
- The tag was only resolved at startup or on a strategy change, so the
hourly automatic check and the manual check of a long-running app kept
polling the release it started with. GithubCdn.checkUpdate() now
re-resolves before every check. A tag resolved within the last minute is
reused, so the startup and strategy-change paths still make one API call
per check. The local strategy is untouched. download_update keeps the URL
of the check that found the update.
- Because the lookup now runs before every check, it is bounded by a 10 s
timeout, so a hung connection cannot stall a manual check.
- A release is published 15-60 minutes before its per-platform latest*.yml
files are uploaded, and some releases never get one (desktop-timer
v111.44.42 only has latest-x64-linux.yml). The lookup now prefers the
newest matching release that already has this platform's update file, and
otherwise falls back to the newest one. electron-updater does not
downgrade (allowDowngrade stays false), so picking an older release only
means "no update" instead of a 404.
Prerelease semantics are unchanged. An appSetting without a prerelease key
now means stable instead of matching nothing.
desktop-updater.spec.ts covers these paths (16 cases). tsc passes on the
changed files and the spec.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Gauzy Server's Windows and Linux release jobs published channel-less
update manifests (latest.yml, latest-linux*.yml) from v107 to v111.44.45.
Its :isolated scripts ran `pack --arch` after the build had already copied
apps/server/src/package.json into dist. electron-builder reads
dist/apps/<x>/package.json, never saw build.publish[].channel and fell
back to `latest`. The updater only requests `latest-${process.arch}`, so
installs stopped auto-updating, and nothing caught it for 4.5 months.
#10299 fixed that app. This change makes the whole class fail loudly.
Add .scripts/electron-package-utils/assert-publish-channel.js, a Node
script with no dependencies. It takes --project <dist dir> --arch
<x64|arm64> and requires every build.publish[] entry in
<dist dir>/package.json to have channel latest-<arch>. Otherwise it prints
a GitHub `::error` annotation naming the file and the expected and actual
channels, plus a hint, and exits 1. A missing file, unreadable JSON or bad
arguments also exit 1.
Run it immediately before electron-builder in the 28 root scripts that
publish for --windows/--linux and stamp the channel with `pack --arch`.
The list was found from the scripts, not typed by hand. Each guard's
--project and --arch are asserted equal to that script's electron-builder
--project, its --x64/--arm64 flag and its pack --arch. A bad channel now
stops the job before anything is published. No other script changes.
The 24 win/linux publishing scripts without `pack --arch` have no callers
in any workflow and are left alone.
Add assert-publish-channel.test.js (node --test, no dependencies) as
`yarn test:publish-channel` and run it in build.yml next to
test:postinstall. It covers the pass, fail and misuse cases. It also
checks that every per-arch win/linux publishing script keeps the guard on
the same arch and dist dir, so a new or edited release script cannot drop
it unnoticed. That check fails against the current develop package.json.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since #9304 the desktop clients call autoUpdater.setFeedURL with
channel `latest-${process.arch}` on Windows and Linux (and `latest` on
macOS), and electron-updater's GenericProvider appends the platform
suffix to that channel. A folder chosen for the "Local Server" update
option is therefore queried for latest-x64.yml / latest-arm64.yml on
Windows, latest-x64-linux.yml / latest-arm64-linux-arm64.yml on Linux
and latest-mac.yml on macOS.
TIMER_TRACKER.SETTINGS.LOCAL_SERVER_NOTE still told users to provide
latest.yml, which the client never requests on any platform. A folder
prepared that way fails with ERR_UPDATER_CHANNEL_FILE_NOT_FOUND on
Windows and Linux.
Rewrite the note in all 13 locales to list the manifest per platform
and architecture and to mention the update files the manifest lists.
Only this value changes in each file; key order, indentation and line
endings are untouched.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`"channel": "latest-x64"` is `pack --arch` output that was committed by
accident in 2c28fa70ae. The other 5 desktop apps keep build.publish
channel-less in source and let `pack --arch --platform` stamp
latest-<arch> at build time (win32/linux only). In CI the channel is
overwritten anyway, but a local or manual macOS build of Gauzy Desktop
would publish latest-x64-mac.yml, which the updater never requests on
darwin (it asks for latest-mac.yml).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Moving `lint` to `RUNNER_LINUX_X64_4` left its Configure Registry step's
`expect-vip` still derived from `RUNNER_LINUX_X64_8`. Flagged by both
greptile-apps and coderabbitai on line 74.
`expect-vip` tells the pinned configure-registry action (aa4ee199) that the
runner is in-network: it probes the Verdaccio VIP twice instead of once and
emits "Verdaccio VIP unreachable from an in-network runner" on failure. Either
way the job falls back to public npm and never fails - it is a diagnostic.
Every other job in the repo derives `expect-vip` from the same variable as its
`runs-on` (all nine x64-8 jobs read x64-8; the matrix release jobs use
"self-hosted or ever-k8s"). `lint` was the only mismatch, introduced by the
previous commit. This restores the invariant.
Took greptile's fix (derive from x64-4). Did NOT take coderabbit's literal
`false`: x64-4 is an in-network ARC runner, so `false` would make `lint` the one
self-hosted job with no retry and no warning, turning a registry outage on that
lane into a silent single-probe fallback.
No behaviour change today, since both variables are set org-wide; this closes
the case where x64-4 is configured and x64-8 is not.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The `lint` job is report-only (`continue-on-error: true`) and runs
`nx run-many -t lint --parallel=2`, so it never uses more than two parallel
tasks and gains nothing from the x64-8 lane's 12-core limit. Its sibling in the
same workflow, `typecheck-configs`, already runs on x64-4.
The x64-8 lane is capped at 12 runners and each one requests 28 Gi, against
12 Gi for x64-4. It was observed saturated at 12/12 on 2026-09-22 while Docker
image builds queued behind it. Moving a job that cannot use the extra cores
frees a slot and 16 Gi of requested memory for the builds that can.
Deliberately NOT moved:
- unit-tests: jest forks workers per project, so fewer cores would slow a job
that already takes 123-233 minutes and risk its 360-minute timeout.
- the e2e suites (playwright / cypress / currents): timing-sensitive, and a
slower runner makes that class of test flakier, not just slower.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Gauzy Server's Windows and Linux release jobs have published channel-less
update manifests (latest.yml, latest-linux.yml, latest-linux-arm64.yml)
since v107 (the :isolated split in #9687). The other 5 apps, and Gauzy
Server up to v102, publish latest-x64.yml, latest-arm64.yml,
latest-x64-linux.yml and latest-arm64-linux-arm64.yml.
Cause: `pack --arch --platform` stamps build.publish[].channel =
latest-<arch> into apps/server/src/package.json, but the :isolated scripts
run it AFTER "Build Server" has already copied apps/server/src/** into
dist/apps/gauzy-server. electron-builder --project dist/apps/gauzy-server
therefore never sees the channel and uses the default `latest`. On Windows
both arch jobs then write latest.yml, and the last writer wins (arm64 in
v111.44.45).
Impact: the updater client (desktop-lib update-strategy, since #9304)
requests latest-${process.arch}, so current Gauzy Server installs have got
a 404 on every check and have not auto-updated since v107.
Fix: copy the freshly stamped package.json into dist between pack and
electron-builder in the 4 build:gauzy-server:{linux,windows}:release:gh:
{x64,arm64}:isolated scripts (-E makes a missing source fail loudly). The
dist file otherwise equals src byte for byte, so the only field that
changes is build.publish[0].channel. The mac script is unchanged
(latest-mac.yml is correct).
Verified by simulation with a red control, using the real pack util,
copyfiles 2.4.1 and electron-builder 26.0.3 filename code plus a real NSIS
build. Old: latest.yml for both arches. New: latest-x64.yml and
latest-arm64.yml on Windows, latest-x64-linux.yml and
latest-arm64-linux-arm64.yml on Linux.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On arm64 electron-builder has no template snap, so it builds without one:
snapcraft pulls stage-packages and, in host (destructive) mode, runs a bare
`apt-get update`. As the unprivileged runner user that fails:
E: Could not open lock file /var/lib/apt/lists/lock - open (13: Permission denied)
Failed to refresh package list: failed to run apt update.
amd64 never hits this because it uses the template snap and runs no apt,
which is why "snap worked before" held only for amd64.
Add a PATH shim in every release-linux-arm64 job that runs ONLY snapcraft
as root, then chowns what it wrote back to the runner user. Every other
target (deb/rpm/AppImage/flatpak/tar.gz) is untouched.
Proven on ubuntu-24.04-arm with an electron@38.2.2 / electron-builder@26.0.3
harness using the apps' exact snap config (base core22):
without the shim: 12/12 jobs fail at "pull app" (runs 35896229973,
35902920526, 35903381014, 35904503217)
with the shim: 2/2 jobs produce the .snap, owned by runner (run 35905018543)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The linux-arm64 snap target has failed intermittently since June 2026 with
Error installing snap 'gnome-3-28-1804' from channel 'latest/stable'
-> ERR_ELECTRON_BUILDER_CANNOT_EXECUTE
while x64 passed. In host (destructive) mode snapcraft installs any build snap it is
missing by running `snap install` as the unprivileged runner user, and on the arm64
images snapd intermittently refuses that:
as the runner user: error: access denied (try with sudo)
with sudo: installed
Proven on branch exp/arm64-snap-snapcraft-version with a minimal Electron app using
the same electron-builder 26.0.3 / Electron 38.2.2 / snap base core22:
- Round 1 (35895302909): arm64 failed on all four snapcraft sources, x64 passed on
all four - so NOT the snapcraft version, and not the runner (the identical error
hit ubicloud arm64 in July), nor host mode (on when arm64 last passed in May).
- Round 2 (35895798132): captured snap's own refusal above; also showed the denial is
intermittent, and that x64 does NOT have the snaps pre-installed - snapd simply
authorises the runner user there.
- Round 3 (35896229973): 4 baseline vs 4 pre-install replicas. Every baseline run
made snapcraft perform its own `snap install` (the operation that gets denied);
every pre-install run made ZERO. The failing path is therefore never reached -
a fix by construction, not by pass rate.
Adds one step before Multipass in the release-linux-arm64 job of all 12 app
workflows (6 prod + 6 stage, identical text, so prod/stage parity is preserved),
installing exactly the list that was proven. x64 jobs are untouched.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
agent-prod.yml said "Bump agent version" while agent-stage.yml said "Bump Agent
version". Display-name only, zero behavioural effect - but it was the single
remaining non-environment difference between the pair, so the two files now differ
by nothing except genuine environment values (branch, tag, prerelease flag).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The `-prod.yml` and `-stage.yml` variants of the six app workflows had drifted:
fixes were applied to one variant and never mirrored to its sibling. This is FILE
drift, not branch drift - both files are identical on develop/stage/master/apps -
so a prod release simply never got work that stage had, and vice versa.
Ported stage -> prod (prod was missing all of this):
1. macOS signing. `Import Apple signing certificate into a keychain` plus the
switch from CSC_LINK to CSC_KEYCHAIN. app-builder-lib passes the p12 password
to `security set-key-partition-list -k`, which wants the KEYCHAIN password, so
every prod mac build died on `SecKeychainUnlock: the user name or passphrase
you entered is not correct`. The credentials were always correct - stage proved
it by building green on 2026-09-21 with the same secrets.
2. Windows code signing, entirely. The Bump-step group (WINDOWS_PUBLISHER_NAME,
AZURE_CERT_PROFILE_NAME, AZURE_CODE_SIGNING_ACCOUNT/ENDPOINT) that makes the
bump script emit build.win.azureSignOptions, and the Build-step group
(WIN_CSC_LINK, WIN_CSC_KEY_PASSWORD, AZURE_*). Prod Windows installers have
been shipping UNSIGNED, and with no publisherName in package.json,
electron-updater signature verification was off for the prod channel only.
3. `Install .NET SDK (required by Azure Trusted Signing)`. Without it
Invoke-TrustedSigning reports sdk-not-found, SKIPS signing, and the job goes
GREEN with an unsigned artefact - so fixing 2 without this would have looked
successful and still shipped unsigned.
4. The ARM64 Visual Studio Build Tools guard: vswhere probe, conditional choco,
tolerant exit. Prod had a bare `choco install` that dies on a chocolatey 504.
Ported prod -> stage (stage was missing these, so stage could not reproduce the
prod Linux packaging path):
5. snapcraft pinned to 7.x/stable. 8.0+ renamed `snap` to `pack` while
app-builder still calls `snapcraft snap`.
6. The Python 3.11 pin for native rebuilds, with npm_config_python/PYTHON exported.
ARM64 Windows signing was deliberately NOT added: stage omits it on purpose
(Microsoft.Trusted.Signing.Client ships only bin/x64 and bin/x86), and that
reasoning is carried across in the comments rather than re-derived.
Deliberately NOT changed: docker-build-publish-prod.yml resolves the version from
a tag pointing AT the built commit with no `git describe` fallback. An audit
flagged that as missing stage's two-tier resolution, but the code documents it as
intentional - a prod image must carry a real release tag or none, never
"v111.32.3-4-gbb20466". The nine deploy/release pairs have no drift at all.
Verified: all 12 files parse under js-yaml; every pair now has identical job and
step counts (100/100, 110/110, 98/98); all 7 feature markers match across all six
pairs; no bare `CSC_LINK:` remains anywhere; zero CRLF; exactly 12 files touched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`release-linux-arm64` is the only app job whose matrix hardcodes a runner with no
`vars.` fallback: `os: [ubicloud-standard-8-arm]`. Every sibling job uses the
`${{ vars.X || 'default' }}` form. ever-co's ubicloud ARM capacity is gone, so the
job never gets a runner - all six app workflows on the 2026-09-22 `apps` promotion
sat queued from 23:12Z with 0/6 arm64 builds.
`timeout-minutes: 300` does NOT rescue this: that clock only starts once a job gets
a runner. An unstarted job waits until GitHub cancels it at ~24h, so the RUNS never
reach a terminal state either, and the run-level conclusion becomes meaningless.
Now `${{ vars.RUNNER_LINUX_ARM64 || 'ubuntu-24.04-arm' }}`, matching the shape of
the x64 line two jobs above it. The variable is not currently set at repo or org
level, so this resolves to GitHub's hosted `ubuntu-24.04-arm` - free for public
repositories, and ever-gauzy is public. Setting RUNNER_LINUX_ARM64 later moves every
one of these jobs to a self-hosted pool without another code change.
Applied to all six *-prod.yml and all six *-stage.yml so stage-apps cannot hit the
same wall. The job is repointed, not removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cspell runs with strict: true on changed files and rejected 'recognisable' and
'customisation'. Fixing my own prose rather than widening the repo dictionary.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`RolePermissionUtils.migrateRolePermissions` is what the `*RolePermissionsReload*`
migrations run, and those execute inside `NestFactory.create()`, before
`app.listen()`. It asked the database "does this role already have this
permission?" once per permission - `roles x permissions` sequential round-trips.
On production that is 41,885 roles x 211 permissions = ~8.8M queries. Measured at
16.3 tenants/min it needs ~5h20m, against a startupProbe budget of 50 minutes
(failureThreshold 300 x periodSeconds 10). So `DocumentsRolePermissionsReload1790000003000`
was SIGKILLed (exit 137) at 50m26s and restarted from zero, forever, and
ever-gauzy-prod could never boot v111.44.21. `PayrollRolePermissionsReload1790000013000`
is queued behind it with the same cost. Stage never caught it because stage has
422 tenants to prod's 5,236.
Now each role's existing permissions are read in ONE query into a Set, and the
missing rows are inserted in batches sized to stay inside the strictest driver's
bind-parameter ceiling. Round-trips drop from `roles x permissions` to roughly
`roles`, so the same work takes about two minutes instead of five hours.
Behaviour is unchanged: still INSERT-only, so re-running never disables or removes
an existing grant; same columns, same `enabled` semantics, and the explicit id is
still generated for MySQL/SQLite via the existing `getInsertPayload`.
`checkPermissionExistence` and `insertRolePermissions` are retained.
Verified three ways:
- The batched read was proven equivalent to the old per-permission probe against
the real production dataset: 0 differences across 107,133 (tenant, role,
permission) pairs from a 500-role sample, with a deliberately-broken control
showing 500 differences so the comparison is known to discriminate.
- New suite `utils.spec.ts` runs the migration against a real better-sqlite3
database and asserts round-trip COUNT, not just the resulting rows.
- That suite was run against the pre-fix implementation as a red control: its
three query-count tests fail there, with the per-permission probe firing 5,064
times (24 roles x 211 permissions), while the three row-correctness tests still
pass - the rows were always right, only the round-trips were fatal. The
idempotence case runs 17,647ms pre-fix against 107ms after.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The owner's instruction was that a pull request into `develop` must not start a workflow — the cost
belonged to every commit on the branch being merged, and the run belongs to the merge. Every other
workflow that fired on such a pull request was changed on `feat/platform-extensions`; this one could
not be, because `codeql.yml` exists only on `develop`.
Measured before the change, over the previous twelve CodeQL runs: **178 run-minutes in total, 137 of
them on pull requests**, at two jobs of 22–45 minutes each on the 4-vCPU ARC pool. It is paid on every
contributor's branch into develop, not on one: `feat/platform-extensions` (44.5 and 29.0 minutes),
`develop` (22.2), `fix/custom-dashboard-drag-and-drop`, `feat/9873-restrict-agent-exit-logout`.
What stays: the `push` trigger still scans `develop`, `stage` and `master`, so code that lands on any
of them is analysed — the run moves to the merge rather than disappearing. The weekly `schedule` scan
and `workflow_dispatch` are untouched, the `concurrency` block is untouched, and pull requests aimed
at `stage` and `master` — the release cascade — still run it.
`branches-ignore` rather than a `branches` list, because a list is what has to be edited when a new
protected branch appears and this is the exception to it. `build.yml` and `secrets-analysis.yml`
already spell their develop exception this way. The block carries the reversed spelling in a comment,
so restoring the trigger is one paste.
The trade-off, stated plainly: a pull request into `develop` is no longer scanned *before* merge. The
security signal moves to the push run on develop, which is after merge. If pre-merge scanning on
develop is wanted more than the runner time, this change should be closed rather than merged — the
alternative that keeps both is to leave the trigger and accept roughly one 30-minute run per push.
Back-merge to flatten the stale topology between master and develop.
Verified before merging: develop already contains 100% of the content unique
to master, so this merge changes no files -- its tree is byte-identical to
develop's pre-merge tree (8aad725eae).
master's only changes since the merge base (5c9b18cbd0) were 34 files:
.github/workflows/docker-build-publish-prod.yml
#9890 ARC runners (vars.RUNNER_LINUX_X64_8) -- already in develop
#10262 DO_ENABLED gates on the 9 DigitalOcean steps -- already in develop
develop is a further 67 lines ahead here: least-privilege GITHUB_TOKEN
scope, release-version stamping, VERDACCIO/NX tokens moved to BuildKit
secrets, Verdaccio URL from an org var.
packages/plugins/legal-ui/** (19 files)
#9884 serve ToS/Privacy/Cookies from the bundled corpus, not iubenda.
Already in develop via b304d07b91, plus the separate cookies route, the
min-width:0 canvas fix and tags:["type:plugin"].
packages/ui-core/i18n/assets/i18n/*.json (14 locales)
LEGAL.DOCUMENT_VERSION / LEGAL.DOCUMENT_EFFECTIVE_DATE -- already present
in all 14 locales with matching translations.
Both merge conflicts (privacy-policy.component.html and .ts) are resolved
wholesale in develop's favour: develop carries route-driven section selection
(showPrivacy / showCookies / resolveSections) that master predates.
No cascade to master; this merge releases nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(security): scope the official-holiday by-id routes to the caller's organization
The listing was fixed in d51b816c6e, but GET, PUT and DELETE /official-holiday/:id
were left on the bare TenantAwareCrudService methods, which scope to the tenant and
stop there. A tenant holds many organizations, and role permissions are keyed by
(tenantId, roleId) with no organization dimension, so TIME_OFF_POLICY_VIEW / _EDIT /
_DELETE carry across every organization of the tenant: a holder could pass the id of
a sibling organization's holiday and read, change or delete it. PUT was the worst of
the three, since the update body may also carry organizationId and would have
re-parented the foreign row into the attacker's own organization.
The by-id routes name no organization, so there is nothing for a request DTO to
validate — the organization has to be read off the STORED row. findOneByIdString now
resolves the holiday (already tenant-scoped, already 404s on a miss) and runs it
through assertCurrentUserBelongsToOrganization, the same helper the listing uses.
TenantAwareCrudService.update() resolves its row through that method before it
writes, so PUT is covered by the same check without a second membership lookup;
delete() never reads the row, hence its own override. The happy path costs exactly
one membership lookup per request.
Two fail-closed details: a row whose organizationId is null is refused with 403
rather than the helper's "required" 400 — the request is well formed, it is the row
that belongs to no organization — and organizationId is read with the relation as a
fallback, because MikroORM maps the relation-id mirror to persist: false and does not
always hydrate it.
Eight specs cover read, update and delete against a sibling organization, the
re-parenting body, the null-organization row, the unhydrated mirror and the missing
user; the six refusal cases pass against the previous service only because it never
refused at all.
One behaviour change: DELETE of an id that does not exist in the caller's tenant now
returns 404 instead of 204, because the pre-read throws. The route already documents
NOT_FOUND, it is @HttpCode(NO_CONTENT) so the { affected: 0 } result was never
serialized to the client, and no caller of the route exists in the monorepo.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(security): carry the authorized organization into the holiday writes
Review follow-up: the membership check added in the previous commit is an unlocked
read, and both writes then ran on predicates that did not reassert it —
CrudService.update() writes by RAW id (no tenant predicate, let alone an
organization one) and TenantAwareCrudService.delete() merges only the tenant
conditions. A holiday re-parented between the check and the write would therefore
still be written by a caller who no longer had any claim on it.
Both writes now name the organization the caller was authorized against:
- update() passes an object criteria, so TenantAwareCrudService.update() resolves it
through findOneByWhereOptions() — which throws NotFoundException when nothing
matches — and the same predicate lands in the UPDATE's own WHERE. That rejects the
raced write without reading an affected count, which under MySQL reports CHANGED
rows and would 404 a legitimate no-op PUT.
- delete() passes it through the options.where hook the base class already exposes,
and treats an explicit zero-row result as "not yours any more". Only an explicit
zero: a driver that reports no count at all must not turn a real delete into a 404.
The organization resolution moves into a private authorizedOrganizationOf(), which
returns the id the write is then pinned to, so the check and the predicate cannot
drift apart. update() now has an explicit override rather than inheriting the check
through the base class's pre-read, and still costs exactly one membership lookup.
Four more specs: the update happy path pinning the criteria, the delete happy path
asserting the organization reached the DELETE, and a raced update and delete that
both 404.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(security): 404 a holiday update that matched nothing, without trusting the count
Review follow-up: the organization predicate on the UPDATE means a holiday
re-parented after `findOneByWhereOptions()` has read it is simply not matched, but
the method still returned that `UpdateResult` instead of raising the 404 its docblock
promises.
A bare `affected === 0` check is not safe here. MySQL is a first-class
`DatabaseTypeEnum` in this repo (mysql2, and nothing sets `CLIENT_FOUND_ROWS`), and
MySQL reports rows CHANGED rather than rows matched — so a PUT that writes the values
a row already holds reports zero too, and would have started 404ing.
The zero case is therefore resolved by asking rather than by inferring: a single
`findOneByWhereOptions({ id, organizationId })`, which raises the NotFoundException
itself when the row is no longer ours and returns normally when the update was just a
no-op. It costs a query only when the count is zero, and it is correct on both
drivers.
Two specs: a holiday that moved after the re-read 404s, and an update that changed
nothing resolves.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* ci: stop concurrent CodeQL and duplicate gate runs on PRs and develop/stage
CodeQL ran through GitHub's code-scanning DEFAULT SETUP, which generates its
workflow server-side (run path `dynamic/github-code-scanning/codeql`). There is
no file to put a `concurrency:` block in, and GitHub does not supersede its own
default-setup runs, so every push to a pull request started another full
analysis alongside the ones already in flight. Measured on #10254 on
2026-09-21: runs 35547460875 and 35548451790 overlapped by 12 minutes, and
35580875040 / 35582701347 / 35582925033 were all running at 09:22. Each run is
two jobs of 25-35 minutes on the 4-vCPU ARC pool.
Converts CodeQL to advanced setup with a real workflow file that reproduces the
default-setup configuration exactly - the two analyses it actually ran
(javascript-typescript, actions; the other three listed languages are aliases),
the default query suite, the default remote threat model, the weekly schedule,
and the ever-k8s-linux-x64-4 pool - and adds the cancel-in-progress policy that
default setup could not have. Branch coverage stays at develop/stage/master
because all three are protected and default setup was scanning all three.
Also closes the remaining concurrency gaps on the PR and develop/stage surface:
- build.yml and secrets-analysis.yml both pair an unfiltered `pull_request:`
with a `push:` on the same branches, so while a release-cascade PR is open
(today #10257, stage -> stage-apps) one push fires two runs of the same tree
in two groups that could never cancel each other. Keying the group on the
head ref collapses them; qualifying it by head repository keeps a fork branch
of the same name from claiming the group and cancelling a real build before
its job skips.
- harvest-secrets-to-openbao.yml was the only workflow of 70 with no
concurrency block, and two overlapping dispatches can interleave into a torn
OpenBao snapshot.
- external-uptime-monitor.yml now cancels superseded PR self-tests while still
never cancelling a live probe, which owns the alert issue.
Deliberately unchanged: the deploy and release workflows keep
`cancel-in-progress: false`, and test-unit.yml keeps it on the stage gate. Each
documents an incident where cancelling caused real harm.
Requires default setup to be disabled first - GitHub rejects CodeQL SARIF
uploads from a workflow while default setup is enabled.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* ci: collapse duplicate CodeQL runs and teach cspell the OpenBao name
Two review follow-ups on this branch.
Greptile P2: codeql.yml had the same duplicate-run defect this PR fixes in
build.yml and secrets-analysis.yml. Both of its triggers cover
develop/stage/master, so while a cascade pull request is open (develop ->
stage), one push to develop fires a push run on refs/heads/develop and a
pull_request run on refs/pull/N/merge. Keyed on github.ref those are different
strings, so cancel-in-progress could never collapse them and the same tree was
analysed twice. Now keyed on the head ref and qualified by head repository, the
same shape used in the other two files.
Cspell: this PR is the first change to harvest-secrets-to-openbao.yml since the
spellcheck was added, and the action only scans changed files - so `openbao` had
never been checked before and surfaced as an unknown word. Added to .cspell.json
next to the other product names.
Not taken: CodeRabbit asked for the actions in codeql.yml to be pinned to commit
SHAs. Declined as inconsistent rather than wrong - this repository tag-pins 547
`uses:` references against a handful of SHA pins, and there is no dependabot
config to keep SHAs current, so pinning one new file would leave an outlier that
nothing updates. Worth doing repo-wide as a deliberate policy, not here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* ci: use US spelling in the codeql concurrency comment
Cspell flagged `analysed` at codeql.yml:60. This repository is US English
throughout (the job is named `Analyze`), so the comment is corrected rather
than the word added to the dictionary.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
All three prod image jobs (gauzy-api, gauzy-webapp, gauzy-worker) have failed at
step #9 'Install doctl' since 2026-08-03. 'Push to Github Registry' sits below
those steps in the same job, so the failure aborts the job before GHCR is ever
reached. The result is that ghcr.io/ever-co/gauzy-api:latest has not been
republished since 2026-07-04 - production in both ever-gauzy-prod and
ever-teams-prod has been running an 11-week-old image that no redeploy can
refresh.
This gates the nine DigitalOcean steps (3 jobs x Install doctl / Log in to
DigitalOcean Container Registry / Push to DigitalOcean Registry) behind
vars.DO_ENABLED, using the exact form already used in
docker-build-publish-stage.yml. The steps are gated, not deleted, so re-enabling
DigitalOcean is a single org-variable change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The first pass left `DEMO=true` on the historical path — an unset secret fell
back to the literal published in this repository — because it could not be
verified whether demo.gauzy.co relied on that fallback, and develop deploys
straight there.
It does not. Every deployment in the fleet sets all four secrets explicitly
(JWT_SECRET, JWT_REFRESH_TOKEN_SECRET, JWT_VERIFICATION_TOKEN_SECRET,
EXPRESS_SESSION_SECRET are 64-96 bytes in ever-gauzy dev/stage/prod and
ever-teams dev/stage/prod), so the exemption protected nothing here while
leaving any other DEMO deployment signing tokens with a key anyone can read in
this repository — the hole the advisory describes.
DEMO now behaves like every other environment: an explicitly set secret is used
as-is, an unset one gets the per-process random value, and the startup guard
still reports that as unconfigured. `resolveSecret()` loses its second argument,
which only existed to carry the literal it was allowed to return.
The spec's DEMO block now asserts the opposite of what it used to, with a
control arm that shows a token forged with the published key verifying against
the old fallback and failing against the new value.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The three media plugins let a caller read another employee's recordings, with only TIME_TRACKER —
a default EMPLOYEE permission.
- The list handlers pinned `uploadedById` to the caller and then spread the client's `where` OVER it
(`where: { ...where, ...params.where }`), so `?where[uploadedById]=<someone else>` won. The server's
keys now go last, which also stops a client-supplied `tenantId`/`organizationId` from replacing the
resolved ones.
- The by-id handlers were tenant-scoped only. These entities carry `uploadedById`, not `employeeId`,
so the per-employee restriction in TenantAwareCrudService never applied to them. They now go through
`assertCallerOwnsUpload` in core, shared by all three plugins rather than copied into each: it
answers 404 for a record the caller did not upload, so the id of someone else's recording stays
unconfirmed, and leaves CHANGE_SELECTED_EMPLOYEE holders alone.
Specs: the ownership rule in core, and per plugin the merge order, the tenant/organization keys and
the delegation. Camshot's tsconfig.lib.json now excludes spec files, as the videos and soundshot ones
already did.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- BILLING_RATE_MAX is now 2147483647, the old integer column's maximum, so every accepted rate
stays revertible: rolling the columns back to integer cannot overflow on a value saved after the
upgrade (Greptile P1). numeric(14,2) stays wider; the cap can be raised without a migration.
- ValueTransformerType supports TypeORM's ValueTransformer[]: `to` in array order, `from` reversed,
matching ApplyValueTransformers. Calling .to()/.from() on the array threw before (CodeRabbit).
- The wrapper and the property now report the same DDL, including an explicit columnType, and
declaredColumnType keeps length and precision-only modifiers (CodeRabbit).
- Real MikroORM round trip on better-sqlite: generated DDL, write conversion, hydration and null
handling for a numeric and an int-backed enum column (Greptile P2). Verified by mutation: it
fails without the bridge.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`deleteScreenshot` limits a caller without CHANGE_SELECTED_EMPLOYEE to their own screenshots by
joining the owning time slot. The TypeORM branch used a LEFT join, which keeps the row when its ON
clause does not match, so the ownership condition removed nothing: the effective filter was id +
tenant + organization. DELETE_SCREENSHOTS is a default EMPLOYEE permission, so any member of an
organization could delete a colleague's screenshot, and with `forceDelete` the stored image as well.
Ids come from any of the list endpoints.
- INNER join instead, so a screenshot whose slot belongs to someone else cannot match.
- A caller with no employee identity and no CHANGE_SELECTED_EMPLOYEE now gets 403 instead of having
the ownership predicate quietly dropped; the same guard is applied to the MikroORM branch, where a
null id would otherwise have searched for slots with a NULL employeeId.
- Spec covers all three cases; two of them fail on the previous code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`GetOrganizationTeamStatisticHandler` flattened every error into
`BadRequestException('Failed to execute organization team statistic query')`. The sensitive-relation
check answers 403 from inside that call, so a caller probing
`GET /organization-team/:id?relations[]=members.employee.timeSlots.screenshots` was told the request
was malformed rather than forbidden — the data was blocked either way, but the reason was masked, and
so was every other deliberate HTTP answer from below.
An `HttpException` now passes through unchanged; anything else still becomes the generic 400.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The per-employee restriction in TenantAwareCrudService applies to the ROOT entity of a read, so a
client-supplied `relations` that starts from a row the whole organization may read walks straight
into everybody's tracked data. Both of these are reachable with default EMPLOYEE permissions
(ORG_TASK_VIEW, ORG_TEAM_VIEW) and were reproduced against a booted API:
GET /tasks/:id?relations[]=timeLogs.timeSlots.screenshots
GET /organization-team/:id?relations[]=members.employee.timeSlots.screenshots
- `TRACKED_DATA_SENSITIVE_RELATIONS` names the hops that cross from a shared entity (Employee, Task,
OrganizationProject, OrganizationTeam) into tracked data, and requires CHANGE_SELECTED_EMPLOYEE for
them. The existing walk already gates by the entity a relation is loaded from, so it needed only to
arm this table per hop, merged with whatever the organization table declared — `employees.user`
keeps needing ORG_USERS_VIEW.
- `TimeLog.timeSlots` and `TimeSlot.screenshots` are deliberately NOT gated: those reads start from a
row already scoped to the caller's own employee id (the desktop retry queue, the screenshot modal),
and reaching someone else's slot needs one of the hops above first.
- `GET /timesheet/timer/status/worked` may legitimately name a teammate, so the table cannot help
there: it narrows the requested relations instead, dropping the tracked-data ones for a caller
without CHANGE_SELECTED_EMPLOYEE. Narrowing rather than refusing keeps existing clients working.
Specs cover the three exploited paths, the admin bypass, the self-service reads that must keep
working, a same-named relation on an unrelated entity, and the narrowing helper.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`getTrackingSessions` took the `employeeIds` of the request as given, with no permission and no
manager check, and the controller gate is OR-semantics over TIME_TRACKER / ALL_ORG_EDIT /
ALL_ORG_VIEW. Every default EMPLOYEE holds TIME_TRACKER, so
`GET /timesheet/custom-tracking/sessions?employeeIds[]=<colleague>&includeDecodedData=true` returned
that colleague's sessions and their decoded payloads. Reproduced against a booted API.
The ids now go through `ManagedEmployeeService.filterAccessibleEmployeeIds`, the same gate the
time-log and statistics reads use, with the request's team and project scope passed along so a
manager keeps their reach. CustomTrackingModule imports EmployeeModule for it, the way TimeLogModule
already does.
The `#10212` guard on these routes does not help here: it enforces an organization setting, it does
not scope by employee.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(security): gate inherited mutating routes and close authorization residuals
GHSA-v79w-54p2-wmh5 (high): CrudController's inherited POST/PUT/DELETE/soft/
recover routes carry no permission metadata, and PermissionGuard passes when the
metadata is empty, so an EMPLOYEE could soft-delete users, contacts and tags.
Those routes are now gated, and a repo-wide spec walks every controller in core
and the plugins (966 mutating routes) and fails on any ungated one unless it is
explicitly opted in: 34 deliberately open routes, each with the service-side
check that covers it, plus a frozen ratchet list of pre-existing bare routes that
may only shrink. crud.service softRecover also passed no withDeleted, so every
inherited recover route answered 404 for the row it exists to restore.
GHSA-c3cj-m3xm-7j5h (high): the organization-contact lookup built its joins
straight from client-supplied relations, bypassing the sensitive-relations
interceptor. It now asserts permissions and resolves through an allow-list, and
pins the employee to the caller unless they may change the selected employee.
GHSA-44pv-34gx-q9p4 (medium) residuals: validators that fell open on an undefined
key now fail closed, a DTO carrying both organization and organizationId
validates both instead of neither, and email-template listings and writes are
pinned to the caller's tenant plus the global templates.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(cspell): add the new vocabulary and use US spellings
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(security): close the review findings on the authz residuals
Review pass over PR #10242 (CodeRabbit, Greptile, SonarCloud). Two of the
findings were real gaps in the fixes this PR exists for.
@Roles() is not a gate on its own (CodeRabbit + Greptile, major)
The repo-wide mutating-route scan accepted any @Roles() decorator as
authorization. @Roles() only calls SetMetadata, and RoleGuard is NOT
registered as an APP_GUARD (app.module.ts provides only the throttler),
so a handler declaring roles without @UseGuards(RoleGuard) reads as
role-gated while nothing ever consults those roles — exactly the kind of
ungated mutating route the scan exists to catch. The scan now requires an
effective RoleGuard, on the handler or on its controller, before it counts
@Roles() as a gate; @Public() is unchanged. Every @Roles() in the tree
today is already paired with @UseGuards(RoleGuard), so the ratchet lists
are unchanged, and a new in-memory test pins both shapes.
An organization named as a bare id string skipped the employee check
(CodeRabbit, major) EmployeeBelongsToOrganizationConstraint resolved the
organization by reading `organization.id` only. A DTO that does not extend
TenantOrganizationBaseDTO has no @IsObject() to refuse a string — e.g.
EmployeeRecurringExpenseQueryDTO, which intersects EmployeeFeatureDTO
alone, behind GET /employee-recurring-expense/month — so
`?organization=<uuid>&employeeId=<foreign>` left the id unresolved and fell
through to the deliberately permissive "no organization named" branch. Both
shapes now resolve to the same value, so the membership lookup runs with
the id the request actually named.
MikroORM findAll could read NULL-tenant rows of an organization (CodeRabbit)
`tenantId: mTenantId ?? null` kept the tenant arm alive without a tenant in
context, and MikroORM compiles a literal null to IS NULL, so the arm matched
every NULL-tenant row rather than nothing. The tenant arm is now built only
when there is a tenant, which is what scopeEmailTemplateWhere already did
for the pagination route.
CreateEmailTemplateDTO declared only organizationId (CodeRabbit)
name, languageCode and hbs are NOT NULL on email_template, so a create
missing one could never persist; declaring them turns a database error into
a 400 and lets Swagger publish the real create schema. The route now
validates with whitelist, so an undeclared body key cannot reach
persistence at all — stripEmailTemplateScopeFields stays as the explicit
statement of which fields are scope fields.
Also the five new SonarCloud code smells: braces around the two switch cases
in EmailTemplateService.findAll (lexical declarations in a case block),
`IFindOneOptions<T> | unknown` collapsed to `unknown`, two useless `?? {}`
spreads, and toHaveLength in the opt-in-list assertion.
Refused: nothing. No finding asked for a protection to be weakened.
Tests: packages/core src/lib/{shared/validators,shared/guards,core/crud,
core/dto,email-template,organization-contact,employee-recurring-expense/dto}
-> 15 suites, 212 tests, all passing (was 13/191 plus one non-compiling suite).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(test): null-check request.query consistently in the guard control arm
DeepScan INSUFFICIENT_NULL_CHECK on
organization-permission.guard.spec.ts:677 — the CONTROL arm replays the
pre-fix `extractRequestOrganizationId` with optional chaining on
`request.query`, then asserted on `request.query.organizationId` without it.
The object is a local literal so nothing could have thrown, but the two
readings of the same expression disagreed. Both now use `?.`, which keeps
the replay a verbatim copy of the pre-fix production code.
No behaviour change; the suite still passes (45 tests).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(email-template): validate languageCode against LanguagesEnum
Second CodeRabbit pass, on the DTO this PR just added.
- languageCode was `@IsString()`, so `xx` was accepted and persisted. Every
reader of the column looks a template up by a LanguagesEnum value (the
seeder, saveTemplate, the mailer) and the column has no enum constraint, so
such a row is one nothing can ever find. Now `@IsEnum(LanguagesEnum)`, with
a test for a rejected code and an accepted non-default one.
- The whitelist regression test passed vacuously: the fixture carried no
tenant keys, so it would have stayed green even if the DTO later declared
them. It now puts `tenant` and `tenantId` in the input, asserts they are on
the transformed instance, runs the same whitelisting `validate()` the pipe
runs, and asserts only the four template fields survive.
packages/core src/lib/email-template/dto -> 1 suite, 8 tests, passing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(email-template): answer create with the stored row, not the request body
CodeQL flagged the create route as reflected XSS: it returned the body it had
just persisted. Not exploitable — the response is JSON and helmet sets
X-Content-Type-Options: nosniff — but echoing the request buys nothing. The
route now reads the row back through the tenant-scoped lookup, so the client
sees the scope fields the server pinned rather than the ones it sent.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(security): check ownership of nested relations, not just the root row
GHSA-jh6m-9fxr-rx3c (high): TenantAwareCrudService verified the tenant of the row
being written but not of the rows named inside it, so PUT /invoices/:id carrying
invoiceItems with another tenant's item id re-parented or overwrote that row, and
PUT /candidate/:id could cascade into another user's credentials. A new
assertGraphNotForeign() walks the payload's relation graph (depth-bounded, one
batched SELECT per relation), refuses foreign-tenant rows through any relation,
refuses re-parenting a global NULL-tenant row, stamps the caller's tenant on
cascaded inserts, and strips credential fields from a nested existing User.
Candidate updates no longer cascade into User at all.
Also closes the named siblings: request-approval's raw employee and team lookups
are tenant-scoped (GHSA-gwpq-mmw7-vx85); UpdateTimeSlotHandler, the bulk activity
save and the time-log routes are tenant-scoped, whitelisted and no longer accept
a foreign employeeId, and the organization policy guard checks the target's
tenant for SUPER_ADMIN too (GHSA-6qvm-3wg4-26w4).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(cspell): add the new vocabulary and use US spellings
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(security): link nested users instead of writing them, fail closed on the report queries
Review follow-up on the nested-relation ownership work. Six findings held up when
checked against the code; each protective line has a spec arm that fails when the
line is reverted.
- nested-graph: an EXISTING `User` reached through a cascade is now reduced to
`{ id }`. Removing only the credential columns still let `user: { id, firstName }`
rename any account of the caller's own tenant, which a tenant check cannot catch —
the victim is a legitimate member of that tenant. A NEW user inserted through a
cascade (candidate sign-up) is untouched. CandidateCreateHandler re-attaches the
user it just created to its response, so POST /candidate still answers with the
user the UI prints.
- nested-graph: a payload that would still persist rows BELOW the walk limit is now
refused (400) instead of being waved through. The row sitting at the limit was
checked, but its own relations were not walked, and `save()` would still cascade or
re-parent them with no ownership check at all.
- user create: `UserCreateCommand` drops a body-supplied `id`. `CrudService.create()`
upserts when the payload carries a primary key, so a body id turned "create a user"
into "overwrite that user" — and POST /candidate / POST /employee hash the request's
`password` into that payload, so `user: { id: <an admin of my tenant> }` reset that
account's password and demoted its role. Same class as the candidate update cascade
this branch already closed, through the create route instead.
- time logs / time slots: `getTimeLogs()`, the report queries built on
`getFilterTimeLogQuery()` / `buildMikroOrmTimeLogWhere()` and `getTimeSlots()` build
their own queries, so the never-matching employee condition added to the CRUD reads
never applied to them. Their employee predicate is only added when `employeeIds` is
non-empty, and `employeeIds` is only narrowed for a caller who HAS an employee
record — so a caller with neither CHANGE_SELECTED_EMPLOYEE nor an employee read the
whole organization, or the employees they named in the body. They now match nothing,
exactly as `findConditionsWithoutOwnEmployee` already does.
- activities: `scopeActivitiesForWrite()` now drops a `projectId` / `taskId` (and
folds a `project` / `task` object into its id) that does not name a row of the
caller's tenant. These are plain foreign keys, so the nested-graph check never sees
them, and the activity is written through the raw repository: a foreign projectId
was stored verbatim and handed back with the victim tenant's project joined onto it.
The bulk handler now applies its request-level `projectId` before that check rather
than over it.
- OrganizationPermissionGuard: the SUPER_ADMIN exemption resolves the tenant before
the no-target shortcut. Not exploitable today (TenantBaseGuard / TenantPermissionGuard
already refuse a tenant-less request on both controllers that apply this guard), but
the guard no longer depends on a sibling guard for it.
Also, for the two new SonarCloud complexity findings: the reference rules of
`assertGraphNotForeign` move into `resolveReference()` / `isGlobalRowWritable()`, and
`UpdateTimeSlotHandler.execute` into `resolveEmployeeScope()` / `collectChanges()` /
`saveActivities()`. Behaviour is unchanged; the existing specs cover both.
Two review findings were NOT applied, deliberately:
- scoping the request-approval employee / team lookups by
`RequestContext.currentOrganizationId()`: cross-organization references inside one
tenant are a different boundary (the advisories are about cross-tenant writes), and
the header-derived organization is absent on legitimate calls, which would silently
drop approvers.
- nothing was relaxed to silence a finding; no test was deleted or loosened.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(time-tracking): drop the redundant optional chaining, keep a null employeeId meaning "no scope"
Follow-up on the review fixes, for DeepScan's three new findings on the previous
commit (all in the code it added).
- `user` is dereferenced unguarded a few lines above each of the new fail-closed
guards, so `user?.employeeId` in them was a null check that can never fire; it now
reads `user.employeeId` like the surrounding code (time-log report filters, both ORM
branches, and `getTimeSlots`).
- `UpdateTimeSlotHandler.resolveEmployeeScope()` returned `ID | undefined | null`,
where `null` meant "refuse" — but a caller holding CHANGE_SELECTED_EMPLOYEE may
legitimately send `employeeId: null` (no employee filter), and that would have been
read as a refusal. It now returns a scope object or `null`, so the two cases cannot
be confused.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(time-tracking): take the organization from the verified employee, not from the body
Two more review findings, both real: the manual-time routes and the bulk activity
save validated the employee against the caller's tenant but then persisted whatever
`organizationId` the body carried.
- `addManualTime` / `updateManualTime` already read the `futureDateAllowed` policy
from `employee.organization`, so a body organizationId of the same tenant had the
write judged by one organization's rules and then filed — time log, time slots and
timesheet — under another's. On update it also decided which rows counted as
conflicting, i.e. which sibling organization's time slots this call was allowed to
delete. Both now derive the organization from the employee, with the body value left
as a fallback for an employee that has none.
- `BulkActivitiesSaveHandler` used the employee's organization only when the body
omitted one. An Employee row belongs to exactly one organization, so the body value
could only ever file that employee's activities under an organization they are not a
member of; the employee's own organization now wins.
Each is covered by a spec arm that fails when the line is reverted (verified by
revert-and-run, files restored and hashed).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(core): split the per-relation walk and the activity normalisation out
SonarCloud's two remaining cognitive-complexity findings on this branch, both in code
it added. Pure extraction, no behaviour change:
- `assertGraphNotForeign()` keeps the level-by-level loop; the per-relation lookup and
the per-reference rules move into `walkRelation()` (31 -> well under the threshold).
- `scopeActivitiesForWrite()` hands the relation-object stripping and the
`project` / `task` fold to `normalizeReferences()`.
The nested-graph and activity specs (112 tests, real better-sqlite3 databases with
their CONTROL arms) cover both and stay green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(user): align the create handler with the merged role normalization
Merging develop brought in GHSA-x4mv's role handling: assertCanAssignRoles now
takes the payload and extracts every role form itself, and a role/roleId pair
naming two different roles is a 400. This spec still asserted the older
two-argument contract with a contradicting pair, so the two changes were green
apart and red together. It now covers both: the agreeing pair is validated, and
the contradicting pair is refused before anything is persisted.
Also replaces a stray console.log with the Nest logger.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(security): verify social-login token audience and bind every purpose token
GHSA-58x4-7mw9-gmqg (critical): POST /auth/signin.email.social accepted any
provider access token that resolved to a victim's email — another app's token or
a GitHub PAT — and signed the caller in as that user. Each provider is now
introspected against an allow-list of OAuth client ids (Google tokeninfo aud/azp
plus email_verified, GitHub /applications/{client_id}/token, Facebook
debug_token app_id) and fails closed when no client is configured. Twitter/X is
refused, since it exposes no verified email. One normaliser rejects an empty id
or email, so an undefined value can no longer reach a find() and be dropped by
TypeORM's undefined:'ignore' behaviour, which returned every user in every
tenant.
GHSA-28wv-vrxj-rp4q (medium): tokens signed with JWT_SECRET were interchangeable.
New signPurposeToken/verifyPurposeToken pin a purpose claim, required non-empty
claims and HS256. Workspace sign-in, invoice share, estimate, invite, team-join,
appointment and password-reset tokens are typed; public invoice and estimate
links are bound to the stored row and the URL id; access-token consumers
(JwtStrategy, RegisterAuthorizationGuard, Zapier, Plane) reject a token whose
purpose says it is something else. Untyped legacy tokens are accepted only where
they are also bound to a stored row, and never on signin.workspace.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(cspell): add the new vocabulary and use US spellings
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(auth): review follow-ups on the purpose-token hardening
Addresses the bot review round on PR #10241. No security behaviour is relaxed;
every change either tightens a check or is a readability fix with the same
runtime semantics, and the affected suites were re-run (9 suites / 155 tests).
- Password reset now goes through `verifyPurposeToken(TokenPurposeEnum.PASSWORD_RESET)`
instead of a bare `verify()` plus a string-literal purpose comparison
(CodeRabbit). It additionally requires a non-empty `id` claim: an `undefined`
id reaching `findOneByIdString` widens the lookup instead of failing closed,
which is exactly the class of bug this PR exists to remove. The stored
password_reset row still binds the token, so this is defence in depth.
`verify` and `JWT_ALGORITHMS` are no longer imported there.
- `JwtStrategy.validate` moves the employeeId/organizationId claim checks into
`attachEmployeeAndOrganizationContext` (SonarCloud: cognitive complexity 16 >
15). The helper RETURNS the UnauthorizedException instead of throwing, so the
exact exceptions and messages the callback received before are unchanged, and
three specs now cover the organization branch (member missing, employee in
another organization, happy path). Control: inverting the rejection makes 12
of the 30 tests in that suite fail.
- `normalizeSocialIdentity` extracts its nested ternary into
`normalizeProviderAccountId` (SonarCloud), same accepted values as before:
trimmed string, or a positive safe integer stringified for GitHub.
- `Number.NaN` over `NaN` in the reschedule-token lifetime (SonarCloud), and the
two unused `catch (error)` bindings in PublicInvoiceService are now bare
`catch`.
Not changed, deliberately: Greptile's P1 "mixed-case emails fail lookup". The
social lookup already queries BOTH the normalised (lowercased) address and the
provider's exact spelling, which is a strict superset of what this code did
before the PR, so nothing regressed. Matching stored emails case-insensitively
would let the holder of `a@x.com` sign into an account stored as `A@x.com` —
a widening of an authentication lookup that password login does not perform —
and belongs in a repo-wide email-normalisation change, not in this advisory fix.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The tracked-data queries apply their employee predicate only when the id list is non-empty
(`if (isNotEmpty(employeeIds))`). `ManagedEmployeeService.filterAccessibleEmployeeIds` returned `[]`
for a caller with no employee identity, and the counts queries scope by hand with
`if (user.employeeId && ...)`, so such a caller got no predicate at all and read the whole
organization's tracked data: time slots with their screenshots, application and window titles,
manual times, members and per-project totals.
The token is self-serve. `POST /auth/switch-organization` needs CHANGE_SELECTED_ORGANIZATION, a
default EMPLOYEE permission; switching to an organization where the user has a user_organization row
but no employee record mints a token with `employeeId: null`, and `IsOrganizationBelongsToUser` then
still accepts the caller's own organization in the query. A user whose employee record was
soft-removed logs in the same way, since the User row stays active.
- `filterAccessibleEmployeeIds` and the hand-rolled scoping in `statistic.service.ts` now return
`NO_ACCESSIBLE_EMPLOYEE_ID` (the nil UUID) for an authenticated caller with no employee identity:
the predicate stays in place and matches nothing, so the answer is empty instead of everything.
- An organization-wide viewer (`ALL_ORG_VIEW`) keeps the access their role gives them.
- A request with no user at all — a public share link, an internal call — is left to the scoping its
own caller applies, so `public-share` team pages are unaffected.
- The three hand-rolled sites now share one helper, which also removes the duplicated block.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-ups from review of this PR:
- @Max(999999999999.99) on all four rate fields, so a value the numeric(14,2) column cannot hold
is a clean 400 from the DTO instead of a database overflow, and cannot block a rollback to the
old integer column.
- CandidateHiredHandler now copies minimumBillingRate into the new employee. It only copied
billRateValue, so a candidate's minimum rate was silently dropped on hire (bug, pre-existing).
- MikroORM ignored the TypeORM `transformer` column option: @MultiORMColumn handed it to @Property,
which drops it, so with DB_ORM=mikro-orm money columns skipped rounding and validation and
int-backed enum columns (actor type, availability status) were stored and read raw.
parseMikroOrmColumnOptions now wraps the transformer in a MikroORM type and keeps the declared
column DDL (numeric(14,2)).
- Specs for each, including the hire handler (new) and the transformer bridge.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`moment().tz(undefined)` returns undefined instead of a moment, so every report that groups its rows
by `.tz(timeZone).format(...)` answered 500 "Cannot read properties of undefined (reading 'format')"
as soon as the organization had rows to group. `GET /timesheet/time-log/report/weekly` was found this
way while booting the API during the #10212 review; it fails for SUPER_ADMIN too.
- Add `resolveTimeZone()` next to `getDaysBetweenDates` and use it there, so the day list and the
grouping keys of a report always come from one and the same zone. The fallback is the server zone,
which is what `getDaysBetweenDates` and the group-by command handlers already defaulted to.
- Apply it to the five report bodies in `time-log.service.ts` (weekly, daily charts, daily, owed
amount and its charts, time limit) and to `payment.service.ts`.
- Regression specs: every report method answers with no time zone and with an empty one, the rows
land in a bucket the day list actually contains, and a named zone is still honoured. Without the
fix these reproduce the exact production TypeError. `RecordingQueryBuilder` grew a `rows` field,
because the report bodies only run over a non-empty result — which is why the existing filter
specs never caught this.
Measured before and after against a booted API on a throwaway database: five routes (weekly,
daily-chart, owed-report, owed-charts, time-limit) went from 500 to 200 once the organization had a
time log; nothing else changed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(security): compile MJML without file includes, lock down integration settings
GHSA-48h9-vwf5-h8m7 (critical): an email or accounting template containing
mj-include with a path was compiled with the file loader enabled, so any caller
who could preview or save a template could read files from the API container,
including .env and /proc/self/environ. All nine mjml2html call sites now go
through compileMjml(), which passes ignoreIncludes: true (verified against
mjml-parser-xml 4.18.0, which returns before any read) and coerces the source to
a string, so a JSON body can no longer arrive as a pre-parsed Handlebars AST. An
ESLint rule keeps raw mjml imports out of the package, and the preview routes
now validate their bodies.
GHSA-4rwq-65wh-45h4 (high): PUT /integration-setting/:id could rewrite
server-managed settings such as a GitHub installation_id, binding another
tenant's installation. Updates are now tenant-scoped, restricted to an allow-list
of user-editable keys, and pin settingsName and integrationId; the inherited
POST /integration-tenant route that reached the same sink is gated; and
installation_id is canonicalised so the uniqueness check compares like with like.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(cspell): add the new vocabulary and use US spellings
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(security): harden template source coercion, address review findings
Review follow-ups on the GHSA-48h9 / GHSA-4rwq fix. No security boundary
is relaxed; the coercion helper gets strictly stronger.
- toTemplateSource(): map every non-primitive (a JSON object or array, a
Handlebars AST, a function) to '' instead of stringifying it. `String(value)`
produced a useless '[object Object]' and, for an object whose `toString` and
`valueOf` are not callable ({"toString":1,"valueOf":2} is valid JSON), threw a
TypeError out of the request handler instead of rendering an empty template.
The preview DTOs already reject a non-string `data`, so this is the second
layer, and it is the layer the stored-`hbs` render path relies on. Primitives
still stringify. Spec updated to assert the stronger outcome (the AST now
renders to '', not to '[object Object]') and extended with the throwing shape.
Also clears SonarCloud "'source ?? ''' will use Object's default
stringification format".
- IntegrationTenantController.create(): replace the nested ternary that encoded
"not an array means refuse" as a sentinel `[null]` element with an explicit
guard. Same three outcomes (absent settings allowed, array checked
element-wise, anything else forbidden), one fewer indirection. Clears
SonarCloud "Extract this nested ternary operation".
- GITHUB_INSTALLATION_ID_PATTERN: `\d` for `[0-9]`. Identical in JS (`\d` is
ASCII-only, with or without the `u` flag); clears a SonarCloud nitpick.
- integration-setting.utils.ts: record WHY the allowlist lookup stays on
`Object.prototype.hasOwnProperty.call`. SonarCloud asks for `Object.hasOwn`,
but that is ES2022 and packages/core/tsconfig.lib.json targets es2021 with no
`lib` override, so it fails to compile (TS2550, verified with tsc).
Tests: packages/core email-template + accounting-template + integration-setting
+ integration-tenant = 8 suites, 123 tests passed; integration-github = 2 suites,
48 tests passed. ESLint over the changed core files: 0 errors.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(security): scope the invoice number sequence to the tenant
GHSA-57hw-jqpj-ww97 (medium): GET /invoices/highest ran an unscoped MAX over
every tenant's invoices, so any authenticated user learned the highest invoice
number in the installation, and numbering leaked business volume across tenants.
The aggregate is now tenant-scoped in both ORM branches and fails closed without
a tenant. Because scoping alone would collide with numbers other tenants already
hold, the global unique on invoiceNumber becomes unique per (tenantId,
invoiceNumber), with a per-dialect migration for postgres, mysql and sqlite.
GHSA-w3mx-m5cr-3gxp residual (low): a tenant AI-provider credential with no
baseUrl still reached the provider's built-in loopback default. Every
tenant-sourced credential is now treated as tenant-controlled, so it is blocked
unless private base URLs are explicitly allowed; only an operator-set environment
base URL bypasses the flag.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(cspell): add the new vocabulary and use US spellings
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(invoice): make the MySQL uniqueness migration resumable and reject fractional numbers
Review follow-up on the tenant-scoped invoice numbering change.
MySQL commits every DDL statement on its own, so `migrationsTransactionMode: 'each'`
does not roll the CREATE and the DROP of `mysqlUpQueryRunner` back together. If the
process died between them — or the information_schema lookup failed — the new
composite index survived while the migration row was never written, and the retry on
the next boot died on `Duplicate key name`, taking the API down until someone repaired
the schema by hand. Both MySQL branches now look the index up by its COLUMNS first
(`GROUP_CONCAT(COLUMN_NAME ORDER BY SEQ_IN_INDEX)`, so column order is part of the
match and the primary key never matches), create only what is missing and drop every
same-shaped leftover whatever it is named. That also fixes the reverse case the
reviewers pointed out: `down()` used to drop only the index name this migration
happens to use.
`invoiceNumber` now requires a whole number. `numeric` (Postgres/SQLite) stores a
fraction that MySQL's `bigint` truncates, so a fractional number made
`MAX(invoiceNumber) + 1` mean different things per database; `@IsNumber()` accepted
it. `@IsInt()` already implies a number, so it replaces rather than joins `@IsNumber()`.
`up()`/`down()` dispatch through a table instead of the two switch statements every
migration in this folder repeats verbatim. The behaviour is identical (including the
`Unsupported database` throw); it is here because those ~40 boilerplate lines were
counted as duplicated new code and failed the Sonar quality gate for this PR.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(invoice): cover the MySQL branch of the uniqueness migration
The MySQL resumability fix was argued from the MySQL manual, not exercised: no MySQL
server is available in this environment. It does not need one — the migration only ever
learns which unique indexes exist through `information_schema.STATISTICS`, so a stub
query runner over a simulated index set drives the real branch end to end and records
the DDL it issues.
Five cases, including the two the review raised: a crash between the CREATE and the
DROP (the index is found, no second CREATE is attempted, the old one is still dropped),
a completed run re-entered (no statements at all), an index the migration chain did not
name, and a revert that has to drop a composite index under an unexpected name. The
primary key shares the `NON_UNIQUE = 0` filter and is asserted to survive all of them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(security): resolve every form of a role reference before authorizing it
GHSA-x4mv-fhwj-g3rp (high): the role-change check read entity.role?.id and
entity.roleId, but TypeORM also accepts a relation given as a bare id string, so
a role sent as a plain SUPER_ADMIN uuid was invisible to the check and still
persisted — self-escalation through the profile update. extractRoleIds and
normalizeRolePayload now read every representation, reject a present but
unresolvable role, refuse a role/roleId pair naming two different roles, and
normalise the payload so the value that was checked is the value that is saved.
Wired into updateProfile, UserCreateHandler, the register handler, invites (which
previously stored a role the check never saw), employee and candidate creation,
and the DTO validator.
GHSA-hh83-hq74-gh9f (low): under DB_ORM=mikro-orm, wrap(entity).toJSON() ignores
class-transformer, so a populated User in a listing carried its credential
columns. refreshToken, code and codeExpireAt are hidden at the ORM level, and a
last-line scrub removes credential keys from User-shaped objects in responses.
GHSA-hjcg-633x-qq74 hardening: the register guard tenant-checks every role
identifier in the body instead of only the first one it finds.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(cspell): add the new vocabulary and use US spellings
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(invite): validate the role payload before branching on the inviter
`createBulk` only read the body's role references inside the fallback branch,
so the checks rode on the inviter's own role: an EMPLOYEE inviter, who is
force-assigned the EMPLOYEE role, had a malformed or self-contradicting payload
(`roleId` and `role` naming two different roles, `role: {}`, `roleId: ''`)
silently accepted, while every other inviter got a 400 for the same body.
That was never an escalation — the EMPLOYEE branch persists the checked role,
which is the point of GHSA-x4mv-fhwj-g3rp — but an answer that depends on who
is asking is a bad place to keep input validation, and it leaves the one caller
whose role is overridden as the only one whose payload nobody parses.
The extraction now runs once, before the branch, and a body naming two
different roles is refused for everyone. The fallback keeps its own
"exactly one" rule, since an invitation there cannot be issued without a role.
Regression coverage added for the EMPLOYEE inviter, plus a control that a body
with no role at all still issues an EMPLOYEE invitation.
Also splits `scrubNode()` out of `scrubUserCredentials()` so the walker stays
under SonarCloud's cognitive-complexity threshold (17 -> 9). No behaviour
change: the same nodes are visited and the same keys deleted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(cspell): use the US spelling in the new invite comment
Cspell flagged "licence" in the comment added by the previous commit; the
wording now avoids the word entirely.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(security): never sign tokens with a published default secret
GHSA-39j7-x845-4w3c (high) and GHSA-chm8-2ggf-pgjq: when JWT_SECRET,
JWT_REFRESH_TOKEN_SECRET, JWT_VERIFICATION_TOKEN_SECRET or
EXPRESS_SESSION_SECRET was unset, the API fell back to a literal published in
this repository, so anyone could forge tokens and sessions. resolveSecret() now
substitutes a per-process random value instead, and the startup guard still
reports such a secret as unset, so production keeps refusing to boot. The
known-default list is shared by the config resolver, the startup guard and the
desktop apps, and it catches a default published for any key, not only its own.
Desktop apps generated no secrets at all: every install shipped the same baked
DESKTOP_JWT_* values while binding the local API to 0.0.0.0. They now provision
random per-install secrets on first run and rotate a stored published default.
DEMO=true keeps today's behaviour, with a TODO, pending a decision on
demo.gauzy.co's secrets.
Also: the MCP refresh grant re-checks the account on every refresh instead of
trusting a 30-day-old token (GHSA-3cgp-wmrg-4fqg residual), and the misleading
comment claiming the Electron seed-credential exemption is harmless is corrected
(GHSA-4r2r-mv32-3468).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(config): resolve signing secrets lazily so load order cannot split them
Found by a runtime probe against a locally built API: login returned 200 but the
access token it issued was rejected on the next request, because the process was
signing with one secret and verifying with another.
apps/api/src/main.ts calls loadEnv() (which reads .env.local and friends) only
after its imports have run, so @gauzy/config can be evaluated while JWT_SECRET is
still unset. With the published literal as the fallback that was invisible: the
early copy and any later copy both ended up on 'secretKey'. Once an unset secret
became a per-process random value, the early copy generated one while a copy
imported after loadEnv() read the configured value — so tokens signed by one
never verified in the other, and every authenticated request 401'd.
The four secrets are now getters on `environment` / `environment.prod` /
`defaultConfiguration.authOptions`, so each is read at first use, after the env
files are loaded. Adds the regression test for exactly that order.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(security): separate an unavailable account lookup from a dead refresh token
Review pass over PR #10240. The substantive fixes:
- MCP refresh grant (CodeRabbit major, Greptile P1). `getMcpUserInfo` caught
every error and answered `null`, so a database blip reached
`refreshAccessToken` as "this user is gone" and the grant handler answered
`invalid_grant`. Well-behaved OAuth clients discard a refresh token on that,
so one transient outage signed active users out for good. `getMcpUserInfo`
now re-throws a failed lookup, `refreshAccessToken` re-throws it as
`UserLookupUnavailableError` (outside its own catch, so it cannot be
flattened into `null`), and the grant answers `temporarily_unavailable` /
503. `null` now means exactly one thing: the account can no longer sign in.
The refresh token is still never revoked on either path.
- `resolveUser` is now REQUIRED (Greptile P2). It was optional on a public
method, so a caller could keep the two-argument shape and silently skip the
account check the parameter exists to perform. A missing resolver now throws
before anything else (500 server_error), matching the missing-provider path.
`UserLookupUnavailableError.is()` is used instead of bare `instanceof`: this
package ships both as source and as a bundle, and a downlevelled
`extends Error` would break the prototype chain and quietly restore the
invalid_grant behaviour.
- Nested credentials reached the desktop logs (CodeRabbit major, CWE-532).
`redactSecretsForLog` only looked at top-level keys, and `apps/desktop` logs
the whole `DesktopSetupConfig`, so `postgres.dbPassword` and
`secureProxy.ssl.key` were printed verbatim. It now walks nested objects and
arrays, with a depth cap and cycle detection.
- The seeded-account warning missed renamed installs (CodeRabbit major).
`getPublishedSeedAccounts()` read only today's `DEMO_*_EMAIL` values, but the
database was seeded in the past: an operator who changed the address after
installing still had `admin@ever.co` with the published password, and the
check walked past it. Both the configured and the canonical addresses are now
checked, deduplicated on the (email, password) pair.
- The bounded read could hide a vulnerable account (Greptile P2). One shared
`IN (...)` query with a global limit of 10 let the rows of whichever address
came back first use the whole budget. The budget is now per address (5 rows
each), so every candidate is actually looked at, and rows are re-matched
against the address they were fetched for.
- The lazy-getter commit (21e0bfd) broke its own regression suite:
`validate-application-secrets.spec.ts` assigned to `environment.JWT_SECRET`,
which is now getter-only, and all 8 tests died at "Cannot set property". The
spec drives `process.env` instead, which is what the getters read, plus a new
test pinning that a secret set AFTER import still decides the verdict.
Static-analysis cleanups: `config?.secret` next to an unconditional
`config.db` in both server launchers (the inconsistent null check DeepScan
flagged); the duplicated `JWT_VERIFICATION_TOKEN_SECRET` block in
.env.demo.compose; and the Sonar smells (`node:crypto`, assignment inside a
return, optional chaining, useless empty object).
Not applied: CodeRabbit's request to point the three k8s demo manifests and
fly.toml at a secret store. Those are the DEMO=true deployments this PR
deliberately holds; wiring `secretKeyRef` to Secrets that do not exist would
break the demo without changing the effective values, and the manifests already
document how to inject real ones.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(auth): correct a stale comment about the refresh account lookup
The deactivated-user test still said the wired provider answers `null` for a
transient lookup failure. It rejects now, and that case is covered by its own
test, so the comment described behaviour the suite no longer has.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(auth): drop a word cspell rejects from the token-manager comments
The Cspell check failed on "downlevelled" in two comments. Reworded to
"compiled for a pre-ES6 target", which says the same thing in words the
dictionary already has, rather than growing the project word list for prose.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(user): split the published-password check into named steps
The per-address rewrite pushed findAccountsUsingPasswords over SonarCloud's
cognitive-complexity limit (16/15) with three nested loops and an inline ORM
switch. The query and the verification loop are now their own methods, and the
candidates collapse into a Map of address -> passwords instead of being
filtered, de-duplicated and re-filtered inline. Same behaviour: one query per
distinct address, every proposed password tested, first match wins.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(core): report a non-exhaustive seed-password check instead of implying clean
The check reads a bounded number of rows per address, because it runs before
the API listens and every scrypt/bcrypt verification is expensive by design.
With the same seeded address in more tenants than the budget, a vulnerable row
can sit outside the sample — and a boot that found nothing looked exactly like
a boot that checked everything.
findAccountsUsingPasswords now returns `{ matches, inconclusive }`;
`inconclusive` names the addresses whose rows filled the budget without a
match, and boot prints a short note asking for those tenants to be audited
separately. It deliberately does NOT raise INSECURE ACCOUNTS in that case:
a scary warning on every boot of any large multi-tenant install is how a
warning gets ignored.
Raising the budget was the alternative and it does not fix anything — it only
moves the cliff, at the cost of boot time.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(security): neutralize spreadsheet formulas in CSV exports
GHSA-7xp5-j564-4752 (medium): exported cells were written verbatim, so a stored
value beginning with = + - or @ executed as a formula when a colleague opened the
export in Excel or Sheets. A shared encoder in @gauzy/utils prefixes an
apostrophe to any cell starting with a formula trigger (including the full-width
forms), leaves strictly numeric values alone, and is reversed on import so an
export/import round-trip stays byte-exact. Every field is now quoted, which also
stops a bare CR inside a value from starting a new spreadsheet row, and the
invoice CSV in the web app gets real RFC 4180 quoting instead of JSON.stringify.
GHSA-86mw-2crg-vmhc residuals (low): the public invite routes are throttled, the
shipped compose files no longer trust a forwarded client IP while publishing the
API port directly, and RequestContext.currentIp() resolves the client address the
same way the throttler does instead of reading a spoofable header.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(cspell): add the new vocabulary and use US spellings
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(export-import): decode CSV cells only for archives we marked
The import side reversed the spreadsheet-formula escape on every parsed row,
but an uploaded ZIP is not necessarily one this server wrote: it can be a dump
from an older Gauzy, a filled-in `/export/template`, or a CSV set built by
external tooling. For those, un-escaping is data loss — a legitimate value such
as `'=notes` was persisted as `=notes`, silently. Both review bots flagged this
as the one thing blocking the merge, and they were right: the decoder had no way
to tell "we escaped this" from "somebody else wrote this".
A data export now carries a `gauzy-export.json` marker at the archive root
(format, version, `spreadsheetSafeCells`), written by `exportTables` and
`exportSpecificTables`. `ImportService` resolves that marker once per import and
decodes rows only when it is present and recognized; anything else is imported
byte for byte as it was parsed. `/export/template` is deliberately NOT marked —
an operator fills it in by hand, so nothing in it was ever escaped. The manifest
reader is defensive about an attacker-supplied file: missing, oversized,
malformed, a foreign format or a newer version all mean "do not decode".
The invoice/payment CSV builder no longer has a path that skips encoding: a
pre-joined header line used to be written through verbatim, and it was the only
value in the file that reached disk unquoted and un-neutralized. `buildCsv` and
`generateCsv` now declare `headers: string[]` (both callers already pass one),
and a stray string from an untyped caller is split and encoded rather than
trusted.
Tests: `import.service.spec.ts` imports the same escaped CSV with and without a
marker and asserts the apostrophe survives when unmarked (reverting the gate
fails those 5 tests); `export.service.spec.ts` asserts the data archive carries
the marker and the template archive does not.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The MySQL column type appears in comments and a spec added by #10212. Existing uses were only inside
packages/core/src/lib/database/migrations, which cspell ignores, so the word was never needed before.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to #10203, which fixed#10199 for employees only.
- candidate.billRateValue / minimumBillingRate become numeric(14,2) / decimal(14,2)
(migration 1790000016000; 1790000015000 is taken by open PR #10212). Postgres and
MySQL alter both columns in one statement, Postgres with a 5 s lock_timeout; SQLite
copies through a temporary column. reWeeklyLimit stays integer.
- Candidate uses the same column options and toBillingRate transform as Employee,
moved to shared/pipes/billing-rate.transform.ts so the two cannot drift.
- Fixes a regression on develop: PUT /candidate/:id validates rates with the employee
UpdateProfileDTO, which keeps cents since #10203, so Postgres rejected 10.49 in the
integer candidate column (400) and MySQL rounded 10.5 to 11.
- ColumnNumericTransformerPipe(scale).to() stores a blank string as NULL instead of
returning 400 (routes that skip DTO validation, e.g. POST /candidate and /employee).
- Specs: candidate transforms, UpdateCandidateDTO request path, response serialization,
migration SQL for all engines, blank-string handling.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The API printed the full REDIS_URL - Valkey password included - to stdout at boot (Redis health indicator and Redis session store). Boot-time log lines now go through dependency-free helpers in core/util/redact-credentials.ts that keep scheme/user/host/port and replace credentials with ***:
- REDIS_URL lines (health indicator, session store): redis://:***@host:port
- Malformed REDIS_URL: the ERR_INVALID_URL error (which carries the URL on error.input) is redacted before console.error; a partially redacted error is never returned
- tracer: Honeycomb API key presence only; OTEL_EXPORTER_OTLP_HEADERS names only; tracing URL redacted
- Unleash config line: customHeaders by name only, URL credentials redacted
The Redis clients, exporters and Unleash still receive the real credentials; only log output changes. Specs cover every path with sentinel values and positive controls, and each was confirmed red against the previous code. No credential rotated.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
