Skip to content

build: harden ABI with hidden visibility and PImpl - #14

Merged
dkruempe merged 3 commits into
masterfrom
feature/abi-hardening
Oct 31, 2026
Merged

dkruempe merged 3 commits into
masterfrom
feature/abi-hardening

Conversation

@dkruempe

Copy link
Copy Markdown
Owner

Summary

  • Build the shared and static targets with hidden default symbol visibility (CXX_VISIBILITY_PRESET hidden, VISIBILITY_INLINES_HIDDEN); only LIBKEEPASS_API declarations and the kp_* C ABI are exported.
  • Hide statically linked dependency symbols on ELF with --exclude-libs,ALL.
  • Move KeePass and KdbxFile state into PImpls, keeping Key, KdbxXml and option flags out of the public headers. Copying deep-copies; moving leaves the source default-constructed but usable.
  • Stop exporting the internal detail::FormatCodecRegistry and add copy/move regression tests.
  • Add tools/check_symbols.sh plus a Release CI gate that rejects leaked internal/third-party symbols and verifies the C ABI surface.
  • Update the ABI policy, roadmap, changelog, stale documentation anchors, the feature-request template and the roadmap tracking notes.

Breaking ABI

This is an intentional ABI break for the 1.0 hardening work. KeePass/KdbxFile layout changed and moved-from semantics changed, so 0.x consumers must rebuild against the new library. SOVERSION intentionally stays at 0 per docs/abi-policy.md.

Verification

  • cmake --build build -j → exit 0
  • ctest --test-dir build --output-on-failure → 100% tests passed, 0 failed out of 21
  • tools/check_symbols.sh build → OK (no detail::* besides BinarySource, no pugixml/OpenSSL/argon2/zlib symbols, kp_* present)
  • static build (build-static, LIBKEEPASS_BUILD_SHARED=OFF) → exit 0
  • clang-format --dry-run --Werror on changed C++ files → exit 0
  • git diff --check → exit 0

Follow-ups for 1.0

  • PImpl for the detail::FormatCodec implementations and the cipher.hh algorithm classes.
  • macOS exported-symbols list (hidden preset applies, but statically linked dependency symbols are not filtered from the export trie).

- build the shared and static targets with hidden default symbol
  visibility; only LIBKEEPASS_API declarations and the kp_* C ABI are
  exported, and --exclude-libs,ALL hides statically linked dependency
  symbols on ELF
- move KeePass and KdbxFile state into PImpls; copying deep-copies the
  state and moving leaves the source default-constructed but usable
- stop exporting the internal detail::FormatCodecRegistry and add
  copy/move regression tests for both classes
- add tools/check_symbols.sh and a Release CI gate that rejects leaked
  internal/third-party symbols and checks the C ABI surface
- update the ABI policy, roadmap, changelog and stale documentation
  anchors; fix the feature-request template and roadmap tracking notes

This is an intentional ABI break for the 1.0 hardening work: 0.x
consumers must rebuild against the new library.
@codecov-commenter

codecov-commenter commented Oct 31, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 88.46154% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/kdbx_import.cc 74.19% 8 Missing ⚠️
src/keepass.cc 85.18% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

The moved-from objects are intentionally reused by the new tests, and the
move operations allocate a fresh default implementation so they cannot be
noexcept. Annotate both with NOLINTNEXTLINE and the precise check names
instead of changing the contract.
Reusing a pooled group whose UUID already appeared in the parsed tree made
a nested group alias an ancestor, turning the object graph cyclic and
recursing without bound in KdbxXml::WriteGroup. fuzz_xml found this as a
stack overflow after parsing succeeded.

Track the UUIDs seen while parsing the tree and reject duplicates with
FormatError. Placeholders created from Meta (RecycleBinUUID,
EntryTemplatesGroup) are still reused once. Add a regression test.
@dkruempe
dkruempe merged commit 641b393 into master Oct 31, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants