* FIX phpstan: Societe::$client (int<0,3>) does not accept -1 (#39244)
Societe::$client is annotated @var int<0,3> (domain 0=no customer,
1=customer, 2=prospect, 3=both) in societe.class.php, but the create
form in societe/card.php pre-set it to -1 as a transient "nothing
pre-selected" placeholder, tripping PHPStan level 10 on develop.
The value is never persisted (save paths compute 0..3) and the only
reader, the switch() building the form, treats -1 and 0 identically via
its default case. Use 0 (the property's own default) so the assignment
matches the declared type. No behavior change.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* Qual: Fix entity handling in bonprelevement.class.php
The variable name 'entities' was changed to 'entity' to better reflect the singular nature of the value being retrieved.
It was also casted to an int instead of using an unquoted db->escape.
* Qual: Improve SQL query construction in company.lib.php
By escaping the concatenated string, injection verification does not identify a false positive.
* Qual: Avoid void positive, sanitize instead of escape
* Qual: Avoid false positive, sanitize instead of escape
* Sec: Fix potential injection with cast to int (timespent_duration)
* Qual: Avoid false positive, sanitize instead of escape
* Qual: Avoid false positive, sanitize instead of escape
* fix: Replace escape() with sanitize() in SQL queries
# FIX: Replace escape() with sanitize() in SQL queries
- Replace escape() with sanitize() in SQL queries to ensure proper SQL injection protection
- Update SQL queries in multiple files to use sanitize() instead of escape()
- Ensure consistent use of sanitize() across the codebase for better security
* fix(sql): Replace escape with sanitize in SQL queries
# FIX: Replace escape with sanitize in SQL queries
- Replace `escape` with `sanitize` in SQL queries to ensure proper SQL injection prevention
* Qual: Improve field escaping/quoting
# Qual: Qual: Improve field escaping/quoting
---------
Co-authored-by: Jam Balaya <jambalaya.pyoncafe@outlook.jp>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The MCP server (htdocs/ai/server/mcp_server.php) loads a service user
from AI_MCP_USER_ID but does not expose it as the PHP global $user.
Several tool classes call 'global $user' to identify the caller --
they therefore see no authenticated user in HTTP MCP context (no PHP
web session exists) and return 'User not authenticated' even though
authentication via X-API-Key/Bearer has already succeeded.
This breaks roughly 40% of the MCP tools shipped by the module when
called from external clients (Claude Desktop Custom Connectors,
Claude Code CLI, MCP Inspector, custom scripts).
Two complementary fixes applied:
1. mcp_server.php: after successfully loading the service user,
assign it to $GLOBALS['user'] and the local $user variable. This
acts as a safety net for any tool (current or third-party) that
relies on the legacy 'global $user' pattern.
2. Three tool classes harmonize their constructor signatures with
the McpHandler contract -- new $className($db, $user, $conf):
- ToolCrudObjects (htdocs/ai/tools/crud_objects.class.php)
- ToolInvoices (htdocs/ai/tools/invoices.class.php)
- ToolReports (htdocs/ai/tools/reports.class.php)
These now accept $user via constructor (matching the pattern
already used in ToolThirdParty, ToolCategories, ToolProducts)
and store it as $this->user. Methods that previously called
'global $user' (execute, validateInvoice, payInvoice) now use
the injected $this->user, with a fallback to the global for
web-session contexts.
Tested with:
- create_other_document (proposal, supplier_invoice): works
- search_invoice, validate_invoice, pay_invoice: works
- get_sales_report, get_purchase_report: works
- AI Assistant web UI: unchanged, no regression
* Qual: Fix Phan notices, set PHP min to 7.2 to avoid some
* Qual: Fix Phan and PHPStan notices
* Update apstats.php
---------
Co-authored-by: Laurent Destailleur <eldy@destailleur.fr>
Unlike Facture::addline() which takes ($desc, $pu, $qty, $txtva, ...),
FactureFournisseur::addline() uses a different parameter order:
($desc, $pu, $txtva, $txlocaltax1, $txlocaltax2, $qty,
$fk_product, $remise_percent, ...)
i.e. $qty is in position 6, not 3.
ToolCrudObjects::processAddLine() was calling
FactureFournisseur::addline() with the Facture order, which produced:
- our $qty (1) passed as $txtva -> effective VAT rate 1%
- our $vat (20) passed as $txlocaltax1 -> wrong local tax
- a hardcoded 0 passed as $qty -> the line was inserted with
qty=0 and silently dropped from the visible totals
The result: every supplier invoice created via create_other_document
ended up with the header correctly created but a 0.00 EUR total,
with no error returned (lines_added was reported as success).
This patch reorders the parameters to match the actual
FactureFournisseur::addline() signature and adds an inline comment
to warn future maintainers about this Dolibarr API quirk.
Other addline calls in the same switch (invoice/order/proposal,
supplier_order/supplier_proposal) use their respective correct
signatures already and are unaffected.
Tested with: create_other_document object_type=supplier_invoice
with one line (qty=1, unit_price=10, vat_rate=20) -> line now
appears with Total HT 10.00 EUR.
ToolCrudObjects::processAddLine() uses the line's 'description' as a
fallback product identifier when 'product' is not explicitly provided.
It then calls findProduct() to look up an existing Dolibarr product
by ref/barcode/label.
When no product matches (the common case for AI-generated lines that
describe a one-off service or a custom item), findProduct() returns
an error array and the function aborts WITHOUT ever calling addline().
The resulting document gets its header created but ends up with zero
lines and a 0.00 total.
This is contrary to standard Dolibarr behavior: free-text line items
(no product link, just a description and a price) are a first-class
feature of invoices, orders, and proposals.
This patch distinguishes between the two cases:
- Caller explicitly passed 'product': they wanted a specific product
-> still abort with the original error if not found (unchanged).
- Caller only passed 'description': it's a free-text line ->
silently fall through with $prod = null. Dolibarr then creates a
free-text line with the user-provided description, qty, unit_price
and vat_rate.
Tested with: create_other_document object_type=proposal containing
11 line items, none of which match existing product references ->
all 11 lines are now created with their correct totals (previously
the document was created with 0 lines and a 0.00 total).
create_customer_invoice and create_other_document declare a 'header'
object whose schema has 'required: [socid]' but no 'properties'
entry. This is technically allowed by JSON Schema but stricter
validators flag it as suspicious: the 'required' array references a
property that is not declared in 'properties'.
Beyond compliance, the omission means LLMs (and human reviewers) have
no machine-readable description of which fields are actually accepted
in the 'header' object. The Dolibarr backend already accepts socid,
date, duree_validite, note_public, note_private and others -- this
patch documents the most useful ones explicitly so consumer tools
can produce richer payloads.
Tested with: create_other_document object_type=proposal accepting
a 'header' that contains all of socid + date + duree_validite +
note_public -> all four fields are correctly mapped to the Propal
object (Dolibarr already accepts unknown properties via dynamic
property assignment, so this is purely a schema-documentation change).