v0.7 hardening: API auth, policy enforcement, safe evidence paths, audit integrity#2
v0.7 hardening: API auth, policy enforcement, safe evidence paths, audit integrity#2Protonmatter wants to merge 13 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68d156252d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tool=name, | ||
| action=req.operation, | ||
| target_type="connector-record", | ||
| target_count=req.target_count, |
There was a problem hiding this comment.
connector_query now treats target_count as trusted input from the request body, so callers can bypass max_targets by sending a low target_count while requesting a large result set in params (for example, top=5000). Because PolicyEngine.evaluate only checks target_count, this weakens the new policy enforcement for connector reads and allows oversized queries that should be denied.
Useful? React with 👍 / 👎.
| - name: Docker build | ||
| run: docker build -t mx-cyberops-agent:ci . |
There was a problem hiding this comment.
The workflow now unconditionally runs docker build -t mx-cyberops-agent:ci ., but this repository does not contain a Dockerfile, so the new CI step will fail every run and block merges. This is a hard failure in the default pipeline unless a Dockerfile is added or the step is made conditional/pointed at an existing build file.
Useful? React with 👍 / 👎.
| route = request.url.path | ||
| rate_limiter.check(f"{client}:{route}") |
There was a problem hiding this comment.
The limiter key uses request.url.path, which includes path parameters, so clients can evade limits on /connectors/{name}/query by cycling connector names (each name gets a fresh bucket). This bypasses the newly added abuse control and allows effectively unbounded request volume from one client IP.
Useful? React with 👍 / 👎.
| with self.audit_path.open("r", encoding="utf-8") as f: | ||
| for line in f: | ||
| if line.strip(): | ||
| last_line = line |
There was a problem hiding this comment.
Each audit write calls _last_hash(), which linearly scans the entire JSONL file to find the last record before appending. In a long-running API process this makes logging cost grow with file size and turns total runtime into quadratic behavior, causing avoidable latency spikes as audit volume increases.
Useful? React with 👍 / 👎.
| prev_hash = self._last_hash() | ||
| record = { |
There was a problem hiding this comment.
The hash chain update is a read-then-append sequence (_last_hash() followed by file append) without any file/process lock, so concurrent writers can compute the same prev_hash and append different successor records. In multi-worker deployments this creates forked chains that break the tamper-evident guarantee and make downstream verification ambiguous.
Useful? React with 👍 / 👎.
1 participant
Summary
This PR hardens the MX CyberOps Agent trust boundary and closes several high-risk gaps identified during the deep review.
Primary focus areas:
Changes
API boundary hardening
control_plane/api_security.pyMXCYBER_API_ALLOW_INSECURE=true)Connector policy enforcement
/connectors/{name}/querynow routes throughPolicyEngineEvidence-pack hardening
utils/safe_paths.pyincident_idandrun_idevidence_root0.7.0Audit integrity
audit.v1)prev_hashandevent_hashMicrosoft Graph connector hardening
Policy semantics cleanup
create_ticketnow requires approvaldraft_ticket/draft_jsm_ticketread-only operationsTests
Added regression coverage for:
Safety posture
This PR preserves the repo’s public-safe defensive posture:
Follow-up work
Planned next hardening wave: