Skip to content

Add file-based logging#451

Open
mun-iulian-uipath wants to merge 1 commit into
AikidoSec:mainfrom
mun-iulian-uipath:feature/file-logging
Open

Add file-based logging#451
mun-iulian-uipath wants to merge 1 commit into
AikidoSec:mainfrom
mun-iulian-uipath:feature/file-logging

Conversation

@mun-iulian-uipath
Copy link
Copy Markdown

@mun-iulian-uipath mun-iulian-uipath commented May 8, 2026

Summary

Adds opt-in file logging to Safe Chain. When configured, every message Safe Chain writes to the terminal is also appended to a file — useful for debugging CI runs, capturing what Safe Chain saw on a developer's machine, and producing artifacts security teams can grep later.

File logging is off by default and only turns on when a path is configured. It does not change anything about the existing terminal output.

Configuration

Three settings, each accepts a CLI flag, an env var, or a config-file key (priority: CLI > env > config). Bracketed each session with Log started, command: ... and Log ended, command: ... so individual invocations can be told apart in a long-running file.

Setting CLI flag Env var Config key Default
Path --safe-chain-log-file SAFE_CHAIN_LOG_FILE logFile none (disabled)
Format --safe-chain-log-file-format SAFE_CHAIN_LOG_FILE_FORMAT logFileFormat json
Verbosity --safe-chain-log-file-verbosity SAFE_CHAIN_LOG_FILE_VERBOSITY logFileVerbosity verbose

Format

  • json - newline-delimited JSON, one self-contained object per line. Suitable for jq and log shippers.
  • plain - [timestamp] [level] message. Intended for quick human inspection.

Verbosity controls which levels reach the file. Independent from --safe-chain-logging, so users can keep their terminal quiet while still capturing diagnostics.

  • verbose (default) - every entry, including diagnostic output the terminal hides under the default logging level.
  • normal - info, warning, error. Drops verbose.
  • silent - errors only.

Path accepts ~/ and creates parent directories on demand. The file is opened in append mode so existing contents are preserved across runs.

Example

npm install express --safe-chain-log-file=~/safe-chain.log
{"timestamp":"2026-05-08T12:00:00.000Z","level":"info","message":"Log started, command: npm install express"}
{"timestamp":"2026-05-08T12:00:00.123Z","level":"verbose","message":"Fetching malware lists from https://malware-list.aikido.dev"}
{"timestamp":"2026-05-08T12:00:01.456Z","level":"info","message":"✔ Safe-chain: Scanned 142 packages, no malware found."}
{"timestamp":"2026-05-08T12:00:01.789Z","level":"info","message":"Log ended, command: npm install express"}

Behavior on failure

File logging is best-effort and never blocks the wrapped package manager. If the configured path can't be opened or a write fails, Safe Chain emits a single warning and continues without file logging — the install / scan / audit still completes.

Summary by Aikido

⚠️ Security Issues: 1 🔍 Quality Issues: 2 Resolved Issues: 0

🚀 New Features

  • Added opt-in file-based logging with configurable format and verbosity

⚡ Enhancements

  • Extended config, CLI, and env to support log settings
  • Initialized and closed file logger during startup and termination

More info

Comment thread packages/safe-chain/src/config/settings.js
Comment thread packages/safe-chain/src/environment/fileLogger.js
Comment thread packages/safe-chain/src/environment/fileLogger.js
// Both must run even if one throws. Losing the session-end entry
// because stopServer() rejected (or vice versa) defeats the point of
// having a log on failure paths.
await Promise.allSettled([proxy.stopServer(), closeFileLogger()]);
Copy link
Copy Markdown
Contributor

@aikido-pr-checks aikido-pr-checks Bot May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Single-line concurrent shutdown using Promise.allSettled with multiple IO operations; split into named steps or separate awaits to improve clarity.

Suggested change
await Promise.allSettled([proxy.stopServer(), closeFileLogger()]);
const stopServerPromise = proxy.stopServer();
const closeLoggerPromise = closeFileLogger();
await Promise.allSettled([stopServerPromise, closeLoggerPromise]);
Details

✨ AI Reasoning
​This change introduced a single-line shutdown using Promise.allSettled to run two independent shutdown/cleanup operations concurrently. Packing both operations into one expression increases cognitive load when reasoning about shutdown order, error handling, and which side effects may occur. Splitting into clearer steps or naming the promises improves readability and makes failure semantics easier to follow.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant