feat: add enterprise security hooks for GDPR/SOC2/HIPAA compliance #528
Conversation
Add comprehensive enterprise security hook for GDPR/SOC2/HIPAA compliance. Features: - File Protection: Blocks access to sensitive files (.env, SSH keys, secrets) - Compliance Check: Detects PII, hardcoded credentials, insecure logging - Bash Validation: Blocks dangerous bash commands - Configurable via disabled_hooks Implements createEnterpriseSecurityHook() with full documentation.
|
Thank you for your contribution! Before we can merge this PR, we need you to sign our Contributor License Agreement (CLA). To sign the CLA, please comment on this PR with: This is a one-time requirement. Once signed, all your future contributions will be automatically accepted. I have read the CLA Document and I hereby sign the CLA Rene seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. |
Greptile SummaryAdds comprehensive enterprise security hook that protects sensitive files (.env, SSH keys, credentials), detects PII and hardcoded credentials, and blocks dangerous bash commands to support GDPR/SOC2/HIPAA/PCI DSS compliance. Major Changes:
Issues Found:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant OpenCode
participant EnterpriseSecurityHook
participant Utils
participant ToolExecutor
User->>OpenCode: Execute tool (Read/Write/Edit/Bash)
OpenCode->>EnterpriseSecurityHook: tool.execute.before(input, output)
alt Tool is Read/Write/Edit
EnterpriseSecurityHook->>Utils: isProtectedFile(filePath)
Utils-->>EnterpriseSecurityHook: {blocked: true/false, reason}
alt File is protected
EnterpriseSecurityHook->>OpenCode: Set blocked=true, message
OpenCode-->>User: SECURITY: Access denied
end
end
alt Tool is Write/Edit
EnterpriseSecurityHook->>Utils: checkComplianceViolations(content)
Utils-->>EnterpriseSecurityHook: violations[]
alt Has error violations (hardcoded credentials)
EnterpriseSecurityHook->>OpenCode: Set blocked=true, message
OpenCode-->>User: COMPLIANCE WARNING: Errors detected
else Has warning violations (PII)
EnterpriseSecurityHook->>EnterpriseSecurityHook: console.warn()
end
end
alt Tool is Bash
EnterpriseSecurityHook->>Utils: checkDangerousBashCommand(command)
Utils-->>EnterpriseSecurityHook: {dangerous: true/false, reason}
alt Command is dangerous
EnterpriseSecurityHook->>OpenCode: Set blocked=true, message
OpenCode-->>User: SECURITY: Dangerous command blocked
end
end
alt Not blocked
OpenCode->>ToolExecutor: Execute tool
ToolExecutor-->>OpenCode: output
OpenCode->>EnterpriseSecurityHook: tool.execute.after(input, output)
EnterpriseSecurityHook->>Utils: checkComplianceViolations(output.output)
Utils-->>EnterpriseSecurityHook: violations[]
alt Has warning violations
EnterpriseSecurityHook->>EnterpriseSecurityHook: console.warn()
end
OpenCode-->>User: Return output
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Comments (4)
-
src/hooks/enterprise-security/constants.ts, line 101 (link)style: IP addresses in code (e.g.,
127.0.0.1,0.0.0.0) will trigger false PII warnings. Consider removing this pattern or adding context checking (e.g., only flag if associated with user data keywords)Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
-
src/hooks/enterprise-security/constants.ts, line 99 (link)style: Phone pattern
\b\(?\d{3}\)?[-. ]?\d{3}[-. ]?\d{4}\bwill match many non-phone number sequences (e.g., version numbers like(123) 456-7890format timestamps). This may cause excessive false positive warnings -
src/hooks/enterprise-security/index.ts, line 86-87 (link)style:
console.warnmay not be visible to users in non-interactive environments or when OpenCode redirects output. Consider using the plugin's notification system or adding warnings to the tool output messageNote: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
-
src/hooks/enterprise-security/constants.ts, line 127 (link)logic: Regex
/rm\s+-rf\s+\/(?!tmp|var\/tmp)/iuses negative lookahead correctly but will still matchrm -rf /home/tmp(which is dangerous). The pattern only checks the immediate next path component. Consider more comprehensive validation or exact path matching
9 files reviewed, 4 comments
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 9 files
Confidence score: 2/5
- Multiple high-severity security gaps remain in
src/hooks/enterprise-security/constants.ts, including incompletermflag coverage and missingcurl|sh/wget|bashprotections, so the PR currently exposes command-execution bypasses. normalizePath()and allowlist checks insrc/hooks/enterprise-security/utils.tsare vulnerable to traversal and substring bypasses, making it likely that.envand other sensitive files can be accessed despite intended safeguards.- Lack of try/catch in
src/hooks/enterprise-security/index.tshook functions risks crashing sessions, which contradicts the documented guidance and could impact users immediately. - Pay close attention to
src/hooks/enterprise-security/constants.ts,src/hooks/enterprise-security/utils.ts,src/hooks/enterprise-security/index.ts- address the identified security gaps and missing error handling before merging.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/hooks/enterprise-security/constants.ts">
<violation number="1" location="src/hooks/enterprise-security/constants.ts:127">
P1: Security gap: Pattern only catches `rm -rf` but not equivalent `rm -fr`. The flags `-rf` and `-fr` are interchangeable in the `rm` command, allowing `rm -fr /` to bypass this protection.</violation>
<violation number="2" location="src/hooks/enterprise-security/constants.ts:133">
P1: Security gap: `curl | sh` and `wget | bash` are not blocked. The patterns inconsistently check for different shells - curl only blocks piping to `bash` while wget only blocks piping to `sh`. Both tools can use either shell for remote code execution.</violation>
</file>
<file name="src/hooks/enterprise-security/index.ts">
<violation number="1" location="src/hooks/enterprise-security/index.ts:18">
P1: Hook functions lack error handling which could crash the session. The hooks documentation explicitly warns against this anti-pattern: "Missing try/catch (don't crash session)". Wrap the function body in try/catch to prevent crashes from unexpected input or utility function failures.</violation>
</file>
<file name="src/hooks/enterprise-security/utils.ts">
<violation number="1" location="src/hooks/enterprise-security/utils.ts:23">
P1: Path traversal bypass: `normalizePath()` doesn't resolve `..` components. A path like `safe/../.env` bypasses the `.env` protection check but accesses the actual `.env` file when read by the filesystem. Use `path.normalize()` to properly resolve path components.</violation>
<violation number="2" location="src/hooks/enterprise-security/utils.ts:32">
P1: Security bypass: `includes()` allows any path containing an allowed substring to pass the allowlist. For example, `malicious.opencode/.env` would be allowed because it contains `.opencode/`. Use proper path prefix/suffix checking or segment-based matching instead of substring matching.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| // Dangerous bash commands | ||
| export const DANGEROUS_BASH_PATTERNS = [ | ||
| /rm\s+-rf\s+\/(?!tmp|var\/tmp)/i, // rm -rf / (except /tmp) |
There was a problem hiding this comment.
P1: Security gap: Pattern only catches rm -rf but not equivalent rm -fr. The flags -rf and -fr are interchangeable in the rm command, allowing rm -fr / to bypass this protection.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/enterprise-security/constants.ts, line 127:
<comment>Security gap: Pattern only catches `rm -rf` but not equivalent `rm -fr`. The flags `-rf` and `-fr` are interchangeable in the `rm` command, allowing `rm -fr /` to bypass this protection.</comment>
<file context>
@@ -0,0 +1,135 @@
+
+// Dangerous bash commands
+export const DANGEROUS_BASH_PATTERNS = [
+ /rm\s+-rf\s+\/(?!tmp|var\/tmp)/i, // rm -rf / (except /tmp)
+ /:\(\)\{\s*:\|:&\s*\};:/, // Fork bomb
+ /dd\s+if=\/dev\/zero/i, // Disk wipe
</file context>
| /mkfs\./i, // Format filesystem | ||
| /> \/dev\/sd[a-z]/i, // Direct disk write | ||
| /chmod\s+-R\s+777\s+\//i, // Chmod 777 on root | ||
| /curl.*\|\s*bash/i, // Curl | bash (potential security risk) |
There was a problem hiding this comment.
P1: Security gap: curl | sh and wget | bash are not blocked. The patterns inconsistently check for different shells - curl only blocks piping to bash while wget only blocks piping to sh. Both tools can use either shell for remote code execution.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/enterprise-security/constants.ts, line 133:
<comment>Security gap: `curl | sh` and `wget | bash` are not blocked. The patterns inconsistently check for different shells - curl only blocks piping to `bash` while wget only blocks piping to `sh`. Both tools can use either shell for remote code execution.</comment>
<file context>
@@ -0,0 +1,135 @@
+ /mkfs\./i, // Format filesystem
+ /> \/dev\/sd[a-z]/i, // Direct disk write
+ /chmod\s+-R\s+777\s+\//i, // Chmod 777 on root
+ /curl.*\|\s*bash/i, // Curl | bash (potential security risk)
+ /wget.*\|\s*sh/i, // Wget | sh (potential security risk)
+];
</file context>
| } from "./utils"; | ||
|
|
||
| export function createEnterpriseSecurityHook(_ctx: PluginInput) { | ||
| const toolExecuteBefore = async ( |
There was a problem hiding this comment.
P1: Hook functions lack error handling which could crash the session. The hooks documentation explicitly warns against this anti-pattern: "Missing try/catch (don't crash session)". Wrap the function body in try/catch to prevent crashes from unexpected input or utility function failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/enterprise-security/index.ts, line 18:
<comment>Hook functions lack error handling which could crash the session. The hooks documentation explicitly warns against this anti-pattern: "Missing try/catch (don't crash session)". Wrap the function body in try/catch to prevent crashes from unexpected input or utility function failures.</comment>
<file context>
@@ -0,0 +1,152 @@
+} from "./utils";
+
+export function createEnterpriseSecurityHook(_ctx: PluginInput) {
+ const toolExecuteBefore = async (
+ input: ToolExecuteInput,
+ output: ToolExecuteBeforeOutput,
</file context>
| const allowedNormalized = normalizePath(allowed); | ||
| if ( | ||
| normalized.endsWith(allowedNormalized) || | ||
| normalized.includes(allowedNormalized) |
There was a problem hiding this comment.
P1: Security bypass: includes() allows any path containing an allowed substring to pass the allowlist. For example, malicious.opencode/.env would be allowed because it contains .opencode/. Use proper path prefix/suffix checking or segment-based matching instead of substring matching.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/enterprise-security/utils.ts, line 32:
<comment>Security bypass: `includes()` allows any path containing an allowed substring to pass the allowlist. For example, `malicious.opencode/.env` would be allowed because it contains `.opencode/`. Use proper path prefix/suffix checking or segment-based matching instead of substring matching.</comment>
<file context>
@@ -0,0 +1,134 @@
+ const allowedNormalized = normalizePath(allowed);
+ if (
+ normalized.endsWith(allowedNormalized) ||
+ normalized.includes(allowedNormalized)
+ ) {
+ return true;
</file context>
| if (normalized.startsWith("./")) { | ||
| normalized = normalized.slice(2); | ||
| } | ||
| return normalized; |
There was a problem hiding this comment.
P1: Path traversal bypass: normalizePath() doesn't resolve .. components. A path like safe/../.env bypasses the .env protection check but accesses the actual .env file when read by the filesystem. Use path.normalize() to properly resolve path components.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/enterprise-security/utils.ts, line 23:
<comment>Path traversal bypass: `normalizePath()` doesn't resolve `..` components. A path like `safe/../.env` bypasses the `.env` protection check but accesses the actual `.env` file when read by the filesystem. Use `path.normalize()` to properly resolve path components.</comment>
<file context>
@@ -0,0 +1,134 @@
+ if (normalized.startsWith("./")) {
+ normalized = normalized.slice(2);
+ }
+ return normalized;
+}
+
</file context>
1 participant
Pull Request: Enterprise Security Hooks
Summary
Add comprehensive enterprise security hooks for GDPR, SOC2, HIPAA, and PCI DSS compliance in OpenCode.
This PR introduces a new
enterprise-securityhook that provides:Motivation
Enterprise users need built-in security controls to prevent accidental exposure of:
This hook helps organizations meet compliance requirements for GDPR, SOC2, HIPAA, and PCI DSS.
Implementation Details
New Files
Hook Integration
tool.execute.before: Validates file access, content compliance, bash commands (blocking)tool.execute.after: Scans tool output for sensitive data (warning only)Configuration
Users can disable the hook via:
{ "disabled_hooks": ["enterprise-security"] }Protected Patterns
Files (glob patterns):
.env,.env.*,*.env.ssh/*,*.pem,*.key,*_rsasecrets/*,.secrets/*,credentials.json.aws/*,gcp-credentials.json,firebase-adminsdk*.json*.tfstate,terraform.tfvarsapi-keys.*,.npmrc,.docker/config.jsonPII Patterns (regex):
Hardcoded Credentials (regex):
api_key = "..."password = "..."token = "..."AKIA[0-9A-Z]{16}Dangerous Bash (regex):
rm -rf /(except /tmp)curl | bash/wget | shCompliance Standards
Helps meet requirements for:
Testing
bun run build)Examples
Example 1: Protected File Blocked
Example 2: Hardcoded Credential Blocked
Example 3: Dangerous Bash Blocked
Documentation
Complete documentation available in
src/hooks/enterprise-security/AGENTS.mdincluding:Breaking Changes
None. This is a new opt-in hook (enabled by default but can be disabled).
Checklist
bun run typecheckpassesbun run buildsucceedspackage.jsonsrc/hooks/index.tssrc/index.tsHookNameSchemainsrc/config/schema.tsRelated Issues
Closes: N/A (proactive enhancement for enterprise users)
Contributors
Note: This contribution is based on existing security patterns from my personal Claude Code configuration, now adapted for OhMyOpenCode to benefit the community.
Summary by cubic
Adds an enterprise security hook that protects sensitive files, detects PII and hardcoded credentials, and blocks dangerous bash commands to help meet GDPR, SOC2, HIPAA, and PCI DSS. This reduces secret leaks and unsafe operations by blocking risky actions and warning on potential exposure.
Written for commit 631563e. Summary will update on new commits.