google / google/capsem

Profile mutation routes race each other and write policy files non-atomically

Open
#202 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
72
Forks
13
Avg merge
1d 2h
Merged PRs (30d)
5

Description

## Problems (all in the profile mutation path of `capsem-service`)

1. **No serialization across mutation routes.** `handle_enforcement_rule_upsert/delete`, `handle_detection_rule_upsert/delete` and `handle_profile_plugin_update` each do load profile → persist → refresh caches → materialize active profiles → `ReloadConfig` broadcast with no lock. Two concurrent edits can lose one update in the profile files (read-modify-write) and interleave `vm/active_profile.toml` writes, so a VM can acknowledge a reload of a file that already contains a later edit while the earlier route is still in flight.
2. **`materialize_active_profile` uses `std::fs::write`** while `capsem-process` may be parsing the file for `ReloadConfig`. `capsem_foundation::unix::fs::atomic_write_private` exists and should be used; `persist_asset_reconcile_state` hand-rolls the same tmp+rename.
3. **Profile source files are written non-atomically in core.** `Profile::upsert_profile_rule`, `delete_profile_rule`, `set_plugin_config` and `Profile::save` (`crates/capsem-core/src/net/policy_config/profile_contract.rs`) use `std::fs::write` on `enforcement.toml` / `profile.toml`. A crash between truncate and write leaves an empty or partial security policy.
4. **Per-domain reload routes ignore their profile.** `handle_enforcement_reload` / `handle_detection_reload` validate the path profile id and then call `handle_reload_config` for every profile.
5. **Partial push failures are reported as a plain 500** ("failed to reload config in some instances") after the profile file was already updated and other VMs applied it; the client cannot tell what state it is in.

## Reproducer for (1)

Test written against `940f8c17e` (service tests, `tests/profile_rule_push.rs`): fake process that delays its `Pong` 300 ms and reads `active_profile.toml` just before acking; upsert rule A, 100 ms later upsert rule B concurrently. Expected: the content seen at A's ack contains A and not B. Actual: B is already present (second edit overtook the first). Available on request; not committed.

## Expected

- One `ServiceState` mutation lock held across the whole route and the reload route.
- Active profile and profile source files written through the shared atomic helper; interrupted-write test proves the previous complete file survives.
- Reload routes scoped to their profile.
- A typed error body naming VMs that did not apply and stating the profile was saved.

Not on the container/benchmark path; tracked here instead of the networking sprint.

Contributor guide

Open the contributing guide

Research direction

Start with the profile mutation routes named in the issue and crates/capsem-core/src/net/policy_config/profile_contract.rs, then inspect capsem_foundation::unix::fs::atomic_write_private and the service test scenario in tests/profile_rule_push.rs. Done means mutation routes are serialized, active and source profile files use atomic writes, reloads target the requested profile, and partial pushes return a typed error identifying affected VMs.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend, security
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.