#22388: Feat/aligned safe openclaw
agents
size: XL
Cluster:
Test Coverage Enhancements
Summary
- Problem: src/security/constitution.ts has unclosed if/try blocks at lines 42–44 that trap the rest of the function inside the constitutionPath branch — when no path is provided the function
returns undefined. The file-loading code also parses customData but never uses it, and a malicious JSON file could inject "immutable": true principles that cannot be disabled. Separately,
src/security/input-validator.ts has a regex.exec() loop with no guard against zero-length matches, risking an infinite loop.
- Why it matters: The constitution loader is broken for the default (no-path) case, which is the primary usage path. The immutable injection vector undermines the safety model's trust boundary
between built-in and user-supplied principles. The regex loop is a latent DoS/hang risk.
- What changed: (1) Closed the if/try blocks, added a catch for invalid paths, merged file-loaded principles with immutable: false forced, removed a duplicate comment. (2) Added a match[0].length
=== 0 → regex.lastIndex++ guard in detectRepetition.
- What did NOT change (scope boundary): output-filter.ts (already fixed in 4ff33ce17), no new dependencies, no config schema changes, no changes to the default constitution JSON, no test files
included in this PR.
Change Type (select all)
- Bug fix
- Feature
- Refactor
- Docs
- Security hardening
- Chore/infra
Scope (select all touched areas)
- Gateway / orchestration
- Skills / tool execution
- Auth / tokens
- Memory / storage
- Integrations
- API / contracts
- UI / DX
- CI/CD / infra
Linked Issue/PR
- Related #19729 (addresses 3 of 4 greptile-apps review comments; the 4th was already fixed)
User-visible / Behavior Changes
- loadConstitution() called without a constitutionPath now correctly returns the default constitution instead of undefined.
- Custom constitution files loaded via constitutionPath now actually merge their principles into the constitution (previously parsed and discarded).
- File-loaded principles can no longer claim immutable: true status.
Security Impact (required)
- New permissions/capabilities? No
- Secrets/tokens handling changed? No
- New/changed network calls? No
- Command/tool execution surface changed? No
- Data access scope changed? No
Repro + Verification
Environment
- OS: macOS (Darwin 25.2.0)
- Runtime/container: Node.js with Vitest v4.0.18
- Model/provider: N/A
- Integration/channel: N/A
- Relevant config: Default security alignment config
Steps
1. npx vitest run src/security/constitution.test.ts — verify constitution loading, invalid path fallback, custom principle merging
2. npx vitest run src/security/input-validator.test.ts — verify repetition detection completes without hanging
3. npx vitest run src/security/ — full suite (289 tests across 16 files)
Expected
- All tests pass, including "handles invalid constitution file path gracefully", immutable injection prevention, and zero-length match edge case
Actual
- 289/289 tests pass, 0 failures
Evidence
- Failing test/log before + passing after
- Trace/log snippets
- Screenshot/recording
- Perf numbers
constitution.test.ts "handles invalid constitution file path gracefully" threw an uncaught exception before the fix (no catch block). After: passes. Full suite output:
✓ src/security/constitution.test.ts (14 tests) 7ms
✓ src/security/input-validator.test.ts (15 tests) 4ms
Test Files 16 passed (16)
Tests 289 passed (289)
Human Verification (required)
- Verified scenarios: Default constitution loading (no config), invalid file path fallback, valid file path with principle merging, immutable injection attempt forced to false, regex loop
termination on repetitive content
- Edge cases checked: Malformed JSON in constitution file (caught by catch), customData.principles not an array (guarded by Array.isArray), zero-length regex match (guarded by lastIndex++)
- What I did not verify: Behavior on Windows/Linux, interaction with the alignment plugin loader at runtime, performance under very large custom constitution files
Compatibility / Migration
- Backward compatible? Yes
- Config/env changes? No
- Migration needed? No
Failure Recovery (if this breaks)
- How to disable/revert this change quickly: git revert <commit-sha> — single commit, no migrations
- Files/config to restore: src/security/constitution.ts, src/security/input-validator.ts
- Known bad symptoms reviewers should watch for: loadConstitution() returning undefined or missing principles, detectRepetition hanging on unusual input
Risks and Mitigations
- Risk: File-loaded principles that previously relied on setting immutable: true will now be demoted to immutable: false and become disableable.
- Mitigation: This is intentional — only built-in default principles should be immutable. External files are an untrusted input boundary.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
Added security alignment and safety oversight framework with constitutional principles, prompt injection detection, and input validation. Previous review comments addressed: non-immutable defaults are now replaced (not appended) when loading custom constitution files, errors are logged instead of silently swallowed, and zero-length regex matches no longer inflate repetition counts in `detectRepetition`.
<h3>Confidence Score: 4/5</h3>
- This PR is generally safe to merge with minor concerns around test coverage verification
- The core security logic in `constitution.ts` and `input-validator.ts` has been iteratively fixed to address identified issues. However, the PR description claims tests pass but no test files are included in the PR diff, making it impossible to verify test coverage. The implementation adds 21 new files (2677 lines) which is substantial, but the critical security boundaries (immutable principles, zero-length match guards, error logging) appear sound after the fixes in commits 004c0257 and 7df85c94.
- No specific files require special attention - the previous review threads identified issues that have been addressed in subsequent commits
<sub>Last reviewed commit: 7df85c9</sub>
<!-- greptile_other_comments_section -->
<!-- /greptile_comment -->
Most Similar PRs
#19500: Custom rust ultimate rewrite
by adybag14-cyber · 2026-02-17
76.7%
#20496: test(utils): add comprehensive unit tests for utility functions
by masifislamm · 2026-02-19
76.7%
#23574: security: P0 critical remediation — plugin sandbox, password hashin...
by lumeleopard001 · 2026-02-22
76.5%
#8086: feat(security): Add prompt injection guard rail
by bobbythelobster · 2026-02-03
76.4%
#11048: fix: address repository issues (env, author, CI comments, security ...
by cavula · 2026-02-07
76.3%
#15794: docs(security): comprehensive security audit report
by kinder-world · 2026-02-13
75.8%
#15757: feat(security): add hardening gap audit checks
by saurabhsh5 · 2026-02-13
75.3%
#20596: Funding
by reconsumeralization · 2026-02-19
75.3%
#19814: Codex/align delimiter parsing assertion with parser
by Johnsonbros · 2026-02-18
75.2%
#8821: Security: Holistic capability-based sandbox (replaces pattern-match...
by tonioloewald · 2026-02-04
75.1%