mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 04:32:32 +00:00
fix(rules): close the rule-ID namespace gaps around the shared global keyspace
Custom rules are tenant-partitioned and functionally collision-free, but three
surfaces still keyed on the bare non-unique ruleId string:
- RuleStats global aggregate (global_{ruleId}) summed same-ID custom rules of
unrelated tenants (title/severity last-writer-wins). The global row is now
catalog-only: analyze gates on IsBuiltIn/IsCommunity, gather gates on the
reserved-namespace pattern (agent events carry no flag; custom rules can
never occupy the reserved pattern).
- The 'never ship a built-in in the CUSTOM namespace' commitment from
RuleIdPolicy was documented but unenforced: combine.js now fails on
duplicate or non-reserved built-in IDs, BuiltInRuleCatalogPolicyTests pins
the same invariants on the embedded catalog, and the GitHub reseed rejects
non-reserved IDs before writing to the global partition.
- DeleteRuleStatesForRuleIdAcrossTenantsAsync (cross-partition delete by bare
RowKey) now refuses IDs outside the reserved namespace with failed=-1 so
callers abort the follow-up global catalog delete.
Verified: prod global partitions (45 analyze + 11 gather) are fully inside the
reserved namespace, so the guards touch no existing data. New OKF doc
docs/rules/rule-id-namespace.md captures the identity/collision model.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -37,6 +37,7 @@ https://docs.autopilotmonitor.com (separate repository).
|
||||
* [Gather Rule Phase Scoping, One-Shot Triggers & Emit-on-Change](rules/gather-rule-phase-scoping.md) - Restricting gather rules to enrollment phases (activePhases / activeFromPhase sticky latch), one-shot collection at a phase's start/end (phase_change / phase_exit), and emitMode "on_change" result-dedup.
|
||||
* [Gather Rule Guardrails](rules/gather-rule-guardrails.md) - Why the allowlists in rules/guardrails.json are enforced on the agent and nowhere else, which hard blocks survive unrestricted mode, and the rule that every collector must call a guard and emit security_warning on refusal.
|
||||
* [Rule Authoring Surface](rules/rule-authoring-surface.md) - How customer AIs author rules against the real contract: the baked MCP resources (guide/schemas/guardrails via combine.js), the MCP-local validate_rule pre-flight, and the side-effect-free POST rules/analyze/dryrun whose trace twin shares the production evaluators (parity + side-effect-freedom pinned by tests).
|
||||
* [Rule ID Namespace & Collision Guards](rules/rule-id-namespace.md) - Why rule identity is the human-readable ruleId (no GUIDs) with tenant isolation via Table Storage partitioning, the reserved built-in namespace (RuleIdPolicy), which collisions are possible (custom-vs-custom across tenants: allowed and harmless), and the guards keeping the global RuleStats aggregate catalog-only and the global partition inside the reserved namespace (combine.js, reseed, cross-tenant RuleState GC).
|
||||
* [Analyze Rule Evaluation Triggers](rules/analyze-rule-triggers.md) - Opt-in `evaluateOn` interim triggers (`whiteglove_sealed`, `on_event:<type>`) so rules can fire before a session is terminal, with RuleResults update-semantics (interim → finalize/resolve, NotifiedAt notification dedupe) replacing the permanent dedupe freeze, KO/stat suppression on interim runs, the ingest-side cached trigger registry, and the authoring lints that keep interim rules monotonic.
|
||||
|
||||
# Backend
|
||||
|
||||
@@ -2,6 +2,8 @@
|
||||
|
||||
## 2026-08-21
|
||||
|
||||
* **New**: `rules/rule-id-namespace.md` — rule identity model: human-readable `ruleId` strings (no GUIDs), tenant isolation via Table Storage partitioning (`global` = built-in/community, tenantId = custom), the reserved built-in namespace from `RuleIdPolicy`, and the four collision guards: catalog-only global `RuleStats` aggregate (same-ID custom rules of unrelated tenants no longer sum into one `global_{ruleId}` row), combine.js duplicate/namespace fail, GitHub-reseed rejection of non-reserved IDs into the global partition, and the namespace gate on the cross-tenant RuleState GC.
|
||||
|
||||
* **New**: `backend/app-install-duration-attempt.md` — attempt-scoped app install durations: `DurationSeconds` = status-defining (last) attempt anchored on the new max-folded `LastAttemptStartedAt` (StartedAt stays the earliest-wins window/version-ordering key; legacy rows without the anchor keep the span fallback); `InstallPassCount` with replay-guarded additive fold; weaker-terminal guard (completed-with-Skipped never overrides Installed/Error, in-batch and cross-batch); attempt-scoped `DownloadDurationSeconds` pairing; the version-regression radar's both-sides cutover gate; time attribution counts the union of per-pass active segments (occupancy merges segments, hulls stay for display/what-if); agent payload timing re-arms both endpoints on terminal→active. No backfill — pre-cutover rows keep span semantics.
|
||||
|
||||
* **Update**: `agent/decision-engine.md` — activity-based RealmJoin timeout extension (report 55e6afd61c9d: a large RJ package catalog outlived the 60-min window mid-install — the timer arms at RJ-agent-MSI detection, so the first deployment had effectively ~44 min). When the `RealmJoinTimeout` deadline fires with `LastDeploymentPhase` 100/101 and deployment activity (phase change or package observation, persisted in `RealmJoinFacts.LastActivityUtc`; the detection-time phase capture deliberately excluded) inside the last 60 min, the deadline re-arms to `lastActivity + 60 min` (Info event `realmjoin_timeout_extended`), clamped at detection + 4 h. `realmjoin_timeout` now carries `reason` = `hard_timeout` / `inactivity` / `absolute_cap`; a fire of a superseded deadline incarnation dead-ends. Idle detection and phase-200/210 sessions keep the original 60-min behavior.
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
---
|
||||
type: concept
|
||||
title: Rule ID Namespace & Collision Guards
|
||||
description: How gather/analyze rule identity is scoped (tenant partition vs global partition), why there are no GUIDs, which collisions are possible, and the guards that keep the reserved built-in namespace and the global RuleStats aggregate clean.
|
||||
resource: backend, rules
|
||||
tags: [rules, ruleid, namespace, collision, rulestats, tenant-isolation]
|
||||
timestamp: 2026-08-21
|
||||
---
|
||||
|
||||
# Schema
|
||||
|
||||
Rule identity is the human-readable `ruleId` string — there are no GUIDs, deliberately: the ID appears in logs, mails, notifications, RuleResults, the agent config payload, and docs, and tenant isolation is provided by Table Storage partitioning instead.
|
||||
|
||||
| Store | PartitionKey | RowKey | Content |
|
||||
|---|---|---|---|
|
||||
| `GatherRules` / `AnalyzeRules` | `"global"` | ruleId | Built-in + community rules (shared, all tenants) |
|
||||
| `GatherRules` / `AnalyzeRules` | tenantId | ruleId | Custom rules (visible only to that tenant) |
|
||||
| `RuleStates` | tenantId | ruleId | Per-tenant enable/notify overrides — built-in/community rules ONLY (custom rules fold state into their own row) |
|
||||
| `RuleStats` | date | `{tenantId}_{ruleId}` and `global_{ruleId}` | Daily fire/eval telemetry; the global row is catalog-only (see guards) |
|
||||
|
||||
Namespace split (`RuleIdPolicy.cs`, shared model):
|
||||
|
||||
- **Reserved built-in namespace**: `(ANALYZE|GATHER)-<CATEGORY>-<NUMBER>` with `CATEGORY != CUSTOM`, case-insensitive, including currently unused numbers (gaps = retired rules that may return).
|
||||
- **Tenant namespace**: everything else — `ANALYZE-CUSTOM-NNN` (portal suggestion), `-CUSTOM` suffixes, org prefixes (`CONTOSO-WIFI-001`), freeform.
|
||||
|
||||
# Collision model
|
||||
|
||||
- **Custom vs built-in**: impossible via API — `CreateRuleAsync`/`UpdateRuleAsync` reject reserved IDs (409) and check existence against `global` ∪ own tenant partition. Legacy debris is neutralized at merge time: global wins, tenant copy dropped with a warning (pinned by `RuleMergeCollisionGuardTests`, motivated by the 5ca2b350 duplicate-key incident).
|
||||
- **Custom vs custom across tenants**: allowed and harmless — separate partitions, separate evaluation, separate RuleResults/tenant stats. Two tenants both using `ANALYZE-CUSTOM-101` never interact.
|
||||
- **JSON copy between tenants** (export → paste into create editor): goes through the same create path, so an ID already taken in the receiving tenant surfaces as 409 — the user renames. There is no bulk import endpoint; note the export→paste round-trip is lossy (gather `parameters`, analyze `evaluateOn` are form-derived and silently dropped by the create editor).
|
||||
|
||||
# Guards (all enforced, 2026-08-21)
|
||||
|
||||
1. **Global RuleStats aggregate is catalog-only.** The `global_{ruleId}` row would otherwise sum same-ID custom rules of unrelated tenants (title/severity last-writer-wins). Analyze gates on `rule.IsBuiltIn || rule.IsCommunity` (`AnalyzeOnEnrollmentEndHandler`), gather gates on `RuleIdPolicy.IsReservedBuiltInId` (`EventIngestProcessor.RuleStats`) because agent events carry no flag — valid because custom rules can never occupy the reserved pattern.
|
||||
2. **Catalog build fails on namespace violations.** `rules/scripts/combine.js` rejects duplicate IDs (case-insensitive) and any gather/analyze built-in outside the reserved pattern; `BuiltInRuleCatalogPolicyTests` pins the same invariants on the embedded resource actually deployed.
|
||||
3. **GitHub reseed rejects non-reserved IDs** before writing to the `global` partition (`ReseedFromGitHubFunction`) — a seeded `*-CUSTOM-*` rule would silently shadow every tenant's same-ID custom rule at merge time.
|
||||
4. **Cross-tenant RuleState GC is namespace-gated.** `DeleteRuleStatesForRuleIdAcrossTenantsAsync` deletes by bare RowKey across all partitions; it now refuses non-reserved IDs with `failed=-1` (callers then skip the global catalog delete, same contract as enumeration failure).
|
||||
|
||||
Invariant chain: schema pattern + combine.js + reseed guard ⇒ the `global` partition only ever contains reserved-pattern IDs ⇒ the pattern is a safe built-in/custom discriminator everywhere no `IsBuiltIn` flag is available.
|
||||
|
||||
# Citations
|
||||
|
||||
- `src/Shared/AutopilotMonitor.Shared/Models/Rules/RuleIdPolicy.cs` — reserved pattern + rationale
|
||||
- `src/Backend/AutopilotMonitor.Functions/Services/AnalyzeRuleService.cs`, `GatherRuleService.cs` — create/update guards, merge collision guard
|
||||
- `src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Rules.cs` — key schemes, cross-tenant GC guard
|
||||
- `src/Backend/AutopilotMonitor.Functions.Tests/BuiltInRuleCatalogPolicyTests.cs`, `RuleIdReservationTests.cs`, `RuleMergeCollisionGuardTests.cs`
|
||||
- `rules/scripts/combine.js` — build-time namespace/duplicate guard
|
||||
@@ -13,18 +13,26 @@ const rulesRoot = path.resolve(__dirname, '..');
|
||||
|
||||
// ── Rule combination ────────────────────────────────────────────────────────
|
||||
|
||||
// Shipped gather/analyze rules must stay inside the reserved built-in namespace:
|
||||
// (ANALYZE|GATHER)-<CATEGORY>-<NNN> with CATEGORY !== CUSTOM. The CUSTOM category is
|
||||
// the sanctioned tenant namespace (RuleIdPolicy.cs) — a built-in shipped there would
|
||||
// silently shadow every tenant's same-ID custom rule at merge time (global wins).
|
||||
const RESERVED_BUILTIN_ID = /^(ANALYZE|GATHER)-(?!CUSTOM-)[A-Z]+-\d{3}$/;
|
||||
|
||||
const configs = [
|
||||
{
|
||||
dir: path.join(rulesRoot, 'gather'),
|
||||
output: path.join(rulesRoot, 'dist', 'gather-rules.json'),
|
||||
schema: '../schema/gather-rule.schema.json',
|
||||
idField: 'ruleId'
|
||||
idField: 'ruleId',
|
||||
reservedNamespace: RESERVED_BUILTIN_ID
|
||||
},
|
||||
{
|
||||
dir: path.join(rulesRoot, 'analyze'),
|
||||
output: path.join(rulesRoot, 'dist', 'analyze-rules.json'),
|
||||
schema: '../schema/analyze-rule.schema.json',
|
||||
idField: 'ruleId'
|
||||
idField: 'ruleId',
|
||||
reservedNamespace: RESERVED_BUILTIN_ID
|
||||
},
|
||||
{
|
||||
dir: path.join(rulesRoot, 'ime-log-patterns'),
|
||||
@@ -37,12 +45,27 @@ const configs = [
|
||||
for (const config of configs) {
|
||||
const files = fs.readdirSync(config.dir).filter(f => f.endsWith('.json')).sort();
|
||||
const rules = [];
|
||||
const seenIds = new Map(); // lowercased id -> filename (case variants collide for humans and RuleIdPolicy)
|
||||
|
||||
for (const file of files) {
|
||||
const content = fs.readFileSync(path.join(config.dir, file), 'utf8');
|
||||
const rule = JSON.parse(content);
|
||||
// Remove $schema from individual entries (it's on the wrapper)
|
||||
delete rule['$schema'];
|
||||
|
||||
const id = rule[config.idField] || '';
|
||||
const idKey = id.toLowerCase();
|
||||
if (seenIds.has(idKey)) {
|
||||
console.error(`ERROR: duplicate ${config.idField} "${id}" in ${file} (already defined in ${seenIds.get(idKey)})`);
|
||||
process.exit(1);
|
||||
}
|
||||
seenIds.set(idKey, file);
|
||||
|
||||
if (config.reservedNamespace && !config.reservedNamespace.test(id)) {
|
||||
console.error(`ERROR: ${file}: ${config.idField} "${id}" is outside the reserved built-in namespace ${config.reservedNamespace} — built-ins must never use the tenant CUSTOM namespace`);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
rules.push(rule);
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
using AutopilotMonitor.Functions.Services;
|
||||
using AutopilotMonitor.Shared.Models;
|
||||
|
||||
namespace AutopilotMonitor.Functions.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Pins the shipped rule catalogs to the reserved built-in ID namespace from
|
||||
/// RuleIdPolicy: every built-in occupies (ANALYZE|GATHER)-<CATEGORY>-<NUMBER> with
|
||||
/// CATEGORY != CUSTOM, and IDs are unique (case-insensitively — case variants are
|
||||
/// indistinguishable to humans and to the policy regex). This is the backend-side
|
||||
/// enforcement of the "platform never ships a built-in in the CUSTOM namespace"
|
||||
/// commitment documented in RuleIdPolicy — without it, a shipped CUSTOM-ID rule
|
||||
/// would silently shadow every tenant's same-ID custom rule at merge time
|
||||
/// (global wins, tenant copy dropped). combine.js mirrors these checks at
|
||||
/// catalog-build time; this test guards the embedded resource actually deployed.
|
||||
/// </summary>
|
||||
public class BuiltInRuleCatalogPolicyTests
|
||||
{
|
||||
[Fact]
|
||||
public void BuiltIn_analyze_ids_are_reserved_namespace_and_unique()
|
||||
{
|
||||
var ids = BuiltInAnalyzeRules.GetAll().Select(r => r.RuleId).ToList();
|
||||
|
||||
Assert.NotEmpty(ids);
|
||||
foreach (var id in ids)
|
||||
{
|
||||
Assert.True(RuleIdPolicy.IsReservedBuiltInId(id),
|
||||
$"Built-in analyze rule '{id}' lies outside the reserved built-in namespace — " +
|
||||
"it would collide with the tenant custom namespace.");
|
||||
}
|
||||
|
||||
var duplicates = ids.GroupBy(id => id, StringComparer.OrdinalIgnoreCase)
|
||||
.Where(g => g.Count() > 1).Select(g => g.Key).ToList();
|
||||
Assert.True(duplicates.Count == 0,
|
||||
$"Duplicate built-in analyze rule IDs: {string.Join(", ", duplicates)}");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuiltIn_gather_ids_are_reserved_namespace_and_unique()
|
||||
{
|
||||
var ids = BuiltInGatherRules.GetAll().Select(r => r.RuleId).ToList();
|
||||
|
||||
Assert.NotEmpty(ids);
|
||||
foreach (var id in ids)
|
||||
{
|
||||
Assert.True(RuleIdPolicy.IsReservedBuiltInId(id),
|
||||
$"Built-in gather rule '{id}' lies outside the reserved built-in namespace — " +
|
||||
"it would collide with the tenant custom namespace.");
|
||||
}
|
||||
|
||||
var duplicates = ids.GroupBy(id => id, StringComparer.OrdinalIgnoreCase)
|
||||
.Where(g => g.Count() > 1).Select(g => g.Key).ToList();
|
||||
Assert.True(duplicates.Count == 0,
|
||||
$"Duplicate built-in gather rule IDs: {string.Join(", ", duplicates)}");
|
||||
}
|
||||
}
|
||||
@@ -137,6 +137,18 @@ namespace AutopilotMonitor.Functions.Functions.Admin
|
||||
|
||||
private async Task<(int deleted, int written, int orphanStatesGcd, int sunsetSkipped)> ReseedGatherAsync(List<AutopilotMonitor.Shared.Models.GatherRule> rules)
|
||||
{
|
||||
// Global-partition rules must stay inside the reserved built-in namespace — an ID
|
||||
// outside it (e.g. GATHER-CUSTOM-*) would silently shadow every tenant's same-ID
|
||||
// custom rule at merge time (global wins). Reject instead of import.
|
||||
var rejectedGather = rules.Where(r => !AutopilotMonitor.Shared.Models.RuleIdPolicy.IsReservedBuiltInId(r.RuleId)).ToList();
|
||||
if (rejectedGather.Count > 0)
|
||||
{
|
||||
_logger.LogError(
|
||||
"GitHub reseed gather: rejected {Count} rule(s) outside the reserved built-in namespace: {Ids}",
|
||||
rejectedGather.Count, string.Join(", ", rejectedGather.Select(r => r.RuleId)));
|
||||
rules = rules.Except(rejectedGather).ToList();
|
||||
}
|
||||
|
||||
var existing = await _ruleRepo.GetGatherRulesAsync("global");
|
||||
var newCatalogIds = rules.Select(r => r.RuleId).ToHashSet(System.StringComparer.Ordinal);
|
||||
// A rule the deployed binary already ships IDENTICALLY is embedded-provenance; one whose
|
||||
@@ -199,6 +211,18 @@ namespace AutopilotMonitor.Functions.Functions.Admin
|
||||
|
||||
private async Task<(int deleted, int written, int orphanStatesGcd, int sunsetSkipped)> ReseedAnalyzeAsync(List<AutopilotMonitor.Shared.Models.AnalyzeRule> rules)
|
||||
{
|
||||
// Global-partition rules must stay inside the reserved built-in namespace — an ID
|
||||
// outside it (e.g. ANALYZE-CUSTOM-*) would silently shadow every tenant's same-ID
|
||||
// custom rule at merge time (global wins). Reject instead of import.
|
||||
var rejectedAnalyze = rules.Where(r => !AutopilotMonitor.Shared.Models.RuleIdPolicy.IsReservedBuiltInId(r.RuleId)).ToList();
|
||||
if (rejectedAnalyze.Count > 0)
|
||||
{
|
||||
_logger.LogError(
|
||||
"GitHub reseed analyze: rejected {Count} rule(s) outside the reserved built-in namespace: {Ids}",
|
||||
rejectedAnalyze.Count, string.Join(", ", rejectedAnalyze.Select(r => r.RuleId)));
|
||||
rules = rules.Except(rejectedAnalyze).ToList();
|
||||
}
|
||||
|
||||
var existing = await _ruleRepo.GetAnalyzeRulesAsync("global");
|
||||
var newCatalogIds = rules.Select(r => r.RuleId).ToHashSet(System.StringComparer.Ordinal);
|
||||
// A rule the deployed binary already ships IDENTICALLY is embedded-provenance; one whose
|
||||
|
||||
+10
-4
@@ -289,10 +289,16 @@ namespace AutopilotMonitor.Functions.Services.Analyze
|
||||
rule.Title, rule.Category, rule.Severity,
|
||||
fired, confidence).ConfigureAwait(false);
|
||||
|
||||
await _metricsRepo.IncrementRuleStatAsync(
|
||||
today, "global", rule.RuleId, "analyze",
|
||||
rule.Title, rule.Category, rule.Severity,
|
||||
fired, confidence).ConfigureAwait(false);
|
||||
// Global aggregate row is catalog-only: custom-rule IDs are tenant-chosen
|
||||
// and not unique across tenants, so a shared "global_{ruleId}" row would
|
||||
// sum unrelated tenants' counters (title/severity last-writer-wins).
|
||||
if (rule.IsBuiltIn || rule.IsCommunity)
|
||||
{
|
||||
await _metricsRepo.IncrementRuleStatAsync(
|
||||
today, "global", rule.RuleId, "analyze",
|
||||
rule.Title, rule.Category, rule.Severity,
|
||||
fired, confidence).ConfigureAwait(false);
|
||||
}
|
||||
}
|
||||
}
|
||||
catch (Exception ex)
|
||||
|
||||
@@ -36,10 +36,18 @@ namespace AutopilotMonitor.Functions.Services
|
||||
ruleTitle, "", "",
|
||||
fired: true, confidenceScore: null);
|
||||
|
||||
await _metricsRepo.IncrementRuleStatAsync(
|
||||
today, "global", ruleId, "gather",
|
||||
ruleTitle, "", "",
|
||||
fired: true, confidenceScore: null);
|
||||
// Global aggregate row is catalog-only. The agent event carries no
|
||||
// IsBuiltIn flag, but custom rules can never occupy the reserved
|
||||
// built-in namespace (create/update guard), so the ID pattern is the
|
||||
// discriminator. Custom IDs are tenant-chosen and not unique across
|
||||
// tenants — a shared "global_{ruleId}" row would mix their counters.
|
||||
if (RuleIdPolicy.IsReservedBuiltInId(ruleId))
|
||||
{
|
||||
await _metricsRepo.IncrementRuleStatAsync(
|
||||
today, "global", ruleId, "gather",
|
||||
ruleTitle, "", "",
|
||||
fired: true, confidenceScore: null);
|
||||
}
|
||||
}
|
||||
}
|
||||
catch (Exception ex)
|
||||
|
||||
@@ -442,6 +442,20 @@ namespace AutopilotMonitor.Functions.Services
|
||||
public async Task<(int deleted, int failed)> DeleteRuleStatesForRuleIdAcrossTenantsAsync(string ruleId)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(ruleId)) return (0, 0);
|
||||
|
||||
// This is a cross-partition delete keyed on a bare RowKey string, so only
|
||||
// catalog (built-in/community) IDs may pass: custom-rule IDs are tenant-chosen
|
||||
// and not unique across tenants, and a call with one would delete state rows
|
||||
// of unrelated tenants. failed=-1 reuses the enumeration-failure contract —
|
||||
// the caller then refuses the follow-up global catalog delete.
|
||||
if (!RuleIdPolicy.IsReservedBuiltInId(ruleId))
|
||||
{
|
||||
_logger.LogError(
|
||||
"Refusing cross-tenant RuleState GC for '{RuleId}': ID is outside the reserved built-in namespace",
|
||||
ruleId);
|
||||
return (0, -1);
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
var tableClient = _tableServiceClient.GetTableClient(Constants.TableNames.RuleStates);
|
||||
|
||||
Reference in New Issue
Block a user