Skip to content

Harden Cascade and Codex hook handling#16

Open
haasonsaas wants to merge 2 commits into
CorridorSecurity:mainfrom
haasonsaas:security/hookshot-findings
Open

Harden Cascade and Codex hook handling#16
haasonsaas wants to merge 2 commits into
CorridorSecurity:mainfrom
haasonsaas:security/hookshot-findings

Conversation

@haasonsaas

Copy link
Copy Markdown

Summary

  • add Cascade RunE blocking helpers and update Cascade examples to use exit-code blocking
  • route unsupported Codex Bash heredoc write shapes through the existing raw fallback path
  • report direct cat-to-tee heredoc pipeline writes and avoid treating shell-expanded path tokens as canonical file paths

Tests

  • go test ./cascade ./codex
  • go test .
  • go test ./...

I kept the PR description implementation-focused because this touches hook policy enforcement behavior.

@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: dcedfe002b

ℹ️ 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 codex/bash_write.go Outdated
Comment on lines +108 to +109
if needsFallback {
return nil, true

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 Preserve parsed writes when falling back

When a command contains both a successfully parsed write and any later fallback-triggering write, this drops all accumulated files and the unified bridge emits a single raw fallback with an empty FilePath. For example, cat <<'EOF' > .env ... EOF; cat <<'EOF' > $HOME/out ... EOF first records .env, then the non-literal $HOME/out sets needsFallback, so FilePath-only policies never see the literal .env write. Please dispatch the parsed files as well as the fallback, or otherwise avoid replacing known write targets with an empty-path event.

Useful? React with 👍 / 👎.

Comment thread codex/bash_write.go
Comment on lines +169 to +173
paths, status := teeTargetPaths(teeCall)
if status != writeFound {
return nil, status
}
return patchFilesForPaths(paths, body), writeFound

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 Include tee stdout redirects in pipeline writes

The new pipeline path only extracts tee's argv targets, but tee also copies stdin to stdout (confirmed via tee --help), and Bash can redirect that stdout to another file. With cat <<'EOF' | tee allowed.txt > ../../.ssh/authorized_keys, this reports only allowed.txt while the redirected stdout writes the same heredoc to authorized_keys, so FilePath-based deny policies miss the sensitive sink. Please include stdout redirects on the tee statement or fall back to the raw command when one is present.

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.

1 participant