Resolve language-file conflicts by taking master's English-removal
cleanup across non-English locales, while stripping the MailChimp
keys master reintroduced (already moved to MailchimpPlugin on this
branch) and reapplying this branch's plugins/integrations rename and
new Module.php plugins keys where master lacked them.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Replace untranslated English strings with empty fallback ('') across all non-English catalogs and sort keys alphabetically (closes#4783, closes#4660)
- Restore missing Sales.no_sales_to_display in English while preserving existing translations (closes#4794, references #4637, #4795)
- Register Georgian (ka) and Swahili (sw-KE, sw-TZ) locales, normalize Croatian to hr per BCP 47, and fix datepicker long month keys (partially addresses #4476, references #3891)
- Hardcode invariant brand constants (OSPOS, website, copyrights) in login/footer views, remove from catalogs, and standardize Latin OSPOS branding
- Fix English catalog typos and maintain consistent British English variants in en-GB
- Add LanguageFilesTest with 4,418 automated tests for syntax, key parity, sorting, and bidirectional locale registrations
- Update AGENTS.md with localization guidelines and development best practices
Introduce a CashupAmounts transfer object and break down large Cashups
controller methods into focused private helpers. Includes a breaking
change to constructor parameter order.
- Add CashupAmounts (TransferObjects/Cashups) to bundle open/transfer/
closed cash, due, card, and check amounts; reorder constructor params
to cash-before-due sequence
- Replace positional _calculate_total() with calculateTotal(CashupAmounts),
eliminating argument-order errors
- Decompose getView() into buildEmployeeLookup(), normalizeCashupInfo(),
initializeNewCashup(), isPendingClose(), closeCashup(),
buildReportDateRangeInputs(), applyPaymentTotals(), subtractCashExpenses()
- Decompose postSave() with parseCashupDate() and getCashupAmountsFromPost()
helpers; update both CashupAmounts call sites to new param order
- Rename snake_case vars/params to camelCase throughout Cashups controller
- Drop stale TODO comments in OSPOS.php and Cashups::getIndex filter block
- Scope escape => false to cashup_headers() note column rather than a
broad regex exclusion in transform_headers()
- Add CashupFixtureTrait and ExpenseFixtureTrait under tests/Support;
tighten cashup view test to assertSeeInField on specific form fields
BREAKING CHANGE: CashupAmounts constructor positional argument order
changed — closedAmountCash now precedes closedAmountDue; any callers
passing positional args must swap those two arguments.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
- Extend composer.json and composer.lock PHP requirement to include PHP 8.5
- Add PHP 8.5 to test matrix in phpunit.yml and main.yml workflows
- Modernize php-linter.yml to use setup-php@v2 with parallel matrix for PHP 8.2–8.5
- Propagate PHP 8.2 lint failures via shell loop in find -exec + while retaining batching for 8.3–8.5
- Add permissions: contents: read and cancel-in-progress concurrency to php-linter.yml
- Remove obsolete Node.js and npm install steps from phpunit.yml
- Update phpunit.yml runner to ubuntu-latest and composer cache action to v4
- Address PHP 8.5 deprecations and syntax:
- Replace deprecated switch-case semicolon with standard colon in app/Config/Database.php
- Replace non-standard (double) cast with (float) in app/Libraries/Token_lib.php
- Remove obsolete curl_close() calls in app/Config/Validation/OSPOSRules.php and app/Libraries/Mailchimp_lib.php
- Update INSTALL.md, workflows README, and bug report issue template to include PHP 8.5
Resolve language file conflicts: take master's translations, strip
MailChimp phrases (MailChimp is now a plugin instead of core), and
retain plugin-system additions (plugins/plugins_configuration keys).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resolves objecttothis's concern in #4726: the version was set to 3.4.3 before
the release was tagged, causing vulnerability reports to reference a non-existent
release. Bump to 3.4.4-dev so master correctly references the development
version.
* fix(release): handle -dev snapshot suffix in release workflows
- release.yml: strip -dev from App.php version when determining release target
- release.yml: append -dev to the bumped next version (App.php, package.json,
package-lock.json all get the snapshot suffix)
- build-release.yml: strip -dev from version for tag builds so tag validation
and archive naming use the clean release version
Add a new detailed item sales report that shows all items sold per row
in an exportable format, including sale transaction information. The
existing detailed transaction sales report lacked per-item row display
and export support, and the items summary report did not classify by
sale.
- Wire sale and discount filters for the report
- Validate detailed item sales route filters
- Add English fallback translations for other languages
- Use blank quotes for other language translations
- Normalize language key placement
- Use PSR-style names throughout the report
* fix(taxes): reject invalid characters in tax code, category, and jurisdiction names
Apply the same unicode_alpha_numeric_punct validation already used for
item tax names to the three tax save endpoints, so stored-XSS payloads
(<, >) are rejected with a validation error instead of being saved.
Mirrors the Items fix and adds a TaxesControllerTest regression suite.
* fix(taxes): permit slash and CJK in tax names, add regression tests
The unicode_alpha_numeric_punct guard rejected '/' (char type Po), which
blocks legitimate slash-separated tax names such as 'GST/HST' and 'VAT/GST'
from being saved. Add '/' to the allowed punctuation set and document it.
Also add regression tests proving acceptance of a slash name (GST/HST) and a
CJK name (消費税) across the tax save endpoints. Addresses CodeRabbit review
on #4679.
* test(4679): adopt test{Method}_{Purpose} naming convention
Align test method names with the convention from #4730
(testPostSaveTaxCodes_RejectsMaliciousName, etc.), per review.
* fix(taxes): allow parentheses in names, make name optional
Address CodeRabbit review on #4733:
- Permit parentheses in tax names (e.g. "VAT (20%)"); slash was
already allowed, so both "GST/HST" and "VAT (20%)" now pass while
< and > are still rejected.
- Drop the rule from tax_code_name, jurisdiction_name and
tax_category so a code with a blank name can be saved, matching the
form (the form does not require the name).
- Add regression tests for the parentheses and blank-name cases.
---------
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(auth): return 401 JSON for AJAX requests on session timeout (#4696)
IsLoggedIn previously threw RedirectException('login') for every request
type. For XHR calls the browser transparently followed the 302 and fetched
the login page's HTML where JSON was expected, so the table silently
rendered empty and the failure looked like a broken sort.
- Server: IsLoggedIn::before() now returns a JSON 401 response when the
request is an AJAX call, keeping the redirect behavior for normal
navigations.
- Client: a global $(document).ajaxError handler in the authenticated
layout redirects to the login URL on a 401 response.
* fix(auth): run isLoggedIn before csrf for expired sessions (#4696)
An expired-session AJAX POST with an invalid CSRF token was hitting the
globally-configured csrf filter first, which throws a CSRF exception before
isLoggedIn can return its 401. Reorder the global before-filters so the
authentication check runs before CSRF validation; CSRF protection is fully
retained for authenticated requests.
---------
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(security): report unwritable .env.lock, make throttle limits configurable
envFileIsWritable() previously checked is_writable(.env) (the file), which passes in Docker even when the mutex file is root-owned by a prior env:provision run. It now checks the real write path: the directory (to create .env.tmp.* + .env.lock) and any existing .env.lock must be writable, so the app throws the clear 'run env:provision' error instead of crashing on 'Unable to open .env.lock'.
The Throttle filter now reads throttle.capacity / throttle.seconds from env (default 5 per 60s); capacity <= 0 disables throttling, so operators serving sequential HTTP clients (e.g. Zabbix) are not caught by the lockout.
* fix(security): address CodeRabbit review findings
- Throttle: validate throttle.capacity as an integer before treating a non-positive value as 'disabled', so a non-numeric value (e.g. 'five') falls back to the default instead of silently bypassing the lockout. Apply the same validation to throttle.seconds.
- envFileIsWritable(): also reject an existing non-writable .env on Windows (where rename() cannot replace a read-only destination); keep the check Windows-only since POSIX rename() replaces a read-only dest when the directory is writable.
- Tests: restore the prior throttle.capacity env state in ThrottleTest (capture/restore instead of delete); add an invalid-capacity fallback case; skip the not-writable fixtures when running as root (where is_writable() is bypassed).
* fix(migration): guard ConvertToCI4 key-write branches with envFileIsWritable()
The migration's 'no key' and 'CI3 key' branches called rotateEncryptionKey()/rotateEncryptionKeyTransaction() directly, bypassing the envFileIsWritable() guard that checkEncryption() uses. On a fresh Docker/Compose install where the web runtime cannot write /app/.env.lock, this produced a raw 'fopen(/app/.env.lock): Permission denied' error instead of the actionable 'run php spark env:provision' message.
A valid CI4 key now short-circuits to checkEncryption() (no write); every write branch is gated on envFileIsWritable() first.
* fix: read provisioned encryption.key so config sees it
env:provision persists the key as 'encryption.key' in .env (matching the
throttle path and .env.example), but Config\Encryption only read the
ENCRYPTION_KEY env var. On a fresh Docker instance the provisioned key was
invisible to config('Encryption')->key, so the app believed no key existed
and tried to write one -- hitting the .env.lock permission wall.
Read encryption.key (via $_SERVER/$_ENV/getenv) first, then fall back to
ENCRYPTION_KEY for Docker '-e' usage. Mirrors checkThrottleEncryption().
* fix: cascade encryption.key lookup past empty-string sources
* refactor: extract Encryption::resolveKey() and test it without global env mutation
* fix(security): decode fallback-selected encryption key like BaseConfig
A key picked in the constructor fallback (notably ENCRYPTION_KEY, which
BaseConfig never inspects) was assigned verbatim, bypassing the
hex2bin:/base64: decode the parent applies to `encryption.key`. Route the
selected key through a parseKey() helper mirroring BaseConfig's
parseEncryptionKey() so prefixed values decrypt consistently, and add a
pure regression test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore: remove advisory ID from comments and tighten verbose comments
Per review: treat GHSA advisory IDs like secrets (drop from code) and
replace the multi-paragraph comments with concise one-liners that keep
the non-obvious "why". No logic changes.
---------
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per the project's policy of treating security advisory IDs as
secret-like, remove the identifiers embedded in source/test comments
and CHANGELOG entries. Each keeps its human-readable description (and
PR number where present), so traceability is preserved. No logic changes.
* fix(security): handle special characters in `.env` key values and improve insertion logic
- Escape backslashes and dollar signs in `applyEnvKeyReplacement` to prevent unintended value corruption.
- Ensure new keys are inserted after `encryption.key` for better organization and manageability.
- Add explicit cast to int to prevent wrong concatenation operator warning.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(security): handle null return in `applyEnvKeyReplacement` and ensure proper `.env` updates
- Update `applyEnvKeyReplacement` to return `null` on failure, improving error handling.
- Adjust calls to `atomicWriteFile` with updated content to prevent unintended behavior.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(security): improve error logging and exception messages in file locking
- Add detailed logging for file open and locking errors in `security_helper`.
- Remove unused `helper` and `checkThrottleEncryption` calls from `Events` for cleanup.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(security): improve atomic file write and handle encryption key placement
- Throw `RandomException` for better error reporting in `atomicWriteFile`.
- Simplify Windows-specific `rename()` fallback logic.
- Fix `encryption.key` assignment order to ensure consistency in `.env` updates.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(security): improve `.env` file handling and add unit tests for helper functions
- Suppress warnings in `file_get_contents` to prevent unnecessary error logs.
- Update `applyEnvKeyReplacement` to use `preg_replace_callback` for better safety.
- Add comprehensive unit tests for `security_helper` functions to ensure `.env` updates and key management work as expected.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(security): enhance `.env` update logic and add robust exception handling
- Add `RandomException` to improve error reporting in encryption key management.
- Introduce environment file locking for safer `.env` updates.
- Ensure `applyEnvKeyReplacement` properly handles and inserts old key comments.
- Replace direct file writes with `atomicWriteFile` for consistency.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(security): refactor `.env` file initialization and encryption key handling
- Introduce `initializeEnvFile` for reusable `.env` setup logic.
- Add `backupEnvFile` and `writeNewEncryptionKey` for robust key management with backups.
- Simplify and clean up redundant `.env` handling code paths.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(security): clarify `checkEncryption` docblock return value description
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(security): escape backslashes and dollar signs in `applyEnvKeyReplacement`
- Ensure `applyEnvKeyReplacement` properly escapes special characters when inserting or appending `.env` keys.
- Add new unit tests to validate correct handling of backslashes and dollar signs.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(i18n): add localized error messages and improve error reporting in `security_helper`
- Add missing translations for error messages across multiple language files.
- Update `security_helper` to use localized exception messages with placeholders.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* Redesign encryption/throttle key provisioning as read-only runtime
- checkEncryption()/checkThrottleEncryption() are now read-only guards that
throw when no valid key is provisioned, instead of writing .env at
request time.
- Add rotateEncryptionKey() and provisionThrottleKey() for explicit,
idempotent provisioning.
- Add php spark env:provision (app/Commands/EnvProvision.php) so Docker can
provision keys once at container startup before any request.
- Add app/Libraries/CI3SecretConverter.php shared CI3->CI4 secret converter
(AES-128-CBC decrypt + CI4 re-encrypt/verify/save) used by both the
interactive migration and the docker startup path.
- Refactor convertToCI4 migration to use the shared converter.
- Persist .env in a named volume and run spark env:provision on boot; stop
baking .env into the shipped image.
- Add guard/rotation/throttle + converter tests; clean up orphaned
msg_pwd_required language keys across all locales.
* fix: save CI4 ciphertext in env:provision and bind-mount a .env file
Addresses CodeRabbit review on PR #4656:
- env:provision CI3 branch was persisting *plaintext* secrets (saveAll($plain))
instead of the CI4 ciphertext, unlike the ConvertToCI4 migration. Now
encrypts with encryptAll(), verifies the round trip, and saves the ciphertext.
- The ospos_env named volume mounted at /app/.env made .env a directory, so
atomicWriteFile's rename() failed and spark env:provision could not start apache.
Switch to a bind mount of a host file (./.env) which persists and stays a file.
- Add a regression test asserting the command persists ciphertext (not plaintext).
* chore: trim redundant docblocks in EnvProvision and provision throttle.key in CI
Follow up on @objecttothis review comments:
- app/Commands/EnvProvision.php: remove the boilerplate docblocks the
property names already convey (group/name/usage/description, run()),
the two inline step comments, the anyNonEmpty() param docblock, and the
legacySecretsPresent() docblock. Keeps the class-level docblock since it
is the only place that states the read-only runtime design + the
never-persist-plaintext invariant.
- .github/workflows/phpunit.yml: provision a per-run throttle.key the same
way the encryption key is already provisioned. The PR makes
checkThrottleEncryption() a read-only guard that throws when
env('throttle.key') is unset; CI only started exporting ENCRYPTION_KEY,
so every test that goes through the Throttle filter (7 ThrottleTest
cases + 4 LoginTest cases) failed with
"No throttle key is provisioned. Run `php spark env:provision`".
Writing `throttle.key=<KEY>` into .env matches what
`php spark env:provision` does on a real container start.
* fix(ci): write throttle.key into .env instead of exporting an OS env var
The previous attempt exported throttle.key via GITHUB_ENV, but CodeIgniter's
env() helper resolves in the order $_ENV[$key] ?? $_SERVER[$key] ?? getenv($key),
and DotEnv populates $_ENV['throttle.key'] from the .env file first. Because the
.env (copied from .env.example) ships with the empty placeholder throttle.key='',
that $_ENV entry exists as '' and short-circuits the ?? chain before getenv()
is reached — so the OS env var was never consulted and every Throttle/Login test
still threw 'No throttle key is provisioned'.
Write the per-run key into the .env file itself (sed-replacing the empty
placeholder), which is exactly what `php spark env:provision` does in
production and is the single source env() actually reads from.
Verify the replacement happened (grep -Eq '^throttle\.key=.') so a future change
to the placeholder format fails the run loudly instead of silently breaking
the 11 throttle-dependent tests.
* fix(security): restore CI3->CI4 auto-provisioning gated by .env writability
checkEncryption()/checkThrottleEncryption() again provision the keys
inline when .env is writable (empty key -> generate; short key -> decrypt,
rotate, re-encrypt, verify, persist legacy CI3 secrets). When .env is not
writable they assume the key was provisioned externally (e.g. docker
env:provision) and throw. Update helper tests to match and correct the
EnvProvision docblock that claimed the runtime was strictly read-only.
* test(security): make short-key conversion branch injectable and test it
checkEncryption() now accepts an optional CI3SecretConverter so the
CI3->CI4 conversion branch can be exercised in unit tests without a
database. Adds testCheckEncryptionConvertsCi3ShortKeyWhenEnvWritable
which seeds CI3-era ciphertexts via a fake Appconfig model and asserts
the key is rotated and the payload verifies back to the original
plaintext.
* fix(security): abort on backup/read/saveAll failure to avoid data loss
Three related data-integrity fixes:
- backupEnvFile() now returns true/false based on whether the backup
actually exists and is readable. rotateEncryptionKey() aborts before
destroying the key when the backup could not be written to disk.
- rotateEncryptionKey() and provisionThrottleKey() throw
RuntimeException(Error.unable_to_read_env_file) when the .env read
fails, instead of silently replacing the whole file with an empty
string. This prevents a permission error from wiping all keys.
- checkEncryption() and EnvProvision::run() now both roll back to the
backup with abortEncryptionConversion() when the post-rotation
saveAll() throws, matching the migration path (which already did this).
A failing fake Appconfig is used to exercise this in the new
testCheckEncryptionRollsBackWhenSaveAllFails test.
* fix(ci): skip comment job in deploy-pr.yml when prepare was not run
The comment job had if: always(), so it ran even when the prepare job
was skipped (e.g. review was not approved). With PR_NUMBER empty the gh
api call posted to issues//comments, received a 404, and the entire run
showed up as failure. Guard the job with
needs.prepare.result == 'success' so it only runs when PR_NUMBER is valid.
* address coderabbit open items: placeholder guards, message neutrality, ar-EG alignment
- backupEnvFile(): fail when mkdir() or either chmod() fails, so the
pre-rotation backup is actually persisted before the key is replaced
- email/message config views: only show the 'already set' placeholder when
the secret is actually present (prevented false positives on fresh installs)
- Error.unable_to_create_env_file / .unable_to_read_env_file (en + en-GB):
use key-neutral wording since both keys are provisioned with the same keys
- ar-EG/Error.php: align all => arrows on the longest key
Item 7 (filesystem test isolation) is a larger refactor — the tests are
serial on CI and tearDown() restores state per test. Left for follow-up.
* test(security): isolate helper FS tests via Config\SecurityEnv
Introduce Config\SecurityEnv holding envPath/backupPath/lockPath so the
security helper reads its target paths from shared configuration instead of
hardcoded ROOTPATH/WRITEPATH literals. security_helperTest.php now redirects
all three to a unique per-run sandbox under sys_get_temp_dir() and tears it
down in tearDown(), so the suite no longer reads/writes the repository's real
.env and is safe to run in parallel.
No helper signature changes; production callers unaffected.
Addresses CodeRabbit item 7 (issue #4700).
Co-Authored-By: opencode <bot@opencode.ai>
* fix(security): run key-conversion as one locked transaction
Address CodeRabbit Major findings from the 4th re-review of the env
helper and its callers:
1. Hold .env.lock for the entire CI3 -> CI4 conversion transaction
(backup -> rotate -> re-encrypt -> verify -> persist -> cleanup) so a
concurrent worker cannot interleave a key write between the rotation
and the ciphertext save. Split rotateEncryptionKey into a lock-free
core (rotateEncryptionKeyUnlock) plus the existing lock wrapper and a
new rotateEncryptionKeyTransaction that owns the lock across the full
unit and performs both the in-lock rollback (abortEncryptionConversion)
and the in-lock backup removal on success.
2. Treat the legacy value '0' as non-empty data so key rotation still
persists the re-encrypted ciphertext when '0' is the only stored
secret (array_filter would have dropped it and skipped saveAll).
3. Wrap the post-rotation re-encrypt/verify/saveAll sequence in a
catch (Throwable) across all three call-sites so CI4
EncryptionException, ReflectionException from batch_save, a failed
round-trip verify, and any other failure all roll the .env key back
to the pre-rotation state.
4. In Docker Compose, use long-syntax bind with create_host_path: false
and document in INSTALL.md that the host .env must be a regular file
(a missing one is no longer auto-created as a directory, and the
mount now rejects a missing source on Compose implementations that
support the flag).
Files touched: app/Helpers/security_helper.php, app/Commands/EnvProvision.php,
app/Database/Migrations/20220127000000_convertToCI4.php, docker-compose.yml,
INSTALL.md. All 4 existing helper tests still pass via CI.
* fix(security): make abortEncryptionConversion fail loudly on restore failure
The rollback path restored the .env backup with a suppressed
file_put_contents() and an unchecked file_get_contents(). If the restore
failed after the key had already been rotated, .env was left holding the new
CI4 key while the DB still held CI3-era ciphertext, so the data became
undecryptable after the next restart.
Now the backup read is checked for false and the restore goes through the
existing atomicWriteFile() helper; either failure throws so the error is
surfaced instead of silently corrupting the config. Adds a regression test
that forces an unreadable backup and asserts the throw plus that .env is
left untouched.
* fix(security): guard abortEncryptionConversion backup read before touching it
Validate the backup is a regular readable file (is_file/is_readable) before
reading it, so a missing/malformed backup fails loudly instead of emitting a
file_get_contents() warning. The unreadable-backup regression test now
exercises this guard rather than relying on a promoted warning.
---------
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Co-authored-by: jekkos <jeroen.peelaerts@gmail.com>
Co-authored-by: jekkos <jekkos@users.noreply.github.com>
Co-authored-by: opencode <bot@opencode.ai>
- Broaden OSPOSRules filesystem path regex to allow space, colon,
and backslash for Windows sendmail paths while still excluding
shell metacharacters used unescaped in popen(); update docblock
accordingly.
- Restore/translate mailpath_invalid message across all language
files so users get a real error string instead of empty text.
- Guard Config controller license rendering against malformed or
non-array npm-prod/npm-dev LICENSES JSON, skipping entries
missing required keys instead of raising warnings.
- Document Node.js 20+ requirement in BUILD.md for license
reporting dependency.
- Remove security advisory IDs/URLs from INSTALL.md and code
comments per new AGENTS.md policy against referencing CVE/GHSA
identifiers anywhere in the repo.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
fix(validation): allow Windows sendmail paths, tighten shell metachar exclusions
Broaden PLAIN_FILESYSTEM_PATH_STRICT to accept real-world sendmail formats
while blocking command injection characters not needed in valid paths.
- OSPOSRules.php: allow space, colon, backslash for Windows paths
(e.g. C:\wamp64\...) and trailing args (-t -i); still excludes
ampersand, backtick, subshell, redirect, and cmd.exe metacharacters
- OSPOSRulesTest.php: add cases for Windows paths, trailing args, and
injection payloads
- Remove 7 ConfigTest assertions that expected metacharacter rejection;
add acceptance test for sendmail path with trailing args
i18n(lang): expand mailpath_invalid message across all locales
- Fill previously empty mailpath_invalid keys across all locales
- Update existing translations (de-CH, de-DE, es-ES, es-MX, fr, nl-BE,
nl-NL) to reflect newly allowed characters; nl locales corrected from
English loanwords to proper Dutch terms
- Add missing key to ckb/Config.php
docs: remove security advisory IDs from public-facing files
- AGENTS.md: extend no-advisory-ID rule to documentation and URLs
- INSTALL.md: drop GHSA reference and advisory link from Host Header
Injection guidance; rationale and fix instructions remain intact
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(tests): resolve all phpunit failures (#4626)
Bring the phpunit suite from 153 failures to 0 (281 tests passing):
- Employee: decouple grants block from save_value success; restructure
save_employee new-employee + disallowed-grants early return
- Sale: unify sales_payments_temp schema (add sale_cash_refund,
reference_code) so both creators produce an identical superset table
- Employees controller: provide placeholder password/hash in testing env
so new-employee insert succeeds and grant logic is testable
- TestDatabaseBootstrapSeeder: reset shared connection table-name cache
after bootstrap reset to avoid stale listTables()/tableExists() results
- Config: fix postSaveLocale validation rule syntax
- Test data: use unique employee usernames to avoid UNIQUE constraint
collisions latching strict-mode transStatus=false on the shared conn
- Various test-file and language-string corrections
* test: consolidate employee fixtures in shared trait
Route test employee creation through a single EmployeeFixtureTrait
that delegates to Employee::save_employee(), so fixtures exercise the
same production code path instead of raw DB inserts. Removes six
near-duplicate helpers across EmployeeTest, SalesControllerTest, and
EmployeesControllerTest while preserving each test's specific grant
set.
Closes a piece of the fixture-scattering flagged in #4626.
Closes#4626
* test: add global DROP/CREATE grant and commit theme fixtures
* fix(ci): remove redundant symlink step, set working encryption key
* fix(ci): run phpunit with --no-coverage to avoid no-driver warning
* fix: address code review findings
- Config: restore strict locale validation (min required|integer|>0) and
fix max cross-field check with a new gte_field rule (CI4's
greater_than_equal_to[field] does not resolve the field value)
- Tests: assert rejection for non-numeric/zero/negative/min>max limits
- .env.example: remove shared hard-coded encryption.key (auto-generates);
document Docker env-var usage
- phpunit.yml: scope CREATE/DROP grant to ospos_test.* and provision a
per-run encryption key as an env var
* feat: support ENCRYPTION_KEY env var for encryption key
Read ENCRYPTION_KEY as a fallback for the encryption key when the
config value is empty. This is a supported, reliable path for Docker /
container deploys and CI, avoiding reliance on the raw dotted
encryption.key env var.
* fix: align Summary_report temp tables with Sale temp table schema
Summary_report created sales_items_taxes_temp and sales_payments_temp with fewer columns than the canonical create_temp_table() in Sale.php. A later reader expecting those columns hit a schema-mismatch SQL error on the shared temp tables. Add internal_tax/sales_tax (sales_items_taxes_temp) and reference_code (sales_payments_temp) so all creators emit the identical column set.
- Introduce `valid_path_strict` rule in `OSPOSRules` to enforce stricter path validation, preventing security issues like injection attempts with newline or special characters.
- Update mail configuration validation in `Config` controller to use the new rule for the `mailpath` field.
- Add unit tests in `OSPOSRulesTest` to cover edge cases for `valid_path_strict`.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
bugfix(items, validation): reject unsafe tax names and fix payments temp table collision
- Add unicode_alpha_numeric_punct rule (OSPOSRules) to allow accented/CJK
chars in text fields while blocking HTML-unsafe chars (<, >) as
defense-in-depth against injection
- Items controller: extract validateItemFields/validateBulkUpdateFields,
validate tax_names on save and bulk update using new rule; add shared
validateFields helper in Secure_Controller to DRY up validation +
JSON error response
- Escape tax_group output in sales/quote.php and receipt_email.php views
to harden output encoding at render time
- Rename sales_payments_temp -> sales_report_payments_temp (Summary_report)
and -> sales_search_payments_temp (Sale model) to avoid name collision
between concurrently-created temp tables
- AGENTS.md: document alignment rule for => columns when inserting new
language keys
Tests:
- Add ItemsControllerTest covering postSave/bulkupdate tax_names validation
- Reject <, > in tax_names on /items/save and /items/bulkupdate
- Verify unicode and apostrophe-containing tax names are accepted
- Cover CSV import helpers: header generation (basic, multiple locations,
attributes), stock-location/attribute header builders, get_csv_file
parsing (plain, BOM-prefixed, multi-row)
- Validate required-header detection for import templates
- Remove outdated tax name test from SalesControllerTest
- Simplify Database class references in SalesControllerTest
i18n:
- Add tax_name_invalid translation to Items.php for 20+ locales, inserted
alphabetically after tax_category in each file
- Normalize quote style in ar-EG/Items.php to single quotes
- Add ka/Items.php Georgian locale scaffold
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
- Sales::getSearch now enforces reports_sales grant before running
search, returns 403 with lang message when missing
- Rename snake_case helpers/methods to camelCase across
Sales controller, Sale model, and tabular_helper
(get_sale_data_row -> getSaleDataRow, get_payments_summary ->
getPaymentsSummary, sales_headers -> salesHeaders, etc.)
- Config/OSPOS: reset DB data cache before checking app_config
table existence to avoid stale schema cache in tests
- TestDatabaseBootstrapSeeder: expose static reset() so tests can
rebuild schema once per class instead of only via seeder run()
- SalesControllerTest: bootstrap DB once per class, seed once,
refresh app settings each setUp, add tests for search endpoint
authorization (cashier denied, supervisor allowed), move
createTestItem into shared ItemFixtureTrait
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
fix(sales): gate per-record endpoints behind reports_sales grant (REDACTED)
Cashiers holding only the base `sales` grant could reach per-sale endpoints
(getRow, getEdit, postSave, getReceipt, getInvoice, getSendPdf, getSendReceipt)
that require `reports_sales`. getManage() enforced this at the list level, but
individual endpoints did not re-check. Regression tests added.
Auth:
- Introduce `IsLoggedIn` filter to centralize login checks across controllers
- Replace custom `AccessDeniedRedirectException` with built-in `RedirectException`
Employees:
- Add `DISALLOW_PASSWORD_CHANGE` and `DISALLOW_GRANT_CHANGE` env vars to restrict
credential and permission changes in locked-down environments
- Extract `hasGrantsChanged()` to streamline `postSave`
Refactor:
- Rename snake_case variables to camelCase in Sales, Items, and Employees
controllers for PSR-12 compliance
- Use explicit `db_connect()` for transaction clarity in Items controller
Fixes:
- SMTP config entries fall back to defaults via null coalescing
- Migration uses `DROP FOREIGN KEY` instead of `DROP CONSTRAINT`
- Password hash upgrade only sets session on successful `hash_version` update
- Correct lang key for unknown error in Module model
Language:
- Translate `error_grant_change_disallowed` / `error_password_change_disallowed`
across all 44 supported locales with => alignment matching en reference
- Fix "cannot be deleted" messages and misc typos across ~15 language files
Tests:
- Bootstrap seeder only once in ItemsCsvImportTest; close connection after
- Restore `DISALLOW_GRANT_CHANGE` in teardown to prevent side effects
- Use `uniqid()` for test user data to avoid collisions
Signed-off-by: 17935339+objecttothis@users.noreply.github.com
* refactor: standardize function and variable names to camelCase and improve naming consistency across files
* refactor(config): remove spaces around `=` in configuration files for improved consistency and formatting as is required by .env formatting rules.
* refactor(security): extract `.env` key management logic into reusable `writeEnvKey` helper, add throttle key provisioning logic, and streamline encryption key updates
* fix(migration): improve error handling in CI3 to CI4 encryption data migration
- Secure `up` and `convertCI3EncryptedData` methods with detailed exception handling for script execution and data saving.
* fix(migration): ensure empty string is correctly handled in CI3 to CI4 encryption data conversion
* refactor(security): enhance `.env` management with durable writes, better locking, and helper abstraction
- Update `writeEnvKey` to return a success flag and handle file locks robustly.
- Introduce `atomicWriteFile` for atomic writes to prevent partial file updates.
- Add `applyEnvKeyReplacement` to streamline `.env` key insertion and updates.
- Improve throttle key provisioning with validation and runtime persistence safeguards.
* refactor(security): implement dedicated `.env` file locking for robust and cross-platform safe write operations
- Add `lockEnvFile` and `unlockEnvFile` helpers to manage `.env` mutex files.
- Refactor `.env` write logic to use lock helpers, improving reliability and preventing race conditions.
- Enhance `atomicWriteFile` for better handling of file overwrites on Windows and POSIX systems.
* fix(migration): improve encryption error handling during CI3 to CI4 data conversion
- Add conditional checks for `checkEncryption` to prevent failed key persistence.
- Introduce `abortEncryptionConversion` for cleanup on failure.
- Update `writeEnvKey` to handle and return errors gracefully.
* refactor(security): improve `atomicWriteFile` for better file locking and cross-platform durability
- Replace `uniqid` with `bin2hex(random_bytes())` for more secure temp file naming.
- Add explicit file permissions and locking for safe concurrent writes.
- Enhance error handling to ensure atomicity on both Windows and POSIX systems.
* Add env temp files to gitignore so they don't get tracked.
---------
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(auth): hash throttler keys to improve security
- Use MD5 hashing for IP and username-based throttler keys to obfuscate sensitive data while maintaining functionality.
* test(auth): add IPv6 throttling test and hash used throttler keys
- Add a test to ensure throttling works correctly with IPv6 addresses.
- Update throttler keys to use MD5 hashes for IPs and usernames for improved security and consistency.
* fix(auth): handle non-scalar usernames in throttler keys
- Ensure username input is validated as scalar before processing to prevent errors and maintain throttling logic integrity.
* fix(auth): enhance throttler key security with HMAC hashing
- Replace MD5 with HMAC-SHA256 for generating throttler keys.
- Include encryption key from app configuration for added security.
* test(filters): update ThrottleTest to use HMAC-SHA256 for throttler keys
- Replace MD5 with HMAC-SHA256 for generating throttler keys in tests.
- Introduce `check_encryption()` to ensure encryption configuration is available.
* fix(events): validate encryption key on app initialization
- Throw ConfigException if encryption key is missing or invalid during `pre_system` event.
- Remove redundant `check_encryption()` call from Throttle filter and tests.
* fix(events): improve encryption key validation in `pre_system`
- Add `check_encryption()` helper call for additional security verification.
- Update error message to highlight `.env` writability issues if the key is invalid.
* test(filters): handle non-scalar usernames in ThrottleTest
- Update `makeRequest` to validate usernames as scalar and cast them to strings before processing.
- Add a test to ensure array usernames are ignored, and throttling is applied only based on IP.
- Improve status code assertions for throttled requests.
---------
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* Implement atomic updates for gift card and reward point decrements, enhance error handling for insufficient balances, and add regression tests for concurrency safety.
* Add translations for insufficient gift card balance and reward points error messages across all supported languages.
* Reorder `clear_suspended_sale_detail` call to ensure transactional consistency.
* Reorder `clear_all` call to align with success and error handling logic.
* Ensure soft-deleted gift cards are excluded in balance updates.
* Refactor change_quantity logic with atomic upserts, improve error handling for insufficient stock, and update related tests and constants.
* Added check for NEW_ENTRY
* Added unit tests to test changes.
* Fix class name casing in ItemQuantityTest for consistency.
* Fix Bulgarian translations for insufficient balance error messages in Sales module.
* Fix Greek translations for insufficient balance error messages in Sales module.
* Fix Armenian translations for insufficient balance error messages in Sales module.
* Fix Tamil translations for insufficient balance error messages in Sales module.
* Implement race condition testing for database methods with concurrent process support.
* Fix class name casing in ItemTest for consistency.
* Improve concurrent process handling in race condition tests; add readiness and synchronization barriers.
* Improve handling of process I/O streams and timeout management in race condition tests.
* Add test for decrementing gift card value when marked as deleted
* Add `finally` block to ensure proper cleanup in async database race condition tests
* Improve error handling and timeout management in async database race condition tests.
* Refactor test utilities to use shared `EmployeeFixtureTrait` and `ItemFixtureTrait`.
* Track process exit codes explicitly in race condition tests for improved error detection and debugging.
* Improve error handling in `ConcurrentDbRaceTrait` by adding exceptions for `mysqli_poll` and `mysqli_reap_async_query`.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
---------
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(auth): throttle login attempts to prevent brute-force attacks
Add Throttle filter and wire into login route to mitigate
credential-stuffing/brute-force risk (redacted).
- Register App\\Filters\\Throttle in Filters config
- Add \"too_many_attempts\" language string for throttled responses
- Add tests for Throttle filter and Login controller throttling
* Correct bug causing error to not display.
* i18n(login): add too_many_attempts translation for login throttling
Add localized \"too many attempts\" message across all language files
to support login throttling feature. Message informs users to wait
before retrying after exceeding attempt limit.
* fix(auth): rate limit login attempts to prevent brute-force attacks
Add Throttle filter that rate limits login/migrate POST requests,
keyed by IP and submitted username, using CodeIgniter's cache-based
Throttler (redacted).
- Wire filter into login/migrate routes
- Show localized error message when rate limit exceeded
- Broaden writable/cache ignore pattern to cover throttler cache file
* Update app/Language/ar-EG/Login.php
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* style(i18n): use single quotes for translation array keys and values
Convert double-quoted array keys and string values to single quotes
across all Login.php language files for style consistency.
* test(auth): update LoginTest to use throttler service directly
Replace clearThrottleState's Services::resetSingle('throttler') call
with Services::throttler() to reset state via the service instance.
Add usedKeys property to track throttle keys used across test cases.
* test(auth): skip login test assertion when migration required
LoginTest now check migration-required response state before
asserting HTTP 200. Prevent false failures when app force
pending-migration redirect during test run.
* Added missing return statements
* fix(auth): validate gcaptcha before password to prevent bypass
Move gcaptcha check before credential validation so a valid captcha
is required prior to any login attempt. Previously, password auth
ran first, allowing timing-based enumeration without captcha.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* test(auth): add regression tests for gcaptcha validation order in OSPOSRules
Guards fix from 5dea748b0: gcaptcha must be validated before
Employee::login() is attempted to prevent auth bypass.
Change gcaptcha_check visibility to protected to allow testing.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
---------
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
Co-authored-by: Travis Garrison <travis@chiraqbookstore.com>
* feat(plugins): add sale document view hooks and a webhook CSRF exemption
Plugins can already inject buttons into the receipt via
view:sales_receipt_buttons, but the invoice, quote and work order views have
no hook point, so a plugin that delivers sale documents can only reach one of
the four. Add the matching hooks, keeping the same name/data shape:
view:sales_invoice_buttons
view:sales_quote_buttons
view:sales_work_order_buttons
Each passes ['saleId' => $sale_id_num] and sits in the same position as the
existing receipt hook, so one plugin callback can serve all four and branch on
the document type.
Also allow a plugin to expose an inbound provider webhook. A server-to-server
delivery carries no CSRF token and no session, so the global csrf filter now
excludes the pattern plugins/*/webhook. Plugins are responsible for
authenticating such requests themselves (typically by verifying the provider's
signature header against a shared secret) — this is documented alongside the
change.
While editing that line, the except list becomes an array. CodeIgniter matches
every entry as \A<pattern>\z, so the previous 'login|migrate' string produced a
single pattern whose inner alternatives were unanchored: 'login/anything' was
CSRF-exempt. Listing the entries separately anchors each one, which closes that
and keeps the new plugin pattern tight — plugins/whatsapp/webhook is exempt
while plugins/whatsapp/send is not.
* refactor(plugins): drop explanatory comments from csrf filter exceptions
---------
Co-authored-by: Joshua Fernandes <“joshua.1234511@yahoo.in”>
- Create log_plugin_message() to prevent plugin logs from spamming the core logs
- Create ability to log to different logs or a base log if parameter is not specified.
- Update README.md
- Change BasePlugin::log() wrapper function to log to log_plugin_message() and add logTo() to log to a specific plugin log.
- Add plugin logger service to keep the logger loaded.
Signed-off-by: objec <objecttothis@gmail.com>
* Add fallback for allowedHostnames environment variable
- In some cases allowedHostnames is set in env but not loaded at the time of check, yet available in other sources. This adds fallback checks.
- Add UnitTest
Signed-off-by: objec <objecttothis@gmail.com>
* Improve the fallback logic for allowedHostnames environment variable
Signed-off-by: objec <objecttothis@gmail.com>
---------
Signed-off-by: objec <objecttothis@gmail.com>
* Bugfixes to get Migration working on MySQL
Signed-off-by: objec <objecttothis@gmail.com>
* MariaDB compatibility fixes
- Drop foreign key constraints before making charset changes
- Fix dropAllForeignKeyConstraints helper function.
- Added `IF EXISTS` to DROP statements
- Do not try to readd FK constraints for tables which were dropped.
- MariaDB 11.8.x changes the default implicit collation to uca1400 which breaks the IndiaGST migration, et. al. Explicitly declare utf8_general_ci in affected migrations.
Signed-off-by: objec <objecttothis@gmail.com>
* Fix changes which break MySQL migrations
- MySQL does not support IF EXISTS in foreign key constraints. Since the PHP is now handling dropping those constraints, these lines are redundant. Remove them.
Signed-off-by: objec <objecttothis@gmail.com>
* Resolve code review recommendations
- Add try/catch around DB connect statement
- Heed result of execute_script function and throw an exception on failure.
Signed-off-by: objec <objecttothis@gmail.com>
* Refactor out duplicate code
Signed-off-by: objec <objecttothis@gmail.com>
* Initialize array variable causing potential issues
Signed-off-by: objec <objecttothis@gmail.com>
---------
Signed-off-by: objec <objecttothis@gmail.com>
* fix(ci): include hidden files in Docker build context
actions/upload-artifact@v4 excludes hidden files (dotfiles) by default,
causing .htaccess files to be missing from the Docker image. Add
include-hidden-files: true to preserve .htaccess in the build artifact.
* fix(docker): correct permissions and add barcode_type default
- Set proper permissions (750) for writable/logs, writable/uploads,
writable/cache, public/uploads, and public/uploads/item_pics
- Set permissions (640) for writable/uploads/importCustomers.csv
- Add barcode_type default value to prevent 'unknown key' error
during initial migration when database is not yet initialized
---------
Co-authored-by: Ollama <ollama@steganos.dev>
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Allow configuring allowed hostnames via ALLOWED_HOSTNAMES environment
variable as an alternative to app.allowedHostnames in .env file. This
is more convenient for Docker/Compose deployments where environment
variables are set directly in compose files.
The ALLOWED_HOSTNAMES variable takes precedence over app.allowedHostnames
if both are set, allowing deployment-specific overrides.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Ollama <ollama@steganos.dev>
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
- updated some files to match the official CodeIgniter 4 skeleton.
- rebuilt package.json from a clean init and modernized metadata and formatting
- rebuilt composer.json with modernized metadata and formatting
- replaced code of conduct text with markdown
- updated Dockerfile to replace deprecated instruction
- Update README.md to reflect information about routes
- Add registerAllNamespaces() function to correctly load plugin namespaces
- center text in modal title
- Properly decrypt the api key
- Refactor getAllLists to getLists
- Naming simplification of strings when mailchimp_ is redundant or unnecessary
- Do not attempt to decrypt a plaintext api_key pasted into the form
- Register namespaces early on in system init
Signed-off-by: objec <objecttothis@gmail.com>
