Recent Activities

Fix tabs.js cleanup on destroy, selector escaping and scroll button clicks 9cd5d6

Fix double-encoded widget_identifier when installers update tabs cc0db3

Cache empty tabs result to avoid repeated tab queries 7982ca

Fix stale full page cache for tabs, ajax tabs and GraphQL

- Tag pages with the generic easytabs_entity tag and flush it on tab save, so new or enabled tabs appear on cached pages
- Add product cache tags to ajax tab responses
- Add CMS block cache tags to GraphQL getEasyTab responses a93cca

Fix mass Enable/Disable skips widget tabs ef2b21

Fix `tabsUpdater._destroy` throws `ReferenceError` 7bae6f

Handle malformed URI sequences in tab anchor decoding 973dd6

Fix customer validation for guest users by handling missing group_id f06ace

Enhance Ajax tab filtering and validation, implement safe parameter handling in LoadOptions and NewConditionHtml, and ensure proper product checks in Index controller ead1aa

Fix missing aria-level on tab headings in expanded layout (#39)

* Fix missing aria-level on tab headings in expanded layout

* Fix missing aria-level attribute for expanded tabs on Breeze

---------

Co-authored-by: Vitaliy Dmytruk <vitaliy@swissuplabs.com> 3dab50

Fix missing aria-level attribute for expanded tabs on Breeze 05c0a6

Fix missing aria-level on tab headings in expanded layout 2523f0

Version 1.13.6 fe61d0

Fix product visibility bypass and GraphQL tab conditions bypass (#38)

* Fix product visibility bypass and GraphQL tab conditions bypass

- Helper\Product::canShow(): parent_id fallback now applies only to
enabled products and only when parent_id is their real parent
(catalog_product_relation). Previously any visible product id in
parent_id exposed disabled products via the easytabs AJAX endpoint.
- GraphQL getEasyTab now validates tab conditions (customer group, etc.)
using the customer group from GraphQL context, like the storefront does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address review: stable parent check, duplicate aliases, real customer group

- Helper\Product: check parent's children via type instance
getChildrenIds() instead of Relation::getRelationsByChildren(),
whose return shape is not a stable API. Drops ObjectManager usage.
- GraphQL data provider: with duplicate aliases, return first tab that
passes conditions instead of failing on the highest-priority one.
- GraphQL resolver: load caller's customer group via customer repository
instead of defaulting to guest when context has no customer_group_id
(guest fallback exposed "not in group X" tabs on older Magento).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Pick same tab as storefront for duplicate aliases in GraphQL

Mirror Block\Tabs::_buildTabs(): among tabs passing conditions, prefer
the requested store view over default store view (0), and within a store
take the same tab the storefront does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> 90330e

Pick same tab as storefront for duplicate aliases in GraphQL

Mirror Block\Tabs::_buildTabs(): among tabs passing conditions, prefer
the requested store view over default store view (0), and within a store
take the same tab the storefront does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> 06c0a8

Address review: stable parent check, duplicate aliases, real customer group

- Helper\Product: check parent's children via type instance
getChildrenIds() instead of Relation::getRelationsByChildren(),
whose return shape is not a stable API. Drops ObjectManager usage.
- GraphQL data provider: with duplicate aliases, return first tab that
passes conditions instead of failing on the highest-priority one.
- GraphQL resolver: load caller's customer group via customer repository
instead of defaulting to guest when context has no customer_group_id
(guest fallback exposed "not in group X" tabs on older Magento).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> 3871b9

Fix product visibility bypass and GraphQL tab conditions bypass

- Helper\Product::canShow(): parent_id fallback now applies only to
enabled products and only when parent_id is their real parent
(catalog_product_relation). Previously any visible product id in
parent_id exposed disabled products via the easytabs AJAX endpoint.
- GraphQL getEasyTab now validates tab conditions (customer group, etc.)
using the customer group from GraphQL context, like the storefront does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> 3a8cc0

Merge branch 'master' into claude/guest-ticket-token-79-d237f7 d04b65

Merge branch 'master' into fix/attachment-link-lifetime-91 2e2bfb

Merge pull request #105 from swissup/claude/youthful-moser-f78a75

fix(test): type the nullable parameter explicitly for PHP 8.4 (#104) 2df7ca

fix(test): type the nullable parameter explicitly for PHP 8.4 (#104)

`execute()` declared `array $uploadResult = null`, which PHP 8.4 treats as
an implicitly nullable parameter and deprecates. The deprecation is raised
while PHPUnit is still loading the test file, so the suite aborts with an
"Internal server error" before a single test runs, and the PHP Linter check
fails too. Both have been red on master since a4bc727, and every branch cut
from master inherits the failure.

A sweep of the module turned up no other implicitly nullable parameter; the
remaining `$param = null` defaults are on untyped parameters, which stay
valid.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 0923b6

fix(security): withdraw a ticket's token when it is moved to another address (#79)

`email` is in TicketDataFilter::ALLOWED_FRONTEND_FIELDS, which filterAdminData
merges, so an administrator saving a ticket can change the address a guest ticket
belongs to.

Until the token existed that revoked the previous reader on its own: the e-mail
check reads the address off the ticket, so once it changed, the old one stopped
matching. The token is held by the reader instead of re-derived from the row, so
nothing withdrew it and a link mailed to the previous address kept working for
good. Rotating on the change restores what the e-mail check gave for free, and it
is the lever the stored column was chosen for.

The address is compared the way TicketAccess compares it — trimmed, without
regard to case — so that re-saving a ticket without touching the field does not
invalidate links that are already out.

customer_id needs no equivalent: once it is set, TicketAccess takes the customer
branch and never reaches the token at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 8fd484

feat(email): put the ticket token in customer notification links (#79)

Both customer templates built the link themselves out of {{store url}} and the
ticket number, which left nowhere to put the token and hardcoded the helpdesk
front name that Helper\Config lets a shop change. The observers now pass a
finished ticket_url and the templates print it.

A ticket with no token yet — one created before the backfill patch has run —
yields the same number-only URL the templates produced before, so nothing
regresses while an upgrade is in flight.

A shop that has customised either template in Marketing > Email Templates keeps
its stored copy and its old markup, so its links stay number-only until the
template is updated. Those links still work: Ticket/View falls back to the
e-mail prompt exactly as it does today.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 868444

feat(security): let a ticket token stand in for the guest e-mail prompt (#79)

A guest following a notification link in a fresh session is asked for the address
the ticket was created with, which is the thing the link was supposed to spare
them. The token is what the link can carry instead.

The token is a bearer credential and is treated as one: holding it is the whole
proof, not a second factor on top of the e-mail. It is sent to the address that
owns the ticket, which is exactly what the session e-mail check was a weak stand
in for, and unlike the attachment signature in #91 it is per ticket and stored,
so a single ticket can be given a new one without touching anything else.

Two limits on what it buys:

A token never unlocks a ticket that belongs to a customer account. Those stay
behind the customer session, so a forwarded link cannot walk past the login that
guards them today.

A token accepted by Ticket/View does not seed the guest e-mail into the session.
That session value opens every guest ticket opened with the same address, while
the token speaks for the one ticket it was issued for; the reply form carries the
token back instead, so a reply works in a session that never saw the address.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> e400a6

feat(security): give each ticket an unguessable access token (#79)

The ticket number is the only identifier a guest ticket has, and
Model/ResourceModel/Ticket builds it from three random lowercase letters over a
sequential id: 17576 candidates per id. Since 1.4.10 that is no longer an access
hole — TicketAccess makes a guest present the ticket's e-mail — but the number
still travels in URLs, in notification e-mails and into every log that records
them, and anything built on top of it inherits the weakness.

The token is a stored column rather than an HMAC over the ticket id, as
AttachmentSignature derives its signature. An HMAC needs no column and no
backfill, but it cannot be withdrawn from a single ticket: the only lever is the
installation crypt key, and AttachmentSignature deliberately accepts every key in
a rotation so that links already sent keep working — so rotating it would not
revoke anything either. A stored value can be replaced one row at a time, is
independent of the crypt key, and leaves room for token-only URLs later without
putting the sequential id back on display.

Existing tickets get a token from the data patch during setup:upgrade, so no
ticket is left without one. The column is nullable so the schema change does not
have to invent values for rows that already exist, and MySQL permits the repeated
NULLs under the unique index until the patch fills them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 7aab75

Merge pull request #101 from swissup/claude/docs-inbound-mail-78-2cbaa9

docs(email): state the SPF, DKIM and DMARC requirement for inbound mail (#78) fb3b5b

feat(security): add an opt-in lifetime for signed attachment links (#91)

A signature is the fallback the download falls back to when nobody is signed in
to the ticket, so it is still a bearer token for that case. Shops that want
those links to stop working after a while can now say how long they last, in
days, under Helpdesk > Security Settings.

The default is 0, no expiry, and signing under it produces exactly the payload
1.4.10 produced: the links already sitting in customers' mailboxes keep
validating, so upgrading changes nothing in the field.

When a lifetime is set, the moment the link dies is part of what is signed and
is carried in front of the MAC, separated by a ":" a hex MAC cannot contain, so
it cannot be moved by whoever holds the URL. Turning the setting on also stops
the links signed without an expiry from being accepted: a link with no expiry in
it can never be aged out, and retiring those is the point of turning it on. The
admin comment on the field says so.

A link that carries an expiry is still honoured, and still expires, after the
setting is turned off again, so flipping it back does not resurrect anything.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> ff0d0c

fix(file): check the visitor against the ticket before serving an attachment (#91)

The download was gated on the signature alone. A valid signature is a bearer
token: it never expires, cannot be revoked and says nothing about who is
presenting it, so a forwarded notification e-mail, a shared mailbox, a proxy
log or a referrer header hands the file to whoever ends up holding the URL.

Ask TicketAccess about the ticket the message belongs to first. The customer
the ticket belongs to, and a guest whose session carries the ticket's e-mail,
are served whether or not the request has a signature at all.

That branch is limited to messages the visitor is shown. Internal notes are
filtered out of the frontend listing, and the stored path is the uploader's
dispersion of the original file name, so without the limit owning the ticket
would be enough to pull a note's attachment out of it by guessing a message id
and a file name.

The signature stays as the fallback for the case it exists to serve: a request
with no session that knows the ticket — a link opened from a notification
e-mail, and the attachment preview in the backend, which builds frontend URLs
and reaches this controller without a customer session. Internal notes are
still previewable there, as before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> dd009b

docs(email): state the SPF, DKIM and DMARC requirement for inbound mail (#78)

The checks added in 1.4.10 (#70) all read the address in the From: header,
and SMTP does not authenticate it. Nothing in PHP can: the module sees the
message long after delivery. A forged sender belonging to a user of the
department owning a ticket is still stored as a staff reply, and a new
ticket is still bound to the customer account owning the sender address.

Document the mail-server side of that in README.md, which is the only
documentation this repository carries; the fuller setup page lives in the
docs site. Add a notice on the Email Storage form's Host field so the
requirement is visible where the mailbox is configured.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> ead4ad

Merge pull request #100 from swissup/fix/admin-upload-payload-99

fix(upload): refuse a malformed admin upload payload with the JSON error (#99) 2cd0af

Merge pull request #98 from swissup/fix/review-follow-ups-94

fix: review follow-ups — header escaping, audit logging, strict compare (#94) 37578f

Merge pull request #97 from swissup/fix/uploader-error-escaping-93

fix(upload): set the uploader's error text instead of interpolating it (#93) e4b7e2

Merge pull request #96 from swissup/fix/upload-payload-shape-92

fix(upload): answer a malformed file payload with the JSON refusal (#92) 1bf272

Merge pull request #95 from swissup/fix/attachment-signature-key-90

fix(file): refuse to sign attachment links without a crypt key (#90) 72401a

Merge pull request #83 from swissup/fix/low-findings-74

fix(security): close the four low-severity audit findings (#74) 6185f6

fix(upload): refuse a malformed admin upload payload with the JSON error (#99)

The admin file upload read two array indexes without checking either, and
handed whatever it found to Model\Upload. A POST without the
swissup_helpdesk_ticket_message[file] part passed null; a field named
...[file][] passed the array-shaped $_FILES entry, where tmp_name and name
are themselves arrays. pathinfo() then raised a TypeError, which extends
\Error and so escaped the catch, and the uploader in the ticket form got an
exception page where it reads {error, errorcode}.

Catching \Throwable would have swallowed the crash instead of refusing the
input, so the shape is checked up front and anything else answers with the
error shape the form already handles.

The refusal is a 200, not a 4xx: Magento_Ui's file-uploader reads the message
off the done handler, and a 4xx reaches a fail handler that only logs to the
console.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> a4bc72

fix(file): state the attachment filename as RFC 6266 (#94)

The name went into Content-Disposition between double quotes, and
FileInfo::isSafeFilename() rejects only NUL and a backslash. What keeps a
quote out today is Magento's Uploader with setAllowRenameFiles(true), an
upstream guarantee this line neither states nor checks.

filename*=UTF-8'' leaves no quoting to get wrong and carries a non-ASCII
name correctly, which the quoted form never did.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> c95c9b

fix(file): compare the preview MIME type strictly (#94)

Both sides are strings today, so it behaves. A comparison that decides
whether a response is rendered inline in the shop's own origin should not
depend on that staying true.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 95b4cf

fix(message): report a posted is_backend_authored flag (#94)

The allow-list already drops it, so there is no mass-assignment hole. But
the forbidden list is what feeds logMassAssignmentAttempt(), and the field
that decides whether a message's text goes through the CMS filter is the one
an attempt to set is most worth seeing in the log.

Both administrator paths set the flag after the check, on filtered data, so
a legitimate backend save still reports nothing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 36e1b0

fix(upload): set the uploader's error text instead of interpolating it (#93)

addError() built an HTML string around its argument, and since #81 one
caller passes jqXHR.responseJSON.message straight off the wire. Every
message that reaches it today is written in this module, so nothing is
exploitable as the code stands, but the sink is one third-party validator
message away from echoing something a visitor chose.

Binding the click handler to '.swissup-attachment-error' also re-bound it to
every error already on the page, so the handlers accumulated with each new
one. The node that was just built is the only one that needs it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> df6fb6

fix(upload): answer a malformed file payload with the JSON refusal (#92)

is_array() passes the array-shaped $_FILES entry a field named fileupload[]
produces, where tmp_name and name are themselves arrays. pathinfo() then
raises a TypeError, which is not a LocalizedException and so escapes the
catch: the ajax caller is handed an HTML exception page instead of the
refusal it knows how to display. A payload whose upload failed arrives the
same way, with an empty tmp_name.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> ca5188

fix(file): refuse to sign attachment links without a crypt key (#90)

An HMAC keyed with the empty string is not a secret. Anyone who knows the
message id and the stored file path could recompute the signature and
isValid() would accept it, which is the defect #73 was filed for, reached
by a different route and without a word in the log.

An installation with no readable crypt/key is broken, not an installation
that should go on handing out download links. Both signing and validation
now stop there.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> af9c9e

fix(upload): stop the new log line from being able to fail an upload (#74)

`Mime::getMimeType()` throws `FileSystemException` when the file is not there,
and it was called only to decorate a notice — on the one path that previously
never touched the file at all. A missing tmp file turned an upload that used to
proceed into a refusal, for the sake of a log field. It is sniffed in a try now
and reported as "undetermined" when it cannot be read.

The `$fileName` parameter loses its default; the method is private and its only
caller always passes one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> d044a6

fix(security): let an address block bite again after the re-keying (#74)

Re-keying the counters on the visitor left `blockIp('1.2.3.4')` writing a record
that nothing would ever read: `checkLimit()` resolves to `customer:` or
`session:` for anyone carrying a session, so the address record was never
consulted. A public method that silently does nothing is worse than no method.

`checkLimit()` now tests the address alongside the visitor, and an identifier
that carries none of the known prefixes is read as the bare address an operator
means. Automatic blocking is unchanged and still only ever lands on the visitor,
so a shared proxy address cannot collect somebody else's violations.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 8e3397

fix(cron): make the per-message guard actually contain a failed message (#74)

Two holes in the containment this file claimed to have.

`catch (\Exception)` does not catch `\Error`, so a TypeError anywhere in the
body still left execute() and took every remaining department with it — the
exact failure this guard exists to stop, and not hypothetical: a TypeError on
the upload path was found in this module this week. Both catches now take
`\Throwable`.

Skipping the removal sweep for a department that had one failure also kept the
messages that succeeded. POP3 carries no flags, so the FLAG_SEEN check never
fires there and every survivor was imported again on the next run; the duplicate
check only holds for a message that carries a Message-Id. A mailbox stuck on one
bad message therefore duplicated the rest, every cron tick. Successful message
numbers are collected instead and removed in descending order, so only the
message that failed is left behind.

The numbers are collected in `finally`, because the body leaves the iteration
with `continue` for a message that was seen, blacklisted or already imported.
Those are handled too and must still be removed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 21285c

Merge remote-tracking branch 'origin/master' into fix/low-findings-74 0df144

Merge pull request #89 from swissup/fix/autoclose-system-change-88

fix(notify): stop reporting auto-close as a customer update (#88) cfd4fd

fix(notify): stop reporting auto-close as a customer update (#88)

The auto-close cron saves tickets with a new status, no message data and
no backend session. _afterSave() therefore wrote the status-change
message with user_id NULL, and AdminNotification treats any message
without an admin user as the customer's. Every auto-closed ticket sent
the department a "customer updated the ticket" e-mail.

A caller can now mark a change as made by the module itself with
is_system_change in the swissup_helpdesk_ticket_message data.
_afterSave() copies the mark onto the new message. The mark is not a
column, so it only exists on the instance the save_after observers
receive. AutoClose sets the mark and states user_id => null. Since #86
a stated user_id wins over the backend session, so a cron run from a
backend request is not credited to whoever triggered it.
AdminNotification only reports messages that are neither from an admin
nor a system change.

AutoClose is the only caller that saves a ticket with neither message
data nor a backend session. The module has no webapi.xml or GraphQL
schema. The frontend controllers are real customer actions, and their
message data is whitelisted to text and file, so a customer cannot set
the mark.

The department-user loop also skipped the user whose id matched
$ticketMessage->getUserId(). That code only runs for messages without
an admin user, where getUserId() is 0, and it compared int to string
strictly. It could never match, so it is removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> fa761f

Merge pull request #87 from swissup/fix/staff-email-replies-86

fix(mail): state who wrote picked-up mail; keep staff e-mail replies internal by default (#86) 3e2e52

fix(security): hash the session id before it becomes a rate-limit identifier (#74)

The identifier is written to the log on every violation and every block, so
keying anonymous visitors on the raw session id put a live session cookie in
var/log — a credential, not an attribute. Hashing it at the point it is resolved
keeps the counter working and leaves nothing reusable in the log. Customer ids
and addresses stay as they are; neither is a secret and both are what an
operator needs to read.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 3d2a4c

Merge remote-tracking branch 'origin/master' into fix/low-findings-74 956abb

fix(mail): state who wrote picked-up mail; keep staff e-mail replies internal by default (#86)

The e-mail pickup put the staff member's id on the message, but
Ticket::_afterSave() built the row with the backend session's user
instead. Under cron that is nobody, so a staff reply by e-mail was stored
as a customer message: the customer got nothing and the department was
told about a "customer update". Run from the Pick Up button, every
message of the run, customers' included, was attributed to the admin
who clicked, and a customer's own mail went back to them as a staff
reply.

_afterSave() now takes user_id from the message data when the caller
states it, null included, and falls back to the session only when it is
absent (mass status/priority actions). The pickup always states it, and
the admin ticket form states the logged-in admin, overwriting any posted
value.

The staff check rests on the From: address alone, which is not
authenticated (#78). A new setting, Mail Settings > "Send staff e-mail
replies to the customer", decides what a staff reply becomes:
- No (default): an internal note by that staff member; the ticket
status is left alone and nothing is mailed;
- Yes: a public staff reply, status Replied, mailed to the customer.
Only safe when the MTA rejects mail failing SPF/DKIM/DMARC.

A staff address that opens a new ticket is that ticket's requester, as
before. The pickup also took the id of the department-user link row
instead of the admin's user_id; it now uses user_id.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> 204b25

Merge pull request #85 from swissup/fix/inbound-message-id-84

fix(mail): keep inbound Message-Ids and make threading lookups match (#84) de1965

fix(mail): keep inbound Message-Ids and make threading lookups match (#84)

Three faults on one path, each enough to break the pickup's threading and
duplicate check:

- ResourceModel\Ticket::_afterSave() dropped the Message-Id the pickup hands
it, so inbound ids were never stored and PickUp's duplicate check could not
match. The notification observers then overwrote the column with the id of
the mail the store sent.
- Those outgoing ids were stored as "<id@host>", while the parser returns
In-Reply-To as "id@host". A reply to a notification was never found by
In-Reply-To either; threading worked only through the subject's ticket
number.
- loadByEmailMessageId('') matched the first row with an empty id, which is
most rows. A new e-mail with no In-Reply-To was resolved to that message's
ticket: rejected with a misleading ownership warning since #70, or appended
to it when the sender owned it or was staff. An e-mail with no Message-ID
was taken for a duplicate and deleted from the mailbox unread.

Ids are now stored and looked up without angle brackets, an empty id never
queries, _afterSave() keeps the inbound id for anything not written in the
backend, and the observers no longer overwrite an id a message already has.
A data patch strips the brackets from existing rows.

The inbound id is attacker-supplied and is echoed into In-Reply-To and
References of outgoing mail, so anything that is not a plain printable msg-id,
or exceeds the 255-character column (an insert error in strict mode), is
dropped rather than stored.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> 1a3961

Merge pull request #82 from swissup/fix/download-hash-and-preview-73

fix(file): sign attachment links with a real secret and stop previewing documents (#73) 25f292

Merge pull request #81 from swissup/fix/upload-endpoint-guards-72

fix(upload): require POST, a form key and the rate limit on the upload endpoint (#72) 321f9b

Merge pull request #80 from swissup/fix/cms-filter-stored-xss-71

fix(message): run only backend-authored text through the CMS filter (#71) 286fb3

fix(file): keep PDF preview out of the CSP sandbox, drop unused encryptor (#73)

The bare sandbox directive puts the inline response in an opaque origin.
Browser PDF viewers have fallen back to downloading the file in that case,
which would turn the admin's PDF preview into a download. PDF script runs in
the viewer's own isolate, never in the shop's origin, so the sandbox buys
nothing there; images and text/plain keep the full policy, and every inline
response keeps nosniff.

FileInfo no longer hashes anything since links are signed by
AttachmentSignature; its EncryptorInterface argument was left unused. The
constructor already changed in this branch, so dropping it costs no extra
compatibility.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> 638b64

fix(upload): upload one file at a time and stop once refused (#72)

Selecting thirty files is one click, and the widget fired one request per file
in parallel. With file_upload now metered, the first twenty are accepted and
every request after that counts as a rate-limit violation; ten violations block
the address for an hour, for ticket and message creation as well. A legitimate
bulk attachment locked the visitor out of the helpdesk entirely.

Chain the uploads and abandon the rest after the first server refusal, so a
bulk selection trips the limit once rather than a dozen times. A file rejected
by the client-side extension or size check still lets the others through.

The endpoint also reached Model\Upload with a null payload when a POST carried
a valid form key but no file, which is a TypeError and an HTML exception page
for an ajax call rather than the JSON refusal the uploader can show.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 09f477

fix(message): record backend authorship instead of inferring it (#71)

isTrustedAuthor() read user_id and an empty email_message_id off the stored
row. Neither says who wrote the text.

The Pick Up button on the ticket grid runs Cron\EmailStorage\PickUp inside the
administrator's own backend request, so ResourceModel\Ticket::_afterSave()
stamped that administrator's user_id onto every inbound message of the run.
_afterSave() never copies the inbound Message-Id, so email_message_id stayed
empty too: an e-mail body reached the CMS filter, raw HTML and {{...}}
directives included, which is the hole #71 is about. Adminhtml\TicketMessage\
AjaxSave gave the same pair to any existing message an administrator re-saved,
including a customer's.

In the other direction the notification observers write the outgoing mail's
Message-Id onto the message they report, so an administrator's own reply lost
its authorship the moment the customer notification went out, and the option
silently stopped working for the one case it exists for.

Record the fact instead. swissup_helpdesk_ticket_message.is_backend_authored is
set by the two paths that know the text came from a backend form -- the admin
ticket Save controller and AjaxSave creating a new message -- and read by the
policy. _afterSave() takes it from its caller rather than from the session.

Existing rows default to 0, so messages written before the upgrade render
escaped. No backfill: user_id plus an empty email_message_id is exactly the
inference this commit removes, and the option is off by default.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 284e11

fix(security): key rate limiting on the visitor, not the remote address (#74)

Every counter was keyed on RemoteAddress::getRemoteAddress(). Behind a proxy or
CDN — the default Magento setup, where remote_addr_headers is empty — that is one
address for the whole shop, so ten violations from a single abuser tripped
BLOCK_THRESHOLD and locked the helpdesk for every visitor for an hour. Configure
remote_addr_headers instead and the value comes from a client-supplied header,
which makes the limit bypassable by rotating it.

Counters are now keyed on the customer id, falling back to the session id, and
only to the remote address when there is no session at all. A shared address no
longer collects other people's violations, and a forwarded header no longer
resets anyone's bucket.

The trade-off: a guest who discards the session cookie between submissions starts
a fresh bucket, which an address-keyed counter would have caught on a shop with
direct connections. That case belongs to the CAPTCHA the module already wires
into both forms, not to a counter that can lock out real customers.

blockIp()/unblockIp()/isBlocked() keep their names — they are public API — but
now act on the resolved identifier, and the log context and the visitor-facing
message no longer say "IP address".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 541e3a

fix(cron): keep e-mail pickup running when one message or mailbox fails (#74)

The try wrapped the whole loop over a mailbox and its catch did `return false`,
leaving execute() entirely. One message that threw — bad encoding, a missing
header, a failing save — therefore aborted the department it was in and every
department queued after it, on every cron run, until someone removed it by hand.

The catch is now per message, and fetching the message happens inside it, since
that is where a malformed message throws. Opening the mailbox is wrapped
separately so an unreachable IMAP host skips only its own department.

Messages are deleted in one sweep after the loop, so a department that failed on
any message keeps its whole mailbox and logs why. Catching and continuing without
that would have deleted the message that was never processed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 7aa1b7

fix(upload): log the attachments that receive no content inspection (#74)

validateFileContent() returns early for any extension outside KNOWN_MIME_TYPES,
so an extension the admin adds to the allow-list — xsl, csv, odt — reaches disk
with nothing but the extension whitelist behind it. The doc block already told
the reader a notice was logged in that case; nothing was.

The early return stays. Refusing unknown extensions outright would break every
installation that extended the list on purpose, and there is no execution path
to abuse today: attachments are written under var/, outside the document root,
and are served by Controller/TicketMessage/File with nosniff and a download
disposition. What was missing is the operator's ability to see it happening, so
the notice carries the file, the extension and the MIME that was sniffed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 813258

fix(admin): require the ticket ACL resource on the ticket grid controller (#74)

Grid declared neither ADMIN_RESOURCE nor _isAllowed(), so Magento fell back to
Magento_Backend::admin and any logged-in admin user could pull the ticket
listing, including the customer-filtered view, without the Swissup_Helpdesk::ticket
permission. Every other controller in Controller/Adminhtml declares one.

The unused Swissup\Askit import went with it; Askit is not a dependency of this
module and the symbol was never referenced.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 367d03

fix(file): sign attachment links with a real secret and stop previewing documents (#73)

The download link's hash was the sha256 of the message's created_at, with no
salt and no key. validateHash() takes the salt out of the hash it is handed, so
the timestamp was the whole secret — and it is printed next to every message as
"x minutes ago". Anyone who knew the message id, the stored path and roughly
when it was written could produce an accepted link, and the hash was the only
authorisation the action had.

Links are now signed with an HMAC over the message id and the file path, keyed
with the installation's crypt key. Every configured key is accepted on
validation and the newest one signs, so rotating the key does not break links
already sent out.

Inline preview no longer serves text/html or image/svg+xml. From the shop's own
origin those are documents that run script in the visitor's session, not
pictures; they are downloaded instead. What is still previewed is sent with
nosniff and a restrictive Content-Security-Policy.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> ccccd9

fix(upload): require POST, a form key and the rate limit on the upload endpoint (#72)

The endpoint wrote a file to var/helpdesk with no checks at all: it ran on GET,
validated no form key, and never called the rate limiter, although a file_upload
bucket had been declared for it since the limiter was added. An anonymous client
could fill the volume in a loop, and the endpoint was reachable cross-site.

It now refuses anything but POST, validates the form key, applies the declared
limit, and — like the ticket and message controllers — only serves guests where
guests may open a ticket at all. Refusals come back as JSON with a 400 so the
uploader can show them, instead of an exception page for an ajax call.

The uploader's error branch was registered as `fail`, which jQuery.ajax never
calls, and used `this` where the widget was meant; it now reports the server's
message.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 5fd3ca

fix(message): run only backend-authored text through the CMS filter (#71)

The CMS page filter renders raw HTML and executes {{...}} directives. Every
message went through it when the option was on, including text written by
customers, guests and inbound e-mail, which is a stored XSS path against the
customer and against any agent opening the ticket.

Authorship now decides, not the option alone: text an administrator wrote in
the backend is still rendered as rich content, everything else is escaped.
An administrator's address on an incoming e-mail does not count, because
From: is not authenticated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 11a308

Version 1.4.10 f95e5c

Merge pull request #76 from swissup/fix/inbound-email-sender-70

fix(cron): do not append inbound e-mail to a ticket the sender does not own (#70) 62b851

Merge pull request #75 from swissup/fix/ticket-reply-ownership-69

fix(message): reject replies to tickets the visitor does not own (#69) e35a26

Merge remote-tracking branch 'composer/master' into fix/ticket-reply-ownership-69

# Conflicts:
# Controller/TicketMessage/Save.php 5b8a62

Merge pull request #77 from swissup/chore/drop-non-english-comments

style: drop the Ukrainian and emoji comments from the security hardening d428a9

fix(cron): guard the remaining header accessors and the blacklist (#70)

Review of this branch found the "no longer throws on a missing From:" claim was
wrong, and that the blacklist had the same first-address weakness the ownership
check had just been fixed for.

`getFromAddresses()` was guarded, but `getFrom()` was not, and `PickUp` calls it
one line earlier — so a message with no From: still threw out of `execute()` and
still took pick-up for every remaining department with it (#74). `getSubject()`
had the same shape and is reached for the ticket number and the new ticket's
title, so a message with no Subject: did the same. Both return an empty string
now, and `getFromName()` goes through `getFrom()` rather than reading the header
itself.

The blacklist only ever saw the first address, so `From: clean@example.com,
blacklisted@evil.test` walked past it while the message was delivered from the
blacklisted mailbox. Every address in the header is checked now.

`getFromAddresses()` caches its result and `setMessage()` clears it, so the
header is parsed once per message instead of three times.

The test for a header-less message now covers every accessor the cron calls on
the parser, which is what would have caught `getFrom()`: it fails on the
previous commit with "Header with Name from or from not found" at Parser.php:243.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 175135

fix(i18n): add the ticket-not-available message to en_US.csv (#69)

The string this branch adds to `Controller/TicketMessage/Save` was the only
user-facing one in that controller missing from the translation file, so it
stayed English on every other locale.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 7cdd8c

style: keep the single-line @var hints and the ResourceModel note

Review of this branch found the cleanup took three things it should not have.

The doc block tidier matched `/** ... */` with DOTALL, so a block that opens
and closes on one line looked like a block whose content had all been removed
and was dropped whole, whatever language it was in. That deleted all nine
`/** @var ... */` type hints in the controllers and the observer — the ones
PHPStan and the IDE need on `resultFactory->create()` and
`$transport->getMessage()`. The tidier now leaves single-line blocks alone.

`TicketDataFilter::ADMIN_ADDITIONAL_FIELDS` loses its Ukrainian notes but keeps
the fact they carried, in English: those four keys are consumed by
`ResourceModel\Ticket::_beforeSave()`/`_afterSave()`, and dropping them from the
whitelist silently discards the data before it reaches them. That is not
visible from this file.

`// varchar(128)` and `// primary key` are gone too. They had survived only
because they happened to be English while every neighbouring entry lost its
note, which read as arbitrary.

Still comment-only: with comments and whitespace removed, the token stream of
all 12 files is identical to master.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 0e443b

style: drop the Ukrainian and emoji comments from the security hardening

The October hardening wave left its working notes in the code — Ukrainian
prose and ✅/⚠️ markers in `// ЗАХИСТ:` comments across the controllers, the
Service filters, the mail classes and the data patch. 120 comment lines are
removed and 61 more lose a Ukrainian trailing note or description.

Doc block tags keep their type and variable: a `@param array $fields` whose
only description was Ukrainian becomes `@param array $fields` rather than
disappearing, so no API documentation is lost. A block left with nothing but
tags keeps the leading `*` line this module already uses elsewhere.

Comment-only: with comments and whitespace removed, the token stream of all
12 files is byte-identical to master.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> fbc509

style(security): drop the explanatory comments from the access check (#69)

The reasoning stays in the commit messages and the pull request.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 9e649d

style(security): drop the explanatory comments from the sender check (#70)

The reasoning stays in the commit messages and the pull request.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 94e19b

fix(security): take the sender from the parsed From:, scope staff to the department (#70)

Review of this branch found the new check could be walked around, and that it
left the staff half of the finding open.

The sender was read with `preg_match('|<(.*?)>|')` over the raw From: header —
the first bracketed address wins. RFC 5322 allows more than one mailbox in
From:, and a display name may itself contain angle brackets, so either of

From: Owner <owner@example.com>, Attacker <attacker@evil.test>
From: "x <owner@example.com>" <attacker@evil.test>

made `getFromEmail()` return the victim's address while the message was
delivered from the attacker's, and the new ownership check then let it into the
victim's ticket. Verified against laminas-mail: both headers yield
`owner@example.com` from the old code.

`getFromAddresses()` now asks Laminas for the parsed address list and reports
every address it holds, so a header naming several mailboxes — or a header sent
twice — stays visible as more than one sender instead of collapsing to the
first. `hasSingleFromAddress()` is what the cron holds the message against: with
no single sender the message reaches no existing ticket, is never attributed to
staff, and opens a ticket of its own without being bound to a customer account.
`getFromEmail()` keeps its contract for the blacklist and the staff lookup, and
no longer throws when the header is absent, which used to abort pick-up for
every remaining department.

Staff status is no longer a blanket pass. It is decided against the users of the
department whose mailbox received the message, which says nothing about a ticket
belonging to another department, so `isAllowedOnTicket()` now also requires the
ticket to be that department's. Without this, any address matching any
department user reached every ticket in the shop and was stored as a staff
reply — finding 2 of the issue. The remaining exposure is an unauthenticated
From:, which needs SPF/DKIM at the MTA.

The new ticket opened for a rejected message now records in its notes which
ticket the message had referenced, so the two can be linked from the grid
rather than only from the log.

Parser tests skip where laminas/laminas-mail is not installed, as it is an
optional dependency.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 157509

fix(security): tighten the ticket access check after review (#69)

Three points from the review of this branch:

* The guest e-mail is now taken from the session only. The reply form never
posts an `email` field, so accepting one from the request added a way to
reply without ever passing through `Ticket/View`, where the address is
checked and remembered. Nothing legitimate used it.

* E-mail addresses are compared with `strcasecmp` instead of `===`. The
ticket keeps the address exactly as it was typed into the form, so an
owner coming back with a differently cased spelling was being refused.
This also makes the rule the same as the one `InboundMailSender` applies
to incoming mail.

* A non-string e-mail is treated as no e-mail rather than cast. `email[]=x`
arrives as an array, and `(string) $array` raises "Array to string
conversion", which Magento turns into an exception in developer mode — a
500 instead of a refusal.

The unit tests now set `customer_id` and `email` as data and let the real
Ticket accessors run, instead of mocking them. `getCustomerId()` casting to
int is what makes the strict comparison correct, and the previous mocks
would have stayed green without it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 3904ce

fix(cron): do not append inbound e-mail to a ticket the sender does not own (#70)

Pick-up resolved the ticket from In-Reply-To or from the number in the
subject and appended the message without looking at the sender, so a
forged From: could post into another customer's ticket — as staff, if
the address matched a department user.

The sender now has to belong to the ticket. When it does not, the
reference is dropped and the message opens its own ticket, with a
warning in the log: nothing is lost and nothing lands in a conversation
it does not belong to.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 10123f

feat(security): add sender check for inbound e-mail (#70)

Answers whether an unauthenticated From: may be appended to an existing
ticket: staff of the receiving department, the address the ticket was
opened with, or the account it belongs to. Covered by unit tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 22fee1

fix(message): reject replies to tickets the visitor does not own (#69)

The reply controller loaded the ticket by its public number and appended
the message without ever tying the ticket to the session, so any visitor
holding a valid form key could post into another customer's ticket.

The ticket now has to pass the same owner check the view controller
applies, and an unknown ticket and a foreign one give the same answer.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 9428a3

refactor(ticket): take the view controller's owner check from the shared service (#69)

No behaviour change: the same two rules, now expressed once so the view
and the reply controller cannot drift apart.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 06c155

feat(security): add ticket access checker shared by view and reply (#69)

A single place that answers "may this visitor act on this ticket":
a customer ticket belongs to its customer, a guest ticket to whoever
presents the e-mail it was created with. Covered by unit tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> 057646

Better dependency resolving inside bundle (checkout tests) cc25ca

Added missing methods to checkout components 3a587f

Fixed possible js error when calling debug early 689d66

Improve runtime nested deps resolving (A > B > C > D) 85a9bd

Read deps from requirejs-config when better compat is enabled 8eb715

Added missing `$.Event` be2d4d

Fixed missing child components at checkout page 37df49

Fixed checkout integration errors a45aac

uiComponents: Fixed empty provider when dataScope is not set a9dfcf

Prepare nested children as well. Fixes checkout flatrate validation 9905d7

Do not ignore rjsResolver anymore 7e40af

CheckoutLoader component 042f34

Align `$.Deferred` with jQuery API c9f87f

Align `$.request` with jQuery API 8aa15c

Improve `:visible` selector support 91a9ff

Fixed undefined child in elems when uiLayout component is not loaded yet 345194

Added `repeat` knockout binding (payment methods list) ba27e4

Wait for mixins before resolving a dependency e24fd4

Custom modal should not render lazily (checkout order summary) 3bfe11

Fixed infinite focus loop when validating single field (login popup) 59cd0c

Allow using `:visible` hack for multiple selectors 268c1a

Fixed mixins not applied to module dependencies when loaded asynchronously 5d91b8

Basic implmentation of `ko.datepicker` 05df54

Align date with moment.js 8ce8c3

Align calendar with jQueryUI f1cd4a

Align ui cmp initialization with Magento. Do not pass element.
This fixes `google-pay-cart` logic. d292d3

Implement `ko.fadeVisible` 4c14f6

Implement simplest version of `rjsResolver` 3440bb

Fix possible js error: read source synchronously first a77cb4

Align js cmp with Luma: Don't overwrite existing fn with `undefined` baea61

Added `validation.check` method 531321

`$.grep` implementation c5ec90

Fixed possible js error `Cannot read properties of undefined` fb7bef

Do not load js component if it uses `componentDisabled: true` cd0baf

Version 2.32.1 c5afe6

Prevent infinite loop when head contains invalid stylesheet link 582a77

Fixed incorrect magnifier image when using expanded gallery fc2f6b

Prevent auth popup form submit when recaptcha is not ready yet
Closes #101 740ada

Fixed incorrectly marked lazyload when main image is not the first one d5cd6b

Do not add dom duplicates to $.registry 84beb6

Version 1.21.1 272eb4

Merge pull request #138 from swissup/fix/critical-css-review-followups

fix(critical-css): review follow-ups for #137 (#133) e6d505

fix(defer-css): trust the built-in critical path, keep Breeze preloads (#133)

Breeze prints its critical CSS as a plain <style> without
data-type="criticalCss", so the DOM check from the previous commit made
DeferCss skip every Breeze page: stylesheets stayed on Breeze's
media="print" fallback instead of rel="preload" (lowest fetch priority).

Look for the criticalCss node only when dev/css/use_css_critical_path is
off; with it on, Magento or Breeze provides the critical CSS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> c9ddc5

fix(critical-css): keep Default Critical CSS from closing its style tag (#133)

The default CSS is printed raw inside <style>; a "</style" sequence in the
admin value would end the tag early and let the rest render as HTML.
Escape it, add the missing docblock summary flagged by phpcs, and cover
the block with unit tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> 47fb93

fix(defer-css): do not defer stylesheets when the page has no critical CSS (#133)

Critical CSS enabled with the built-in path off and an empty Default
Critical CSS still turned every stylesheet into rel="preload", leaving the
page unstyled until they loaded. DeferCss now runs only when a non-empty
<style data-type="criticalCss"> is present in the document.

HTML fragments without critical CSS are no longer deferred either; the
tests now inject a critical style block into their fixtures.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> 1f6912

Merge pull request #137 from swissup/fix/default-critical-css-without-builtin

fix(critical-css): output Default Critical CSS without built-in critical path (#133) 72ae02

fix(critical-css): output Default Critical CSS without built-in critical path (#133)

With "Use built-in critical CSS feature" off, DeferCss still deferred the
stylesheets, but the Default Critical CSS never reached the page: it was
only appended through a plugin on Magento's Header\CriticalCss block, which
Magento renders solely with dev/css/use_css_critical_path. The page got
deferred CSS and no critical CSS.

Up to 1.9.9 the CriticalCss optimizer inserted the default CSS itself;
27ecf46 ("Fix criticalCss dublication", 1.10.0) dropped that path.

Add a head.critical block that renders the default CSS as
<style data-type="criticalCss"> only when critical CSS is enabled for the
page and the built-in path is off, so the CriticalCss optimizer still
minifies it and the built-in path cannot produce a duplicate.

Fixes #133

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> b98e40

Version 1.21.0 28f0bf

fix(installer): do not enable Advanced JS Bundling by default 19fc09

Merge pull request #134 from swissup/feat/extract-advanced-js-bundling

refactor: move Advanced JS Bundling to swissup/module-advanced-js-bundling 343655

fix: keep deprecated config path constants, skip the order test without the new module

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> 9d53ff

Merge master into feat/extract-advanced-js-bundling

# Conflicts:
# composer.json 1714b2

Version 1.2.0 904b04

Version 1.1.13 879c90

Added Apple login, see #7 3fedc9

Version 1.1.13 97eeb1

A social account is now permanently tied to one customer, linked accounts stored in separate table 46e8bd

Added Apple login, see swissup/module-social-login#7 63067b

Improve isApiRequest logic so it don't return true on direct link 087787

Version 1.2.4 a81b40

Merge pull request #5 from swissup/fix/password-injection

Fix password injection: don't read registration password from session in repository plugin bef991

Password injection vulnerability fix: don't read password from session

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> fab4c1

Do not change header cart styles on the checkout page 7d5dec

Version 1.36.1 e818c9

CSS fixes for checkbox inside address and address lookup button f5c3e8

Implement labels loading and updating for configurable products with AJAX support at listing as well (close #2) c90c71

Add AJAX controller for fetching product labels and update label handling logic 33072b

Version 1.0.10 b8b88d

Fix child labels ignoring schedule, store and product status

- Skip expired labels and pass active period to frontend
- Filter index by store view, skip disabled and out-of-website children
- Validate $mode before using it in SQL column names
- Require swissup/module-pro-labels ^1.10.0 (active period columns) 3074b5