Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions launchd/install-newsyslog.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#!/usr/bin/env bash
Comment thread
macroscopeapp[bot] marked this conversation as resolved.
set -euo pipefail

SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
SRC="$SCRIPT_DIR/newsyslog.d/brainlayer.conf"
DST="/etc/newsyslog.d/brainlayer.conf"
OWNER="${BRAINLAYER_LOG_OWNER:-${SUDO_USER:-$(id -un)}}"
GROUP="${BRAINLAYER_LOG_GROUP:-staff}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GROUP variable not validated for whitespace like LOG_DIR

Low Severity

BRAINLAYER_LOG_GROUP is never validated for whitespace or existence. The script explicitly rejects whitespace in LOG_DIR because "newsyslog.conf is whitespace-delimited," but the same reasoning applies to the owner:group field. Unlike OWNER, which is implicitly validated via id -u (which rejects non-existent users), GROUP passes through unchecked. A whitespace-containing value would produce a malformed newsyslog config line where field boundaries shift.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a0cbf99. Configure here.

RENDERED_CONFIG="$(mktemp "${TMPDIR:-/tmp}/brainlayer-newsyslog.XXXXXX")"
trap 'rm -f "$RENDERED_CONFIG"' EXIT

escape_sed_replacement() {
printf '%s' "$1" | sed 's/[\/#&\\]/\\&/g'
}

if ! id -u "$OWNER" >/dev/null 2>&1; then
echo "ERROR: log owner does not exist: $OWNER" >&2
exit 1
fi

if ! OWNER_HOME="$(dscl . -read "/Users/$OWNER" NFSHomeDirectory 2>/dev/null | sed 's/^NFSHomeDirectory:[[:space:]]*//')"; then
echo "ERROR: could not resolve home directory for $OWNER" >&2
exit 1
fi
if [ -z "$OWNER_HOME" ]; then
echo "ERROR: could not resolve home directory for $OWNER" >&2
exit 1
Comment thread
cursor[bot] marked this conversation as resolved.
fi
LOG_DIR="${BRAINLAYER_LOG_DIR:-$OWNER_HOME/Library/Logs/brainlayer}"
if [[ "$LOG_DIR" =~ [[:space:]] ]]; then
echo "ERROR: newsyslog log paths cannot contain whitespace: $LOG_DIR" >&2
exit 1
fi

if [ ! -f "$SRC" ]; then
echo "ERROR: $SRC not found" >&2
exit 1
fi

LOG_DIR_ESCAPED="$(escape_sed_replacement "$LOG_DIR")"
OWNER_GROUP_ESCAPED="$(escape_sed_replacement "$OWNER:$GROUP")"

sed \
-e "s#/Users/etanheyman/Library/Logs/brainlayer#$LOG_DIR_ESCAPED#g" \
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject whitespace log directories before rendering

When the invoking user's home or BRAINLAYER_LOG_DIR contains whitespace, this substitution writes the path into newsyslog.conf as a bare field. newsyslog.conf lines are whitespace-delimited, so a path like /Users/Jane Doe/Library/Logs/brainlayer is parsed as /Users/Jane plus shifted columns, making the installer fail validation instead of supporting the space-preserving home lookup that the new test/README advertise. Please reject such paths with a clear error or render them in a format newsyslog actually accepts.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Escape whitespace in rendered newsyslog paths

When BRAINLAYER_LOG_DIR or the resolved OWNER_HOME contains spaces, this substitution writes the raw path into the config. macOS newsyslog.conf(5) documents that each config line's fields are separated with whitespace, so sudo newsyslog -nv -f "$RENDERED_CONFIG" will parse the path as multiple fields and reject or misconfigure the install despite the added test claiming home paths with spaces are preserved. Render a newsyslog-safe escaped path or reject such paths explicitly.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in a958d49. The installer now rejects whitespace-containing LOG_DIR values before rendering because newsyslog.conf is whitespace-delimited; README and tests document that explicit limitation.

-e "s#etanheyman:staff#$OWNER_GROUP_ESCAPED#g" \
"$SRC" >"$RENDERED_CONFIG"

sudo mkdir -p "$LOG_DIR"
# Ensure already-created logs are writable by user LaunchAgents before the first rotation.
sudo chown "$OWNER:$GROUP" "$LOG_DIR"
for log in "$LOG_DIR"/*.log; do
[ -e "$log" ] || continue
if [ -L "$log" ] || [ ! -f "$log" ]; then
echo "Skipping non-regular log path: $log" >&2
continue
fi
sudo chown "$OWNER:$GROUP" "$log"
Comment on lines +51 to +57
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid following user-controlled log symlinks

When this installer is run by an admin for a BRAINLAYER_LOG_OWNER whose log directory already exists or is user-writable, a *.log symlink in that directory is expanded by the glob and passed to sudo chown without -h; the macOS chown(8) docs indicate -h is the option that changes the link itself, so this follows the link and can transfer ownership/chmod of arbitrary files outside the log directory. Please skip symlinks or operate only on verified regular files before doing privileged chown/chmod.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in a958d49. The ownership repair loop now skips symlinks and non-regular paths before any sudo chown/chmod, so user-controlled log symlinks are not followed.

sudo chmod 0644 "$log"
done
Comment thread
macroscopeapp[bot] marked this conversation as resolved.

sudo newsyslog -nv -f "$RENDERED_CONFIG"
sudo mkdir -p /etc/newsyslog.d
sudo install -o root -g wheel -m 0644 "$RENDERED_CONFIG" "$DST"
sudo newsyslog -nv -f "$DST"
echo "Installed $DST"
31 changes: 31 additions & 0 deletions launchd/newsyslog.d/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# BrainLayer newsyslog

BrainLayer LaunchAgents write logs as the user, not as root. macOS `newsyslog`
creates rotated replacement files as `root:admin` unless the config line
specifies an owner and group. A root-owned replacement silently breaks later
appends from user-level daemons.

This drop-in only rotates finite scheduled LaunchAgent jobs. Long-running jobs
such as BrainBar, watch, and enrichment keep their `StandardOutPath` and
`StandardErrorPath` descriptors open; macOS `newsyslog` has no post-rotate hook
or copy-truncate mode, so those logs need a coupled launchd restart or pid-file
signal path before they can be safely added. Drain is also excluded because it
can be spawned while a rotation pass is running.

Install `brainlayer.conf` into `/etc/newsyslog.d/` with:

```sh
launchd/install-newsyslog.sh
sudo newsyslog -nv -f /etc/newsyslog.d/brainlayer.conf
```

The checked-in config targets Etan's LaunchAgent account as `etanheyman:staff`.
The installer renders that config for `BRAINLAYER_LOG_OWNER`,
`BRAINLAYER_LOG_GROUP`, and `BRAINLAYER_LOG_DIR` before installing it into
`/etc/newsyslog.d/`. The rendered config is validated with `newsyslog -nv`
before replacing the live drop-in. Because `newsyslog.conf` is
whitespace-delimited, the installer rejects log directories containing
whitespace.

Every installed entry uses mode `644`, `J` compression, and `N` so rotation does
not signal user LaunchAgents. Launchd owns job lifecycle.
16 changes: 16 additions & 0 deletions launchd/newsyslog.d/brainlayer.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# BrainLayer finite user LaunchAgent logs.
# owner:group is explicit so rotated files remain writable by user-level daemons.
# Long-running StandardOutPath/StandardErrorPath jobs must not be added without
# a coupled launchd restart or pid-file signal path; newsyslog has no
# post-rotate hook or copy-truncate mode on macOS.
# logfilename owner:group mode count size when flags
/Users/etanheyman/Library/Logs/brainlayer/backup-daily.out.log etanheyman:staff 644 4 512 * JN
/Users/etanheyman/Library/Logs/brainlayer/backup-daily.err.log etanheyman:staff 644 4 512 * JN
/Users/etanheyman/Library/Logs/brainlayer/decay.out.log etanheyman:staff 644 7 1024 * JN
/Users/etanheyman/Library/Logs/brainlayer/decay.err.log etanheyman:staff 644 7 1024 * JN
/Users/etanheyman/Library/Logs/brainlayer/wal-checkpoint.out.log etanheyman:staff 644 7 1024 * JN
/Users/etanheyman/Library/Logs/brainlayer/wal-checkpoint.err.log etanheyman:staff 644 7 1024 * JN
/Users/etanheyman/Library/Logs/brainlayer/index.out.log etanheyman:staff 644 7 1024 * JN
/Users/etanheyman/Library/Logs/brainlayer/index.err.log etanheyman:staff 644 7 1024 * JN
/Users/etanheyman/Library/Logs/brainlayer/repair-fts.out.log etanheyman:staff 644 7 1024 * JN
/Users/etanheyman/Library/Logs/brainlayer/repair-fts.err.log etanheyman:staff 644 7 1024 * JN
87 changes: 87 additions & 0 deletions tests/test_newsyslog_config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
from __future__ import annotations

from pathlib import Path

REPO_ROOT = Path(__file__).resolve().parents[1]
CONFIG = REPO_ROOT / "launchd" / "newsyslog.d" / "brainlayer.conf"
INSTALLER = REPO_ROOT / "launchd" / "install-newsyslog.sh"


def _entries() -> list[list[str]]:
return [
line.split() for line in CONFIG.read_text().splitlines() if line.strip() and not line.lstrip().startswith("#")
]


def test_newsyslog_config_uses_user_writable_rotated_logs():
entries = _entries()
assert entries
for fields in entries:
assert fields[0].startswith("/Users/etanheyman/Library/Logs/brainlayer/")
assert fields[0].endswith(".log")
assert fields[1] == "etanheyman:staff"
assert fields[2] == "644"
assert fields[6] == "JN"


def test_newsyslog_config_covers_launchd_log_pairs():
names = {Path(fields[0]).name for fields in _entries()}
for daemon in {
"backup-daily",
"decay",
"wal-checkpoint",
"index",
"repair-fts",
}:
assert f"{daemon}.out.log" in names
assert f"{daemon}.err.log" in names


def test_newsyslog_config_excludes_held_open_long_running_launchd_logs():
names = {Path(fields[0]).name for fields in _entries()}
for daemon in {"brainbar", "enrichment", "watch", "drain"}:
assert f"{daemon}.out.log" not in names
assert f"{daemon}.err.log" not in names

config = CONFIG.read_text()
assert "post-rotate hook or copy-truncate mode" in config


def test_newsyslog_installer_documents_root_owned_replacement_footgun():
readme = (REPO_ROOT / "launchd" / "newsyslog.d" / "README.md").read_text()
assert "root:admin" in readme
assert "etanheyman:staff" in readme
assert "Long-running jobs" in readme
assert "post-rotate hook" in readme
assert "sudo newsyslog -nv -f /etc/newsyslog.d/brainlayer.conf" in readme


def test_newsyslog_installer_repairs_root_owned_logs_for_invoking_user():
script = INSTALLER.read_text()
assert 'OWNER="${BRAINLAYER_LOG_OWNER:-${SUDO_USER:-$(id -un)}}"' in script
assert 'sudo mkdir -p "$LOG_DIR"' in script
assert '[ -L "$log" ] || [ ! -f "$log" ]' in script
assert "Skipping non-regular log path: $log" in script
assert 'sudo chown "$OWNER:$GROUP" "$log"' in script
assert 'sudo chmod 0644 "$log"' in script


def test_newsyslog_installer_renders_runtime_owner_and_log_dir():
script = INSTALLER.read_text()
assert 'mktemp "${TMPDIR:-/tmp}/brainlayer-newsyslog.XXXXXX"' in script
assert "trap 'rm -f \"$RENDERED_CONFIG\"' EXIT" in script
assert 'LOG_DIR_ESCAPED="$(escape_sed_replacement "$LOG_DIR")"' in script
assert 'OWNER_GROUP_ESCAPED="$(escape_sed_replacement "$OWNER:$GROUP")"' in script
assert "s#/Users/etanheyman/Library/Logs/brainlayer#$LOG_DIR_ESCAPED#g" in script
assert "s#etanheyman:staff#$OWNER_GROUP_ESCAPED#g" in script
assert 'sudo newsyslog -nv -f "$RENDERED_CONFIG"' in script
assert 'sudo install -o root -g wheel -m 0644 "$RENDERED_CONFIG" "$DST"' in script


def test_newsyslog_installer_handles_home_paths_with_spaces_explicitly():
script = INSTALLER.read_text()
assert "awk '{print $2}'" not in script
assert 'if ! OWNER_HOME="$(dscl . -read "/Users/$OWNER" NFSHomeDirectory' in script
assert "sed 's/^NFSHomeDirectory:[[:space:]]*//'" in script
assert '[[ "$LOG_DIR" =~ [[:space:]] ]]' in script
assert "newsyslog log paths cannot contain whitespace" in script
Loading