fix(security): sanitize GET /api/config — exclude TLS file paths and add plugin sanitization contract

[SckyzO]

fix(security): sanitize GET /api/config output — exclude filesystem paths to TLS material and prevent plugin secret leakage

Symptom

GET /api/config is a public read endpoint that returns the running AppConfig. It already excludes a few known-sensitive fields, but the exclusion list is incomplete:

# src/rackscope/api/routers/config.py:28-34
enriched_config = app_config.model_dump(
    exclude={
        "telemetry": {"basic_auth_password"},
        "auth": {"password_hash", "secret_key"},
    }
)

Three filesystem paths to TLS material are returned in clear:

• telemetry.tls_ca_file
• telemetry.tls_cert_file
• telemetry.tls_key_file

The last one is particularly sensitive — it is the path to the private key used to authenticate to Prometheus when mTLS is configured. Disclosing this path:

1. Helps a network-adjacent attacker construct targeted file-read attacks (e.g., via a future path-traversal vulnerability, or via a misconfigured container with shared volumes).
2. Reveals filesystem layout of the deployment (/etc/rackscope/certs/prometheus-client.key style hints).
3. Confirms that mTLS is in use, which by itself helps an attacker understand the environment.

Additionally, the endpoint enriches plugin configuration with plugin.config.model_dump() without any sanitization. Today's bundled plugins (simulator, slurm) have no secrets in their config, but future plugins (IPMI credentials, API tokens, third-party DCIM access keys) would silently leak through this endpoint with no warning to the plugin author.

Technical Analysis

Why a denylist is the wrong long-term shape

The current implementation is a model_dump(exclude={...}) denylist. Every time a new sensitive field is added to AppConfig, the author must remember to add it to this list. Forgetting is silent. There is no test that fails when a new sensitive field appears.

The correct long-term shape is an allowlist: explicitly enumerate the fields safe to return, refuse everything else by default. New fields must be opt-in to the public response.

However, an allowlist refactor is more invasive and can break the frontend Settings panel that consumes GET /api/config. This issue ships the immediate denylist extension to close the known leak, and explicitly flags the allowlist refactor as a recommended follow-up (Phase 2 below).

Plugin secret leakage — forward-looking concern

# src/rackscope/api/routers/config.py:37-42
if registry and hasattr(registry, "_plugins"):
    for plugin_id, plugin in registry._plugins.items():
        if hasattr(plugin, "config") and plugin.config:
            enriched_config["plugins"][plugin_id] = plugin.config.model_dump()

Today, simulator and slurm have no secret fields, so no leak occurs in practice. But the contract is wrong: a future plugin that adds, for example, ipmi_admin_password: str would silently leak through this endpoint.

A plugin should declare what its config can safely expose, rather than the core router needing to know about every plugin's internals.

Proposed Fix

Phase 1 — Immediate denylist extension (this PR)

1. Extend the exclude block in src/rackscope/api/routers/config.py

enriched_config = app_config.model_dump(
    exclude={
        "telemetry": {
            "basic_auth_password",
            "tls_ca_file",
            "tls_cert_file",
            "tls_key_file",
        },
        "auth": {
            "password_hash",
            "secret_key",
        },
    }
)

2. Add a sanitized_config() contract on the plugin base class

In src/rackscope/plugins/base.py (or wherever the abstract plugin lives), add a default method on the base class:

class Plugin(ABC):
    config: Optional[BaseModel] = None

    def sanitized_config(self) -> Optional[dict]:
        """Return a dict safe to expose via /api/config.

        Default implementation: same as model_dump(). Plugins with sensitive
        fields MUST override this method and exclude those fields explicitly.
        """
        if self.config is None:
            return None
        return self.config.model_dump()

Update the router to use the new contract:

if registry and hasattr(registry, "_plugins"):
    for plugin_id, plugin in registry._plugins.items():
        sanitized = plugin.sanitized_config() if hasattr(plugin, "sanitized_config") else None
        if sanitized is not None:
            enriched_config["plugins"][plugin_id] = sanitized

The current bundled plugins (simulator, slurm) keep their existing behaviour — they inherit the default sanitized_config() which returns the full dump (no secrets present). This is non-breaking.

Document the contract in rackscope_documentation/docs/plugins/index.md (or equivalent): "If your plugin config contains secrets, override sanitized_config() to exclude them. The default implementation returns the full dump — fine for plugins with no secrets, dangerous otherwise."

Phase 2 — Recommended follow-up (separate future issue, NOT this PR)

Refactor GET /api/config to use an explicit allowlist (Pydantic include={...} or a dedicated response model PublicAppConfigResponse). The frontend Settings panel is updated to consume this dedicated response shape rather than the raw AppConfig.

This change is non-trivial because the frontend currently treats GET /api/config as a mirror of AppConfig. Track as separate issue with its own review cycle.

Frontend impact

The frontend Settings panel that consumes GET /api/config to show "TLS CA file: /etc/.../ca.pem" must be updated. Replace the displayed value with a boolean indicator: TLS CA: configured / TLS CA: not configured. Same for cert and key.

This is non-breaking from a UX perspective — the user still sees that mTLS is configured, just without revealing the exact path.

Files to update (approximate, verify at implementation time):

• frontend/src/app/pages/editors/SettingsPage.tsx or wherever telemetry config is displayed
• Any read-only "current configuration" rendering

Test Checklist

Unit tests

• tests/unit/test_config_sanitization.py: full AppConfig with all sensitive fields populated → model_dump(exclude=...) does not contain tls_ca_file, tls_cert_file, tls_key_file, basic_auth_password, password_hash, secret_key
• tests/unit/test_config_sanitization.py: missing TLS fields (None) → response shape unchanged from the existing contract (no new keys appear)

Functional tests

• tests/integration/test_config_endpoint.py: GET /api/config returns 200, response JSON does not contain any of the six sensitive field names anywhere in the structure (full recursive scan against a known sensitive-keys list)
• tests/integration/test_config_endpoint.py: GET /api/config with auth disabled (default) still works as before — non-regression
• tests/integration/test_config_endpoint.py: GET /api/config with auth enabled and valid JWT still works as before — non-regression

Plugin sanitization tests

• tests/unit/plugins/test_sanitized_config.py: a mock plugin with secret: str = "***" in its config and an overridden sanitized_config() returning {} → GET /api/config does not contain ***
• tests/unit/plugins/test_sanitized_config.py: a mock plugin with the default sanitized_config() (no override) → returns the full dump (current behaviour, no regression)

Security regression test (the important one)

Add tests/security/test_no_secret_in_config.py that:

1. Builds an AppConfig populated with every known sensitive field set to a unique sentinel string ("SECRET_TLS_CA_PATH_xyz", "SECRET_PASSWORD_HASH_xyz", etc.)
2. Calls GET /api/config
3. Asserts none of the sentinel strings appear anywhere in the serialized response

This single test prevents future leaks: when a new sensitive field is added to AppConfig, the author must either add it to the exclude list (test passes) or accept the leak (test fails, PR rejected). It is the safety net that compensates for the denylist shape.

Impact and Severity

• Audience affected: every deployment that uses mTLS to authenticate to Prometheus (HPC clusters with internal CA, on-prem deployments with corporate PKI). The exposure is read-only and requires network access to the API, so the immediate risk is moderate — but for security-sensitive environments, leaking the location of a TLS private key is unacceptable.
• Severity: high. Information disclosure of paths to private key material. Compounded by the fact that GET /api/config is unauthenticated in the default config (auth.enabled=false), so the leak is reachable anonymously.
• Priority: ship in the v1.0.0 security hardening series alongside Issue A.

Breaking Changes

None at the API contract level. The three TLS path fields disappear from the response, but:

• They were not documented as a stable part of the contract.
• The frontend update (showing "configured" / "not configured" instead of the path) preserves the user-facing information without revealing the path.
• The plugin sanitized_config() default implementation matches the previous behaviour for existing plugins (simulator, slurm).

Related

• Audit ref: SEC-04 in AUDIT_ARCHITECTURAL.md
• Companion to Issue A (require_admin hardening) — both are part of the v1.0.0 security hardening series, but distinct concerns (mutation auth vs. response sanitization). Ship as separate PRs.
• Future: Phase 2 allowlist refactor (separate issue, not blocking this fix).

Out of Scope

• Allowlist refactor of GET /api/config (Phase 2, separate issue).
• Hiding auth.username (low-value change, would regress the Settings UI showing "Current admin: admin"; deferred unless a real threat case emerges).
• Auditing every plugin for unsafe model_dump() patterns in their own endpoints (this issue's contract handles the central GET /api/config; plugin-internal endpoints are reviewed during plugin code reviews).
• Adding a Cache-Control: no-store header to GET /api/config to prevent caching of formerly-leaked TLS paths in CDNs / proxies. Considered overkill for the threat model; can be added in a follow-up if requested.