Enforce capability and resource authorization #86
No reviewers
Labels
No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
nsaspy/starintel-server!86
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "agent/issue-28-authorization-policy"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Runtime policy
HTTP and embedded boundaries
501stubs after policy allow; ordinary principals are rejected before backend dispatchRabbit provenance
Mandatory acceptance tests
Remaining merge gates
PR remains draft until every gate passes.
Fixes #28
CI review: not ready to merge
I reran the failed workflow jobs on the current head (
dfa121e). Both the unit job (Test system) and service-backed job (Test with services) fail again after their Nix/container/RabbitMQ/CouchDB setup succeeds, so this is a reproducible branch defect rather than stale CI or service startup.Blocking defect:
make-capabilityis not visible to the test packageThe new helpers in both:
t/authorization-policy-test.lispt/authorization-quota-test.lispcall
make-capabilitywithout a package qualifier.capabilityis defined withdefstructinsource/authorization/policy.lisp, so SBCL creates the constructorstar.authorization::make-capability. However,source/authorization/package.lispexports the structure and accessors but does not exportmake-capability.The test package only
:usesstar.authorization;:useimports external symbols only. Consequently, the unqualified test call resolves asstarintel-gserver-tests::make-capability, which has no function definition. The first test using either helper will therefore fail with an undefined-function error. Both CI jobs execute this test system, which explains the shared failure path.Required fix
Choose one intentional constructor API and update every caller consistently:
#:make-capabilityfromstar.authorization; ormake-authorization-capability, then update the tests/callers; orstar.authorization::make-capabilityin white-box tests only.Option 2 is preferable for a stable public API. Option 1 is acceptable if the generated constructor is intentionally public.
After fixing it, run both exact CI entry points:
Do not skip or remove the new tests to obtain green CI.
Secondary review issue: load-order-dependent HTTP security behavior
source/frontends/http-status-message.lispintroduces anotherstatus-msgimplementation that includes atracefield and omitscorrelation_id, while the existing HTTP boundary tests require the opposite. A later ASDF-loaded file currently redefines the function again, so behavior depends on serial load order.Consolidate
status-msginto one canonical implementation that:correlation_id;The PR also contains other silent function redefinitions in the HTTP integration path. Those should be converted to explicit wrappers/methods or changes to the canonical definitions instead of relying on whichever file loads last.
Merge verdict
Not ready. Keep this draft unmerged until both workflow jobs are green and the duplicate security-sensitive definitions are resolved.