Implement the authentication.bin trie mmap format#1061
Conversation
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
jviotti
force-pushed
the
authentication-bin
branch
from
5f59dc4 to
1f123bc
Compare
🤖 Augment PR SummarySummary: This PR implements a memory-mappable trie format for Changes:
Technical Notes: Matching walks path segments and binary-searches each node’s sorted edge list; decisions currently treat the policy set as an allow-list (any governing policy grants public access). 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| throw std::runtime_error("The authentication policy artifact is malformed"); | ||
| } | ||
|
|
||
| this->nodes_ = view->as<AuthenticationNode>(header->nodes_offset); |
There was a problem hiding this comment.
src/authentication/authentication.cc:34 — Since sourcemeta::core::FileView::as<T>() only has assert bounds checks (and requires caller-provided alignment), a corrupted artifact with correct magic/version but bad offsets/counts could cause OOB/UB when mapping nodes_/edges_/strings_ and later in match(). Consider validating node_count > 0, offset+size ranges against view->size(), and alignment for AuthenticationNode (and that edge child/segment ranges are within the declared sections) before storing the section bases.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| auto Authentication::save(std::span<const Authentication::Policy> policies, | ||
| const std::filesystem::path &path) -> void { | ||
| assert(policies.size() <= Authentication::MAXIMUM_POLICIES); |
There was a problem hiding this comment.
src/authentication/authentication_save.cc:49 — Guarding policies.size() <= MAXIMUM_POLICIES with assert means release builds can still hit UB (e.g., 1ULL << index shifting by >= 64) if configuration ever supplies too many policies. Consider enforcing this limit at runtime so production behavior is deterministic and fail-closed.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| stream.write(garbage.data(), garbage.size()); | ||
| stream.close(); | ||
|
|
||
| EXPECT_THROW(const sourcemeta::one::Authentication authentication{path}, |
There was a problem hiding this comment.
test/unit/authentication/authentication_test.cc:43 — The malformed-artifact test only covers “garbage header” (magic/version mismatch); it might be worth adding a case where magic/version are correct but offsets/counts are invalid to ensure the loader rejects structurally-corrupt artifacts without crashing.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 9 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
1 participant
Signed-off-by: Juan Cruz Viotti jv@jviotti.com