Skip to content

[Security] Redact persisted secrets and harden controller file permissions #229

Description

@fettpl

Audit metadata

  • Priority: P1
  • Estimated effort: M
  • Implementation risk: MED
  • Category: security
  • Evidence baseline: 07be5be7ce69bea0c3118744ab90d148b010fce0 (origin/main on 2026-07-17)

Dependencies

None.

Description and impact

The controller intentionally keeps raw secrets in on-disk controller/engine logs and relies on default umask for log and installer-created credential files. API keys, provider tokens, or authorization headers printed by a dependency can therefore remain readable to other local users. Redact at every persistence/console boundary and enforce owner-only directories/files, including existing installations.

Existing issue overlap: no matching open or closed issue was found.

Current state and evidence

  • controller/src/core/log-redaction.ts:1-7 says raw files are preserved and redaction is HTTP/SSE-only.
  • controller/tests/integration/log-redaction.test.ts:141-177 asserts synthetic secrets remain in the file after GET.
  • controller/src/core/logger.ts:20-29,57-74 writes the raw formatted line with default modes; console output is raw too.
  • controller/src/modules/engines/process/process-manager.ts:311-335 writes raw child lines to the engine log and recent failure tail.
  • scripts/install-controller.sh:49-65 creates .env containing the controller API key without umask/chmod; lines 83-91 append raw service output to controller.log.
  • controller/src/stores/sqlite.ts:42-53 is the repository precedent for owner-only sensitive storage (0600).

Do not include any discovered or real value in the implementation plan or tests.

Solution design

Make redactLogLine idempotent and apply it before a line reaches console, file stream, SSE callback, engine recent tail, or engine event publication. One formatted/redacted logger line should feed every sink. Create log/data directories as 0700 and files as 0600; repair existing modes best-effort on startup. Precreate service logs privately and set an owner-only umask in the installer and systemd unit. Set .env to 0600 on both new and reused installs without printing its contents.

Keep API redaction as defense in depth. Do not add a default raw-log escape hatch. If an opt-in diagnostic mode is required, hit the STOP condition for a separate threat-model decision. Add no source comments.

Verification commands

Purpose Command Expected on success
Redaction tests cd controller && bun test tests/integration/log-redaction.test.ts exit 0
Process tests cd controller && bun test tests/integration/process-manager-spawn.test.ts exit 0
Shell syntax bash -n scripts/install-controller.sh exit 0
Controller gate npm run check:controller exit 0
Full integration npm run test:integration exit 0

Scope

In scope

  • controller/src/core/log-redaction.ts
  • controller/src/core/log-files.ts
  • controller/src/core/logger.ts
  • controller/src/modules/engines/process/process-manager.ts
  • controller/tests/integration/log-redaction.test.ts
  • controller/tests/integration/process-manager-spawn.test.ts if needed for engine output
  • scripts/install-controller.sh
  • One new hermetic installer-permission shell test under scripts/ if the repository's test harness supports it

Out of scope

  • Rotating any operator credential; document that operators should rotate secrets known to have entered old logs.
  • Encrypting SQLite or logs at rest.
  • Broadening the redactor to guess arbitrary unlabelled high-entropy strings.
  • Publishing raw diagnostics by default.

Implementation plan

Step 1: Replace the raw-file assertion with sink tests

Use synthetic sentinels to test logger file output, console interception, SSE/onLine output, engine child stdout/stderr, recent failure tails, and GET logs. Assert the sentinel is absent and [redacted] is present. Add mode assertions using statSync(path).mode & 0o777 on Unix.

Verify: cd controller && bun test tests/integration/log-redaction.test.ts -> persistence assertions fail on baseline.

Step 2: Redact once before all logger and engine sinks

Refactor named formatting helpers so raw details never reach a sink. Redact engine lines before storing, writing, or publishing. Preserve ordinary diagnostic context and make a second redaction pass a no-op.

Verify: redaction and process-manager targeted tests pass.

Step 3: Enforce owner-only filesystem modes

Update log directory creation/opening to request 0700/0600 and chmod existing paths where supported. Avoid following or replacing unexpected symlinks; fail closed for secret-bearing files when a safe open cannot be established.

Verify: targeted tests assert exact modes and pass on supported Unix CI; unsupported filesystems exercise a documented safe failure/best-effort branch without leaking data.

Step 4: Harden installer-created files

Set umask 077 before secret/file creation, chmod reused .env, create data/log paths privately, precreate controller.log as 0600, and set UMask=0077 in the generated service. Ensure health output retains the existing machine-readable API-key handoff but no debug/log line prints the value.

Verify: bash -n scripts/install-controller.sh and the hermetic installer permission test -> exit 0; .env/logs are 0600 and directories 0700.

Test plan

Extend log-redaction.test.ts rather than duplicating regex cases. Use fake process output in process-manager-spawn.test.ts. Test new/reused files under a permissive test umask, existing 0644 repair, console/service paths, redaction idempotence, and permission failure. Never use plausible live credential values.

Acceptance criteria

  • Known secret markers never appear in default persisted controller or engine logs.
  • Console, file, SSE, and failure-tail views receive consistently redacted text.
  • .env, database, controller logs, and engine logs are 0600; containing private directories are 0700 on Unix.
  • Existing permissive files are repaired idempotently where supported.
  • Installer and systemd paths establish an owner-only umask.
  • Tests contain synthetic sentinels only.
  • npm run check and npm run test:integration exit 0.

Risks and stop conditions

  • A product requirement insists on retaining raw logs by default; obtain an explicit threat-model/product decision.
  • Safe file creation would require following an existing symlink or overwriting an unknown file.
  • A proposed test or diagnostic would expose a real credential.
  • Changes require an encryption/key-management design.
  • Source drift or two verification failures occur.

Maintenance considerations

Every future log sink must receive already-redacted lines and owner-only storage. Reviewers should inspect error/fallback paths, not only the normal logger, and release notes should advise rotation/removal of secrets from historical logs.


Generated from Local Studio 2.0 main-branch audit proposal 010.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions