lockbitchat
3212138a0d
No protocol or message-protection changes. The version in the application header was a literal and had fallen behind, showing v5.6.0 while running 5.7.1. It now comes from package.json, and a test fails if a hard-coded one reappears or if meta.json, the README badge, the changelog and the docs disagree about the release. Documentation reorganised so that everything technical lives in doc/ with an index, and the root keeps only what belongs there by convention: README, SECURITY, CHANGELOG and LICENSE. - SECURITY.md rewritten. It listed a supported release line three major versions out of date and made claims the software does not make. It now states what is guaranteed, what is not, and how to report a problem. - SECURITY_DISCLAIMER.md and RESPONSIBLE_USE.md merged into doc/USE-POLICY.md, which says what the software cannot protect against rather than listing generic advice. - doc/SECURITY-ARCHITECTURE.md renamed to doc/ARCHITECTURE.md and rewritten around the session lifecycle, what verification gates, and how recovery works. - doc/CRYPTOGRAPHY.md rewritten: key schedule, the Double Ratchet, framing, and memory handling, with values taken from the source rather than restated. - doc/CONFIGURATION.md rewritten with the real file-type policy, ICE and TURN guidance, and the deployment caching rules that matter. - docs/webrtc-config.md moved to doc/CALLS.md and rewritten; the obsolete docs/webrtc-audit.md, a working document full of stale line numbers, removed along with the docs/ directory. - doc/CONTRIBUTING.md records what the recent regressions taught us about writing tests that can actually fail. - doc/README.md added as an index. Internal security review notes are excluded from the repository via .gitignore. Those describe attack paths against specific releases in enough detail to reproduce them, which is useful privately and harmful in public while users are still updating.
90 lines
3.4 KiB
Markdown
90 lines
3.4 KiB
Markdown
# Contributing
|
|
|
|
## Workflow
|
|
|
|
```bash
|
|
npm install
|
|
npm test # 41 suites, plain node:assert, no framework
|
|
npm audit
|
|
npm run build
|
|
```
|
|
|
|
Tests are individual `.mjs` files run in sequence by `npm test`. There is no test
|
|
runner and no mocking library. A new suite is a new file, added to the `test`
|
|
script in `package.json`.
|
|
|
|
## Areas that need extra care
|
|
|
|
Changes touching any of these should come with tests that would fail without the
|
|
change:
|
|
|
|
- Verification: the safety code, the gate on control frames, protocol
|
|
compatibility
|
|
- The Double Ratchet: key derivation, chain advance, skipped-key bounds, the
|
|
order in which state is committed
|
|
- The inbound message path: anything that decides what reaches the interface
|
|
- ICE and TURN behaviour, and the connection recovery cycle
|
|
- File transfer consent and type policy
|
|
- IndexedDB migration
|
|
- Disconnect and resource cleanup
|
|
|
|
## Writing tests that are worth having
|
|
|
|
Two bugs reached production during recent work, and both had the same cause: the
|
|
test built its own input instead of using what the application actually produces.
|
|
|
|
One test generated its own ECDH key pairs with usages the real generator did not
|
|
grant, and passed against a build that could not establish a session at all.
|
|
Another passed a locally generated public key where the application always
|
|
supplies an imported, non-extractable one, and missed a failure that disabled
|
|
forward secrecy for one side of every conversation. Locally generated public keys
|
|
are always extractable in WebCrypto regardless of the flag you pass, so that
|
|
difference is invisible unless you look for it.
|
|
|
|
The lesson is worth stating plainly: use the real factory functions, and where a
|
|
value crosses a boundary in the application, make the test cross the same
|
|
boundary. A test that constructs its inputs verifies the algorithm. Only a test
|
|
that uses the shipped path verifies the code.
|
|
|
|
Before relying on a new test, confirm it fails when the fix is removed. A test
|
|
that cannot fail is worse than no test, because it is read as coverage.
|
|
|
|
## Documentation
|
|
|
|
When behaviour changes, update the documentation in the same commit:
|
|
|
|
| Change | Documents |
|
|
| --- | --- |
|
|
| Anything user-visible | `README.md`, `CHANGELOG.md` |
|
|
| Verification, keys, the ratchet | `doc/CRYPTOGRAPHY.md`, `doc/ARCHITECTURE.md` |
|
|
| Deployment, ICE, file policy | `doc/CONFIGURATION.md` |
|
|
| Calls, codecs, adaptation | `doc/CALLS.md` |
|
|
| Internal interfaces | `doc/API.md` |
|
|
| Anything security relevant | `SECURITY.md` |
|
|
|
|
Values in the documentation (limits, timeouts, algorithm parameters) are taken
|
|
from the source. If you change one in code, change it in the documentation too,
|
|
otherwise the next person will trust the wrong number.
|
|
|
|
## Release notes and security fixes
|
|
|
|
Release notes describe what improved. They do not spell out how a weakness could
|
|
have been exploited, and neither do source comments. Users who have not updated
|
|
are the ones exposed by that detail, and with no server there is no way to update
|
|
everyone at once.
|
|
|
|
Comments explaining why a guard exists are valuable and should stay, because they
|
|
are what stops the guard being removed later. The distinction is between "this
|
|
check exists because completing the handshake does not prove identity" and a
|
|
reproduction recipe.
|
|
|
|
## Pull requests
|
|
|
|
Include:
|
|
|
|
- what the problem is
|
|
- what the change does
|
|
- which tests you ran, and which new ones you added
|
|
- what could regress
|
|
- for user-visible changes, a screenshot or a log
|