Skip to content

fix(acl): separate scope enumeration from store-level bypass#226

Merged
rwmjhb merged 1 commit intoCortexReach:masterfrom
Disaster-Terminator:upstream-prep/acl-bypass-clean-v2
Mar 17, 2026
Merged

fix(acl): separate scope enumeration from store-level bypass#226
rwmjhb merged 1 commit intoCortexReach:masterfrom
Disaster-Terminator:upstream-prep/acl-bypass-clean-v2

Conversation

@Disaster-Terminator
Copy link
Copy Markdown
Contributor

Summary

This PR separates scope enumeration from store-level bypass handling by introducing an explicit store-layer scope filter API (getScopeFilter() / resolveScopeFilter(...)) instead of inferring bypass from getAccessibleScopes().

It also fixes related consistency issues in hook agentId resolution, importConfig() validation, ACL key normalization, reserved bypass ID handling, and reflection loading.

What changed

Scope enumeration vs. bypass

  • Introduced getScopeFilter(agentId?) and resolveScopeFilter(...) as the explicit store-layer API for bypass signaling
  • Kept getAccessibleScopes() as pure enumeration — it no longer implies store bypass
  • Enforced scopeFilter semantics: [] denies all reads/writes, while undefined means bypass
  • Behavior is now consistent across list(), delete(), and update() paths

Hook agentId resolution

  • Unified before_agent_start and before_prompt_build to both use resolveHookAgentId() with sessionKey fallback
  • Fixed before_prompt_build falling back to hardcoded "main" when only ctx.sessionKey is available

ImportConfig validation

  • importConfig() now suppresses console.warn during validation; warnings emit only after successful validation
  • Failed validation rolls back config completely without leaking misleading warnings

ACL hardening

  • Normalized agentAccess keys (trim on construction/import) to prevent whitespace-related failures
  • Centralized reserved bypass ID rejection (system, undefined) in both config and session key parsing

Reflection loading

  • Updated reflection loading to handle bypass callers correctly when scopeFilter is omitted
  • Preserved automatic access to reflection:agent:{id} even with explicit agentAccess config

Compatibility notes

Custom or legacy scope managers must implement getScopeFilter() explicitly if they need store-level bypass behavior.

Returning [] from getAccessibleScopes() should no longer be relied on to imply bypass. The legacy fallback remains supported for compatibility, but bypass is now defined as an explicit store-layer behavior.

Tests

Added / expanded regression coverage for:

  • test/reflection-bypass-hook.test.mjs — hook bypass and sessionKey agentId resolution
  • test/scope-access-undefined.test.mjs — bypass ID rejection and scope access control
  • test/smart-extractor-scope-filter.test.mjs — SmartExtractor scopeFilter semantics
  • test/store-empty-scope-filter.test.mjs — empty array deny-all semantics

All existing tests continue to pass.

@rwmjhb
Copy link
Copy Markdown
Collaborator

rwmjhb commented Mar 16, 2026

I think this PR has real value, and I’d support bringing it in once the current conflicts with master are resolved.

What I like here is that this is not “abstraction for abstraction’s sake”. It fixes a genuine semantic mix-up that was spread across multiple layers:

  • getAccessibleScopes() was being used both as an enumeration API and as an implicit store-bypass signal
  • scopeFilter=[] vs scopeFilter=undefined was not consistently meaningful across store/read paths
  • hook agent resolution was not fully consistent between before_agent_start and before_prompt_build
  • reflection loading and trusted/bypass callers were coupled to those ambiguities

The separation introduced here (getScopeFilter() / resolveScopeFilter(...)) makes the contract much clearer:

  • undefined means intentional bypass
  • [] means deny-all
  • non-empty arrays mean an actual scoped filter
  • getAccessibleScopes() remains an enumeration/access view rather than a transport for bypass semantics

That is a worthwhile cleanup because it improves correctness, not just code style.

I also like that this is backed by targeted regression coverage rather than just comments/docstrings. I ran the new ACL/scope tests locally, and I also ran the full npm test suite on the PR branch in an isolated worktree. Everything passed there, which gives me a lot more confidence that this change is internally coherent and not just locally green in the new test files.

The one practical caveat is that GitHub currently shows this PR as CONFLICTING, so I would not treat this as merge-ready in its current branch state. But from a value/correctness perspective, I think this is worth rebasing or selectively porting rather than closing out.

So my read is:

  • good direction
  • meaningful correctness win
  • worth integrating
  • but needs conflict resolution against current master before merge

核心变更:
- ACL 键规范化:trim agentAccess 键,避免空白填充导致 ACL 失效
- 明确 deny-all 语义:scopeFilter=[] 拒绝所有读写,与 undefined(绕过)区分
- 统一 hook agentId 解析:before_agent_start 和 before_prompt_build 均使用 resolveHookAgentId
- importConfig 原子性:验证失败时警告不泄漏,配置完整回滚
- 保留 bypass ID 限制:system/undefined 仅限内部使用,拒绝配置和 sessionKey 解析
- reflection 加载安全:bypass 调用方使用可选 scopeFilter,避免过滤绕过

新增测试:
- test/reflection-bypass-hook.test.mjs - hook bypass 和 sessionKey 解析
- test/scope-access-undefined.test.mjs - bypass ID 拒绝和 scope 访问
- test/smart-extractor-scope-filter.test.mjs - SmartExtractor scopeFilter 语义
- test/store-empty-scope-filter.test.mjs - 空数组 deny-all 语义

影响范围:6 个源文件 + 4 个测试文件,约 970 行净变动
@Disaster-Terminator Disaster-Terminator force-pushed the upstream-prep/acl-bypass-clean-v2 branch from 422940d to a1185c2 Compare March 16, 2026 05:01
@Disaster-Terminator
Copy link
Copy Markdown
Contributor Author

Rebased onto current master and resolved the remaining conflict.
It was only the package.json test script merge; npm test passes after the rebase, and the PR is now mergeable.

@rwmjhb rwmjhb merged commit a0ce458 into CortexReach:master Mar 17, 2026
3 checks passed
AliceLJY pushed a commit to AliceLJY/memory-lancedb-pro that referenced this pull request Mar 19, 2026
…on (CortexReach#226)

核心变更:
- ACL 键规范化:trim agentAccess 键,避免空白填充导致 ACL 失效
- 明确 deny-all 语义:scopeFilter=[] 拒绝所有读写,与 undefined(绕过)区分
- 统一 hook agentId 解析:before_agent_start 和 before_prompt_build 均使用 resolveHookAgentId
- importConfig 原子性:验证失败时警告不泄漏,配置完整回滚
- 保留 bypass ID 限制:system/undefined 仅限内部使用,拒绝配置和 sessionKey 解析
- reflection 加载安全:bypass 调用方使用可选 scopeFilter,避免过滤绕过

新增测试:
- test/reflection-bypass-hook.test.mjs - hook bypass 和 sessionKey 解析
- test/scope-access-undefined.test.mjs - bypass ID 拒绝和 scope 访问
- test/smart-extractor-scope-filter.test.mjs - SmartExtractor scopeFilter 语义
- test/store-empty-scope-filter.test.mjs - 空数组 deny-all 语义

影响范围:6 个源文件 + 4 个测试文件,约 970 行净变动
Papyrus0 pushed a commit to Papyrus0/memory-lancedb-pro-fork that referenced this pull request Mar 20, 2026
…on (CortexReach#226)

核心变更:
- ACL 键规范化:trim agentAccess 键,避免空白填充导致 ACL 失效
- 明确 deny-all 语义:scopeFilter=[] 拒绝所有读写,与 undefined(绕过)区分
- 统一 hook agentId 解析:before_agent_start 和 before_prompt_build 均使用 resolveHookAgentId
- importConfig 原子性:验证失败时警告不泄漏,配置完整回滚
- 保留 bypass ID 限制:system/undefined 仅限内部使用,拒绝配置和 sessionKey 解析
- reflection 加载安全:bypass 调用方使用可选 scopeFilter,避免过滤绕过

新增测试:
- test/reflection-bypass-hook.test.mjs - hook bypass 和 sessionKey 解析
- test/scope-access-undefined.test.mjs - bypass ID 拒绝和 scope 访问
- test/smart-extractor-scope-filter.test.mjs - SmartExtractor scopeFilter 语义
- test/store-empty-scope-filter.test.mjs - 空数组 deny-all 语义

影响范围:6 个源文件 + 4 个测试文件,约 970 行净变动
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