Skip to content

Comments

bridge: fix stale runtime slack-bridge path references#160

Merged
benvinegar merged 2 commits intomainfrom
fix/slack-bridge-opt-release-path
Feb 24, 2026
Merged

bridge: fix stale runtime slack-bridge path references#160
benvinegar merged 2 commits intomainfrom
fix/slack-bridge-opt-release-path

Conversation

@benvinegar
Copy link
Member

@benvinegar benvinegar commented Feb 24, 2026

Summary

  • update tool-guard protected runtime bridge security paths to /opt/baudbot/current/slack-bridge/*
  • expand chmod-runtime-security deny rule to also match /opt/baudbot/current/slack-bridge/security* while keeping legacy runtime matcher for defense-in-depth
  • update tool-guard tests to reflect /opt release path protections
  • update control-agent startup-cleanup.sh bridge launch loop to bypass varlock run and load env directly from ~/.config/.env after clearing inherited SLACK_BROKER_* vars
  • add deploy cleanup for legacy ~/runtime/slack-bridge directory

Validation

  • bin/test.sh (15/15 passed)

@greptile-apps
Copy link

greptile-apps bot commented Feb 24, 2026

Greptile Summary

This PR migrates slack-bridge security path references from the legacy agent-owned runtime/slack-bridge location to the immutable release path /opt/baudbot/current/slack-bridge. The changes ensure defense-in-depth by protecting both old and new paths in tool-guard patterns while updating runtime code to use only the release path.

Key changes:

  • Updated tool-guard.ts protected paths to reference /opt/baudbot/current/slack-bridge/security.mjs instead of runtime/slack-bridge/security.mjs
  • Expanded chmod deny rule to match both legacy runtime/slack-bridge/security and new /opt/baudbot/current/slack-bridge/security paths
  • Modified startup-cleanup.sh to bypass varlock and load env vars directly from $HOME/.config/.env after clearing inherited SLACK_BROKER_* vars
  • Added deploy cleanup that removes legacy runtime/slack-bridge directory
  • Updated all corresponding tests to reflect new path protections

All tests pass and the change properly maintains security boundaries while migrating to the immutable release model.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • All changes are well-tested (87 tool-guard tests, 77 bridge security tests pass), maintain defense-in-depth by protecting both old and new paths, and follow the architectural pattern of migrating to immutable /opt/baudbot releases. The varlock bypass change properly clears and reloads env vars to prevent stale configuration.
  • No files require special attention

Important Files Changed

Filename Overview
bin/deploy.sh Added cleanup of legacy runtime/slack-bridge directory path as bridge now runs from /opt/baudbot/current/slack-bridge
pi/extensions/tool-guard.ts Updated protected paths from runtime/slack-bridge to /opt/baudbot/current/slack-bridge, added new chmod deny pattern for release path
pi/extensions/tool-guard.test.mjs Updated test paths and expectations to match new /opt/baudbot/current/slack-bridge release path, marked bridge files as immutable
pi/skills/control-agent/startup-cleanup.sh Improved Node bin resolution, removed varlock wrapper, added explicit env clearing and reloading in bridge launch loop

Last reviewed commit: 0dea634

echo \"[\$(date -Is)] bridge: starting $BRIDGE_SCRIPT\" >> $BRIDGE_LOG_FILE; \
varlock run --path \$HOME/.config/ -- node $BRIDGE_SCRIPT >> $BRIDGE_LOG_FILE 2>&1; \
for v in \$(env | grep ^SLACK_BROKER_ | cut -d= -f1 || true); do unset \$v; done; \
set -a; source \$HOME/.config/.env; set +a; \
Copy link

Choose a reason for hiding this comment

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

Bug: The script sources .env without error handling. If the file is missing or invalid, the bridge starts without required environment variables, causing an infinite crash loop.
Severity: CRITICAL

Suggested Fix

Ensure the script exits if sourcing the .env file fails. This can be achieved by adding set -e to the beginning of the tmux command string or by checking the exit code of the source command, for example: source \$HOME/.config/.env || exit 1.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: pi/skills/control-agent/startup-cleanup.sh#L149

Potential issue: The script sources the `$HOME/.config/.env` file within a `tmux`
command that does not have `set -e` enabled. If the `source` command fails because the
file is missing, unreadable, or contains syntax errors, the script will not exit.
Instead, it will proceed to launch the Node.js bridge script without the required
environment variables. The bridge script is designed to exit immediately if these
variables are not present. Since this logic is inside a `while true` loop, this will
result in an infinite crash loop, preventing the Slack integration from functioning and
consuming system resources.

Did we get this right? 👍 / 👎 to inform future reviews.

@benvinegar benvinegar merged commit c67a481 into main Feb 24, 2026
9 checks passed
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