Skip to content

🧃 feat: Add Opt-In RTK Shell Output Filtering - #41

Open
danny-avila wants to merge 4 commits into
mainfrom
danny-avila/optional-rtk-output-filter
Open

🧃 feat: Add Opt-In RTK Shell Output Filtering#41
danny-avila wants to merge 4 commits into
mainfrom
danny-avila/optional-rtk-output-filter

Conversation

@danny-avila

Copy link
Copy Markdown
Collaborator

Summary

I added request-scoped RTK output filtering for Bash executions while preserving raw execution as the default and keeping all command rewriting inside the sandbox boundary.

  • Add shell_output_filter: "raw" | "rtk" to the public and internal execution contracts with Bash-only validation at both trust boundaries.
  • Build RTK v0.45.0 from its immutable release commit with Rust 1.91 so amd64 and arm64 binaries match the Debian bookworm sandbox glibc baseline.
  • Execute RTK rewrites from the Bash runtime wrapper inside NsJail, preserve script arguments, and fail open to the original script when rewriting is unsupported or unavailable.
  • Keep RTK history and tee artifacts ephemeral for opted-in runs and reserve the internal environment controls against caller override.
  • Record low-cardinality Prometheus execution outcomes and stdout/stderr byte sizes for raw-versus-RTK comparisons.
  • Document the opt-in request field, direct binary availability, image behavior, metrics, and Apache-2.0 attribution.

Change Type

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Testing

  • Ran cd api && bun run build.
  • Ran cd service && bun run build; the existing Rollup export/circular-dependency warnings remain unchanged.
  • Ran cd api && bun test src/bash-run.test.ts src/job-helpers.test.ts (69 passed).
  • Ran cd service && bun test src/config.spec.ts (17 passed).
  • Ran bash -n docker/bash-run.sh docker/package-init.sh.
  • Built the rtk-builder target from both Dockerfiles on linux/arm64 and verified rtk 0.45.0.
  • Verified the real binary rewrites git status && git diff to rtk git status && rtk git diff, honors the ephemeral RTK environment, and creates no files under sandbox HOME.
  • Attempted the complete API and service suites; this managed environment blocks their existing ephemeral socket listeners and external Redis/k6 dependencies, and inherited AWS_CA_BUNDLE trips existing hardened-startup fixtures. No focused RTK test failed.

Test Configuration:

  • macOS arm64 host
  • Docker Desktop linux/arm64 builder
  • Bun 1.3.13
  • RTK 0.45.0
  • Rust 1.91 bookworm builder

Checklist

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • I have commented in complex areas
  • I have made pertinent documentation changes
  • My changes do not introduce new warnings
  • I have written tests demonstrating that my changes are effective

@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 737eb2c130

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@danny-avila
danny-avila marked this pull request as ready for review August 7, 2026 23:17
Comment thread docker/bash-run.sh Outdated
@danny-avila danny-avila changed the title feat: add opt-in RTK shell output filtering 🧃 feat: Add Opt-In RTK Shell Output Filtering Sep 2, 2026
@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

Please review the current PR head 7de33dd. Confirm that this exact commit is the reviewed commit and ignore findings that apply only to earlier heads.

@danny-avila
danny-avila force-pushed the danny-avila/optional-rtk-output-filter branch from 7de33dd to 13b5915 Compare September 6, 2026 18:46
@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

Please review the current PR head 13b5915. Confirm that this exact commit is the reviewed commit and ignore findings that apply only to earlier heads.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-06T19:06:14.495928Z f09636c Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 13b5915b1d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread api/Dockerfile
Comment thread docker/bash-run.sh Outdated
@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

Please review the current PR head f09636c. Confirm that this exact commit is the reviewed commit and ignore findings that apply only to earlier heads.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f09636c30f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docker/bash-run.sh
# Match parameter or array expansion rather than a literal mention in a
# comment or string. The wrapper only needs to preserve file execution
# when the script actually depends on Bash's source-file metadata.
if ! grep -qE '\$\{BASH_SOURCE([^[:alnum:]_]|$)|\$BASH_SOURCE([^[:alnum:]_]|$)|(^|[^[:alnum:]_])BASH_SOURCE\[' -- "$1" 2>/dev/null; then

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 Handle prefixed BASH_SOURCE expansions

At the confirmed current head f09636c30f2c4ca081773dddc66e9d1d60ad6650, fresh evidence shows that the revised guard still misses valid prefix-operator forms such as ${#BASH_SOURCE} because the pattern requires { to be followed immediately by BASH_SOURCE. For an opted-in script using this expansion, file execution reports the source path's nonzero length, whereas the rewritten bash -c path reports 0, so filtering silently changes program behavior. Extend the guard to recognize Bash prefix operators as well, or avoid executing rewritten scripts via a command string.

Useful? React with 👍 / 👎.

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.

2 participants