TenantAtlas/docs/security/REDACTION_AUDIT_REPORT.md

Redaction / Masking / Sanitizing — Codebase Audit Report

Auditor: Security + Data-Integrity Codebase Auditor
Date: 2026-03-06
Scope: Entire TenantAtlas repo (excluding vendor/, node_modules/, compiled views)
Severity Classification: SEV-1 = data loss / secret exposure in production path; SEV-2 = data quality degradation; SEV-3 = cosmetic / defense-in-depth

---

Executive Summary

The Bug

PolicySnapshotRedactor — the central class that masks secrets before persisting PolicyVersion snapshots to the database — uses substring-matching regex patterns (/password/i, /secret/i, /token/i, /certificate/i) against JSON keys. This causes false-positive redaction of Intune configuration keys such as:

Key	Actual Meaning	Wrongly Redacted?
passwordMinimumLength	Config: minimum password length (integer)	YES — matches /password/i
passwordRequired	Config: boolean toggle	YES — matches /password/i
deviceCompliancePasswordRequired	Config: boolean toggle	YES — matches /password/i
localAdminPasswordRotationEnabled	Config: boolean toggle	YES — matches /password/i
passwordExpirationDays	Config: integer	YES — matches /password/i
passwordMinimumCharacterSetCount	Config: integer	YES — matches /password/i
passwordBlockSimple	Config: boolean	YES — matches /password/i
passwordPreviousPasswordBlockCount	Config: integer	YES — matches /password/i
certificateValidityPeriodScale	Config: enum	YES — matches /certificate/i
certificateRenewalThresholdPercentage	Config: integer	YES — matches /certificate/i
tokenType	Config: enum	YES — matches /token/i

Impact

1. SEV-1: Persistent data loss — Redaction happens before storing PolicyVersion.snapshot (JSONB). The original values are irrecoverably destroyed. This breaks:

   • Drift detection (diffs show [REDACTED] vs [REDACTED] — always "no change")
   • Baseline compare (config values masked, comparison meaningless)
   • Restore flows (restored policies could lose critical settings)
   • Evidence packs / review pack exports

2. SEV-2: False drift suppression — Two snapshots with different passwordMinimumLength values (e.g., 8 vs 12) will both store [REDACTED], producing identical hashes → drift goes undetected.

3. Real secrets remain correctly redacted — The existing patterns DO catch actual secrets (password, wifi_password, clientSecret, etc.) — the problem is OVER-matching config keys that contain the word "password".

Affected Layers

• 3 distinct redaction/sanitization systems with the same bug pattern
• 2 persistent storage paths (PolicyVersion, AuditLog)
• 1 non-persistent rendering path (Verification reports)
• ~8 additional downstream consumers of already-redacted data

---

Findings — Complete Redaction Map

Finding F-1: PolicySnapshotRedactor (CRITICAL — SEV-1)

Attribute	Value
File	app/Services/Intune/PolicySnapshotRedactor.php
Lines	14-23 (pattern definitions), 64-70 (isSensitiveKey), 72-93 (redactValue)
Layer	Storage (persistent JSONB)
What's redacted	Any key whose name substring-matches 8 regex patterns
Entry points	PolicyCaptureOrchestrator::capture() (L148-150), VersionService::captureVersion() (L47-49)
Persisted	YES — redacted data stored as PolicyVersion.snapshot, .assignments, .scope_tags
Reversible	NO — original values lost permanently

Rules (exact patterns):

private const array SENSITIVE_KEY_PATTERNS = [
    '/password/i',      // ← FALSE POSITIVES: passwordMinimumLength, passwordRequired, etc.
    '/secret/i',        // ← FALSE POSITIVES: possible but less common in Intune
    '/token/i',         // ← FALSE POSITIVES: tokenType, tokenAccountType, etc.
    '/client[_-]?secret/i',  // ← OK (specific enough)
    '/private[_-]?key/i',    // ← OK (specific enough)
    '/shared[_-]?secret/i',  // ← OK (specific enough)
    '/preshared/i',           // ← OK (specific enough)
    '/certificate/i',         // ← FALSE POSITIVES: certificateValidityPeriodScale, etc.
];

False positive examples:

• passwordMinimumLength: 12 → becomes passwordMinimumLength: "[REDACTED]"
• passwordRequired: true → becomes passwordRequired: "[REDACTED]"
• certificateValidityPeriodScale: "years" → becomes certificateValidityPeriodScale: "[REDACTED]"
• tokenType: "user" → becomes tokenType: "[REDACTED]"

Risk Score: 🔴 CRITICAL — Permanent data loss in production path. Destroys drift detection, baseline compare, and restore fidelity.

Double-redaction issue: VersionService::captureVersion() (L47-49) applies the redactor, AND PolicyCaptureOrchestrator::capture() (L148-150) also applies it before passing to captureVersion(). When called through the orchestrator, data is redacted twice (harmless but indicates confusion about ownership).

---

Finding F-2: AuditContextSanitizer (HIGH — SEV-2)

Attribute	Value
File	app/Support/Audit/AuditContextSanitizer.php
Lines	36-44 (shouldRedactKey)
Layer	Audit logging (persistent)
What's redacted	Keys containing substrings: token, secret, password, authorization, private_key, client_secret
Entry points	WorkspaceAuditLogger::log() , Intune\AuditLogger::log()
Persisted	YES — stored in AuditLog.metadata (JSONB)

Rules (exact logic):

return str_contains($key, 'token')
    || str_contains($key, 'secret')
    || str_contains($key, 'password')
    || str_contains($key, 'authorization')
    || str_contains($key, 'private_key')
    || str_contains($key, 'client_secret');

False positive examples: Same as F-1 — any audit metadata key containing password or token as a substring is redacted. If audit context includes policy configuration metadata (e.g., passwordMinimumLength), it will be masked.

Risk Score: 🟠 HIGH — Audit log data corruption. Less severe than F-1 because audit logs typically contain operational metadata, not raw policy payloads. However, if policy config keys appear in audit context (e.g., during drift detection logging), they will be silently destroyed.

---

Finding F-3: VerificationReportSanitizer (MEDIUM — SEV-3)

Attribute	Value
File	app/Support/Verification/VerificationReportSanitizer.php
Lines	10-18 (FORBIDDEN_KEY_SUBSTRINGS), 131, 276, 357, 415, 443-460
Layer	UI rendering (non-persistent)
What's redacted	Values/keys containing: access_token, refresh_token, client_secret, authorization, password, cookie, set-cookie
Entry points	VerificationReportViewer (Filament UI), OperationRunResource
Persisted	NO — sanitization at rendering time only

Rules:

private const FORBIDDEN_KEY_SUBSTRINGS = [
    'access_token', 'refresh_token', 'client_secret',
    'authorization', 'password', 'cookie', 'set-cookie',
];
// Used via str_contains() on key names AND value strings

False positive examples: If a verification report evidence value contains the substring "password" (e.g., a check message like "passwordMinimumLength must be ≥ 8"), the entire value would be nulled out. The containsForbiddenKeySubstring is also applied to values, not just keys (L357, L415), which could null out diagnostic strings.

Risk Score: 🟡 MEDIUM — No data loss (rendering only), but could hide useful diagnostic information from operators.

---

Finding F-4: RunFailureSanitizer::sanitizeMessage() (LOW — SEV-3)

Attribute	Value
File	app/Support/OpsUx/RunFailureSanitizer.php
Lines	119-140
Layer	Error message sanitization (persistent in OperationRun context)
What's redacted	Bearer tokens, JWT-like strings, specific key=value patterns (access_token, refresh_token, client_secret, password)
Entry points	Various jobs, OperationRunService
Persisted	YES — stored in OperationRun error context

Rules: Uses exact key names (access_token, refresh_token, client_secret, password) in regex patterns matching key=value or "key":"value" format. Also does a final str_ireplace of these exact substrings.

False positive risk: LOW. The regex targets key=value patterns specifically. However, the blanket str_ireplace on line 137 will replace the substring password in ANY position — a message like "Check passwordMinimumLength policy" becomes "Check [REDACTED]MinimumLength policy". But these are error messages, not structured data.

Risk Score: 🟢 LOW — Messages may be garbled but no structured data loss.

---

Finding F-5: GenerateReviewPackJob::redactArrayPii() (SAFE)

Attribute	Value
File	app/Jobs/GenerateReviewPackJob.php
Lines	340-352
Layer	Export (ZIP download)
What's redacted	PII fields: displayName, display_name, userPrincipalName, user_principal_name, email, mail
Entry points	Review pack generation
Persisted	No (exported ZIP)

Rules: Exact key match via in_array($key, $piiKeys, true).

Risk Score: 🟢 SAFE — Uses exact key matching. No false positive risk for policy config keys.

---

Finding F-6: VerificationReportSanitizer::sanitizeMessage() (LOW — SEV-3)

Attribute	Value
File	app/Support/Verification/VerificationReportSanitizer.php
Lines	382-413
Layer	UI rendering
What's redacted	Same pattern as RunFailureSanitizer: Bearer tokens, key=value secrets, long opaque blobs, plus blanket str_ireplace

Same issue as F-4: The str_ireplace at L404-409 will corrupt any message containing "password" as a substring.

---

Finding F-7: DriftEvidence::sanitize() (SAFE)

Attribute	Value
File	app/Services/Drift/DriftEvidence.php
Lines	12-28
Layer	Drift evidence formatting
What's redacted	Nothing — this is an allowlist-based key filter, not a secret redactor

Risk Score: 🟢 SAFE — Uses allowlist approach. No false positives.

---

Finding F-8: InventoryMetaSanitizer (SAFE)

Attribute	Value
File	app/Services/Inventory/InventoryMetaSanitizer.php
Layer	Inventory metadata normalization
What's redacted	Nothing secret-related — normalizes structural metadata (odata_type, etag, scope_tag_ids)

Risk Score: 🟢 SAFE — Not a secret redactor at all.

---

Finding F-9: GraphContractRegistry::sanitizeArray() (SAFE)

Attribute	Value
File	app/Services/Graph/GraphContractRegistry.php
Lines	637+
Layer	Graph API payload preparation (restore path)
What's redacted	Read-only/metadata keys stripped for Graph API compliance

Risk Score: 🟢 SAFE — Not secret redaction. Strips OData metadata keys using exact-match lists.

---

Finding F-10: Model-level $hidden (SAFE)

Models	Hidden attributes
User	password, remember_token
ProviderCredential	payload (encrypted:array)
AlertDestination	(needs verification)
PlatformUser	(needs verification)

Risk Score: 🟢 SAFE — Standard Laravel serialization protection. Correct usage.

---

Finding F-11: ProviderCredentialObserver (SAFE)

Attribute	Value
File	app/Observers/ProviderCredentialObserver.php
Layer	Audit logging for credential changes
What's redacted	Logs 'redacted_fields' => ['client_secret'] as metadata

Risk Score: 🟢 SAFE — Explicitly documents redacted fields; doesn't perform substring matching.

---

Root Cause Analysis

Primary Root Cause: Substring Regex on Key Names

The PolicySnapshotRedactor (F-1) is the single root cause by severity. It uses:

'/password/i'  →  preg_match('/password/i', 'passwordMinimumLength') === 1  ← FALSE POSITIVE

This pattern has no word boundaries, no exact-match semantics, and no allowlist of known safe keys.

Secondary Root Cause: Redaction at Persistence Time

The redactor runs before data is stored in PolicyVersion.snapshot. This makes the data loss permanent and irrecoverable. If redaction only happened at rendering/export time, the original data would still be available for drift detection, compare, and restore.

Tertiary Root Cause: No Centralized Redaction Service

Three independent implementations (PolicySnapshotRedactor, AuditContextSanitizer, VerificationReportSanitizer) each define their own rules with inconsistent approaches. There is no single source of truth for "what is a secret key."

---

Patch Plan

Phase 1: Quick Fix — Allowlist + Word Boundaries (PRIORITY A — do first)

Fix PolicySnapshotRedactor to use an explicit allowlist of exact secret key names instead of substring regex:

// BEFORE (broken):
private const array SENSITIVE_KEY_PATTERNS = [
    '/password/i',
    '/secret/i',
    '/token/i',
    // ...
];

// AFTER (fixed):
private const array EXACT_SECRET_KEYS = [
    'password',
    'wifi_password',
    'wifiPassword',
    'clientSecret',
    'client_secret',
    'sharedSecret',
    'shared_secret',
    'sharedKey',
    'shared_key',
    'preSharedKey',
    'pre_shared_key',
    'presharedKey',
    'psk',
    'privateKey',
    'private_key',
    'certificatePassword',
    'certificate_password',
    'pfxPassword',
    'pfx_password',
    'token',              // bare "token" only — NOT tokenType
    'accessToken',
    'access_token',
    'refreshToken',
    'refresh_token',
    'bearerToken',
    'bearer_token',
    'secret',             // bare "secret" only — NOT secretReferenceValueId
    'passphrase',
];

// Match exact key name (case-insensitive):
private function isSensitiveKey(string $key): bool
{
    $normalized = strtolower(trim($key));
    foreach (self::EXACT_SECRET_KEYS as $secretKey) {
        if ($normalized === strtolower($secretKey)) {
            return true;
        }
    }
    return false;
}

Apply the same fix to AuditContextSanitizer::shouldRedactKey().

Phase 2: Move Redaction to Render Time (PRIORITY A — architectural fix)

Currently the redaction happens before persistence:

Graph API → PolicySnapshotRedactor → PolicyVersion.snapshot (REDACTED! 💀)

The correct architecture is:

Graph API → PolicyVersion.snapshot (ORIGINAL ✅)
                   ↓
         [On read/render/export]
                   ↓
         PolicySnapshotRedactor → UI / Export / Notification

This requires:

1. Remove snapshotRedactor->redactPayload() calls from PolicyCaptureOrchestrator and VersionService::captureVersion()
2. Apply redaction at rendering time in Filament resources, export jobs, and notification formatters
3. Ensure logs (Log::info/warning) never include raw snapshot data (they currently don't — they only log IDs)

Migration consideration: Existing PolicyVersion records with [REDACTED] values cannot be recovered. Document this as known data loss for historical versions.

Phase 3: Centralized Redaction Service (PRIORITY B — proper fix)

Create a single SecretKeyClassifier service:

final class SecretKeyClassifier
{
    // Exact secret keys from Intune/Graph schemas
    private const array SECRET_KEYS = [ /* ... */ ];
    
    // Keys that LOOK like secrets but are config values (safety net)
    private const array SAFE_CONFIG_KEYS = [
        'passwordMinimumLength',
        'passwordRequired',
        'passwordExpirationDays',
        'passwordMinimumCharacterSetCount',
        'passwordBlockSimple',
        'passwordPreviousPasswordBlockCount',
        'deviceCompliancePasswordRequired',
        'localAdminPasswordRotationEnabled',
        'certificateValidityPeriodScale',
        'certificateRenewalThresholdPercentage',
        'tokenType',
        'tokenAccountType',
        // ... extend as new policy types are added
    ];
    
    public function isSecretKey(string $key): bool { /* ... */ }
    public function isKnownSafeConfigKey(string $key): bool { /* ... */ }
}

All three sanitizer systems (PolicySnapshotRedactor, AuditContextSanitizer, VerificationReportSanitizer) should delegate to this single classifier.

Phase 4: Schema-Driven Secret Classification (PRIORITY C — long-term)

Extend GraphContractRegistry with per-policy-type secret field metadata:

'deviceConfiguration' => [
    // ...existing...
    'secret_fields' => ['wifi_password', 'preSharedKey', 'certificatePassword'],
],

The SecretKeyClassifier would then consult the contract registry for type-specific secret lists, providing the most precise classification possible.

---

Required Tests

Unit Tests (new)

tests/Unit/PolicySnapshotRedactorFalsePositiveTest.php

Sample test data:

{
    "passwordMinimumLength": 12,
    "passwordRequired": true,
    "wifi_password": "SuperGeheimesPasswort123!",
    "clientSecret": "abc",
    "token": "xyz",
    "deviceCompliancePasswordRequired": true,
    "localAdminPasswordRotationEnabled": true,
    "certificatePassword": "pfxpw",
    "sharedKey": "psk",
    "osVersionMinimum": "10.0.22631",
    "certificateValidityPeriodScale": "years",
    "tokenType": "user",
    "passwordExpirationDays": 90,
    "passwordBlockSimple": true
}

Expected after redaction:

Key	Expected
passwordMinimumLength	12 (VISIBLE)
passwordRequired	true (VISIBLE)
wifi_password	[REDACTED]
clientSecret	[REDACTED]
token	[REDACTED]
deviceCompliancePasswordRequired	true (VISIBLE)
localAdminPasswordRotationEnabled	true (VISIBLE)
certificatePassword	[REDACTED]
sharedKey	[REDACTED]
osVersionMinimum	10.0.22631 (VISIBLE)
certificateValidityPeriodScale	years (VISIBLE)
tokenType	user (VISIBLE)
passwordExpirationDays	90 (VISIBLE)
passwordBlockSimple	true (VISIBLE)

Unit Tests (update existing)

• tests/Unit/AuditContextSanitizerTest.php — add false-positive key tests
• tests/Feature/Intune/PolicySnapshotRedactionTest.php — add config key preservation tests
• tests/Feature/Audit/WorkspaceAuditLoggerRedactionTest.php — add config key preservation

Integration Tests

• Drift compare with passwordMinimumLength changes → must detect change
• Baseline compare with passwordRequired difference → must show diff
• Review pack export → config keys visible, actual secrets redacted
• Restore flow → original config values preserved through full cycle

---

Guard Rails (Regression Prevention)

1. CI Regression Test

A dedicated test that asserts known Intune config keys are NEVER redacted:

it('never redacts known Intune configuration keys', function (string $key, mixed $value) {
    $redactor = new PolicySnapshotRedactor();
    $result = $redactor->redactPayload([$key => $value]);
    expect($result[$key])->toBe($value, "Config key '{$key}' was incorrectly redacted");
})->with([
    ['passwordMinimumLength', 12],
    ['passwordRequired', true],
    ['passwordExpirationDays', 90],
    ['passwordMinimumCharacterSetCount', 4],
    ['passwordBlockSimple', true],
    ['passwordPreviousPasswordBlockCount', 5],
    ['deviceCompliancePasswordRequired', true],
    ['localAdminPasswordRotationEnabled', true],
    ['certificateValidityPeriodScale', 'years'],
    ['certificateRenewalThresholdPercentage', 20],
    ['tokenType', 'user'],
]);

2. Static Analysis / CI Grep

Add a CI step that fails if any new code introduces substring-matching regex against "password"/"secret"/"token" without word boundaries:

# Fail if new redaction patterns use substring matching
grep -rn --include='*.php' "preg_match.*password\|str_contains.*password" app/ \
  | grep -v "SAFE_CONFIG_KEYS\|EXACT_SECRET_KEYS\|SecretKeyClassifier" \
  && echo "FAIL: New substring-based password detection found" && exit 1

3. Central Redaction Service

All secret-classification logic must flow through SecretKeyClassifier (Phase 3). No component should independently define what constitutes a "secret key."

---

Summary Table

#	File	Layer	Persisted	False Positives	Severity	Fix Priority
F-1	PolicySnapshotRedactor.php	Storage	YES	HIGH (password*, certificate*, token*)	SEV-1	IMMEDIATE
F-2	AuditContextSanitizer.php	Audit	YES	HIGH (password, token substrings)	SEV-2	HIGH
F-3	VerificationReportSanitizer.php	UI	No	MEDIUM (password in values)	SEV-3	MEDIUM
F-4	RunFailureSanitizer.php	Errors	YES	LOW (str_ireplace on messages)	SEV-3	LOW
F-5	GenerateReviewPackJob.php	Export	No	None (exact match)	SAFE	—
F-6	VerificationReportSanitizer::sanitizeMessage()	UI	No	LOW (same as F-4)	SEV-3	LOW
F-7	DriftEvidence.php	Drift	No	None (allowlist)	SAFE	—
F-8	InventoryMetaSanitizer.php	Inventory	No	None (structural only)	SAFE	—
F-9	GraphContractRegistry.php	API prep	No	None (exact match)	SAFE	—
F-10	Model $hidden	Serialization	N/A	None	SAFE	—
F-11	ProviderCredentialObserver.php	Audit	No	None (explicit docs)	SAFE	—

---

Assumptions & Caveats

1. ASSUMPTION: Intune Graph API responses contain keys like passwordMinimumLength at the top-level of policy JSON. Verified against Microsoft Graph documentation for deviceManagement policies — confirmed.

2. ASSUMPTION: No other secret redaction exists in Monolog processors or Laravel exception handlers. Verified: no custom Monolog processors found in config/logging.php or app/Exceptions/.

3. CANNOT VERIFY: Whether any existing PolicyVersion snapshots in production have already suffered data loss from this bug. Recommendation: Query production DB for PolicyVersion records where snapshot::jsonb ? 'passwordMinimumLength' and check if the value is [REDACTED].

4. ASSUMPTION: Telescope / Debugbar are not deployed to production. If they are, their built-in redaction (which typically uses similar patterns) could also cause false positives in debug data.

5. Double redaction in orchestrator path: When PolicyCaptureOrchestrator::capture() calls VersionService::captureVersion(), the redactor runs twice (once in each). This is functionally harmless (idempotent) but indicates unclear ownership.