Repository navigation
feat(ipc): resolve calling user on IPC calls via the registered auth backend - #291
Merged
Merged
Conversation
…backend Separate from the existing HTTP-route RBAC (RBACChecker): a new AuthResolverMiddleware sits in the plugin-call pipeline, just before PermissionMiddleware. It resolves a `principal` (AuthPayload) from a raw `token` via AuthBackend.decode_token() registered by an auth plugin, or passes through an already-resolved `principal` without contacting the backend again. PluginSupervisor._dispatch() exposes it for the duration of the call via a ContextVar (same pattern as the tenant-id ContextVar), read with get_current_principal() — never by mutating the plugin's shared PluginContext. The generic HTTP endpoint resolves it best-effort from the incoming request; TrustedBase.call_plugin() forwards the current principal automatically on nested plugin-to-plugin IPC calls, with no re-decoding. Resolution only — nothing enforces it yet. Also: @action(name, permissions=None) gains an optional declarative permissions list (fn._xcore_action_permissions), mirroring @route's existing permissions param — the hook a future per-action permission check will read. @Schema stays fully independent, usable standalone or stacked with @action in either order. Depend on the now-published xcdk (>=0.2.0) and xcorecli (>=2.3.0) packages instead of only the vendored xcore/sdk/ fallback. Trim makefile targets now superseded by xcorecli (plugin clone/symlink, deploy, service management). Drop stale generated report files under reports/ that shouldn't have been tracked.
IPC principal resolution via the registered auth backend, @action(permissions=...), xcdk/xcorecli as real dependencies.
Contributor
Reviewer's GuideThis release adds non-enforcing authenticated-principal resolution to the HTTP and IPC execution paths, propagates identity safely through nested async calls, introduces declarative action permission metadata, and updates dependencies, tooling, documentation, and release artifacts for version 2.8.0. Sequence diagram for authenticated principal resolution across HTTP and nested IPC callssequenceDiagram
participant C as HTTPClient
participant R as Router
participant A as AuthBackend
participant S as PluginSupervisor
participant M as AuthResolverMiddleware
participant PA as PluginA
participant PB as PluginB
C->>R: POST /plugin/action
R->>A: extract_token(request)
A-->>R: token
R->>A: decode_token(token)
A-->>R: AuthPayload
R->>S: call(..., principal=AuthPayload)
S->>M: pipeline(..., principal=AuthPayload)
M->>S: next_call(..., principal=AuthPayload)
S->>PA: _dispatch()
PA->>PA: get_current_principal()
PA->>S: call_plugin(plugin_b, action, principal)
S->>M: pipeline(..., principal=AuthPayload)
M->>S: next_call(..., principal=AuthPayload)
S->>PB: _dispatch()
PB->>PB: get_current_principal()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
|
github-actions
Bot
added
documentation
Contributor
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 8 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="xcore/kernel/api/auth.py" line_range="78" />
<code_context>
+ Principal (sub/roles/permissions) résolu pour l'appel IPC en cours via
+ l'AuthBackend enregistré — None si aucun token n'a été transmis ou résolu.
+ """
+ return _current_principal.get()
+
+
</code_context>
<issue_to_address>
**Nested plugins trust altered identity**
When plugin code mutates the current principal before making a nested call, `get_current_principal()` exposes a mutable `AuthPayload`, and `TrustedBase.call_plugin()` forwards the same object. Mutations alter the current identity and reach downstream plugins, allowing forged roles or permissions and cross-call interference.
Make principals immutable or return and forward defensive deep copies at both boundaries.
Also at `xcore/kernel/api/contract.py:171`, `xcore/kernel/runtime/supervisor.py:348`.
</issue_to_address>
### Comment 2
<location path="xcore/kernel/runtime/supervisor.py" line_range="348" />
<code_context>
+ # Même pattern que le ContextVar tenant : posé une fois ici pour l'appel
+ # en cours, lu par get_current_principal() dans le plugin — jamais de
+ # mutation d'un objet PluginContext partagé entre requêtes concurrentes.
+ principal_token = _current_principal.set(principal)
+ try:
+ # Les services du plugin sont déjà wrappés (TenantAwareDB/Cache) au
</code_context>
<issue_to_address>
**Background tasks retain caller identity**
When a handler starts a detached task during an authenticated IPC call and that task later makes a nested plugin call, `_dispatch()` resets `_current_principal` only in its own task context; the detached task keeps its inherited copy. `TrustedBase.call_plugin()` then forwards that stale principal, so the downstream plugin sees the original user after the request ends.
Prevent detached tasks from retaining or forwarding the request principal after `_dispatch()` completes.
Also at `xcore/kernel/api/contract.py:171`.
</issue_to_address>
### Comment 3
<location path="makefile" line_range="122-129" />
<code_context>
@poetry run bandit -r xcore/ -f txt -o "$(REPORT_DIR)/bandit.txt" 2>>/dev/null || true
@echo "Rapports : $(REPORT_DIR)/bandit.json / bandit.txt"
-security-check: ## Vérifications rapides (.env, mots de passe en dur)
- @if git ls-files | grep -q "\.env$$"; then \
- echo "❌ .env est tracké par git !"; exit 1; \
- else echo "✅ .env correctement ignoré"; fi
- @if grep -rn "password\s*=\s*[\"'][^\"']*[\"']" xcore/ 2>/dev/null; then \
- echo "❌ Mots de passe potentiels détectés !"; \
- else echo "✅ Aucun mot de passe en dur"; fi
-
# ============================================================
</code_context>
<issue_to_address>
**CI security check becomes a no-op**
When the CI security job or `ci-security` target runs, `ci-security` and the GitHub security job still invoke `make security-check`, but the target's recipe is removed while `security-check` remains in `.PHONY`; Make therefore succeeds without checking whether `.env` is tracked or passwords are hard-coded.
Restore the `security-check` recipe or remove its callers and replace the checks with an equivalent active target.
</issue_to_address>
### Comment 4
<location path="xcore/kernel/api/auth.py" line_range="92-94" />
<code_context>
+ backend = get_auth_backend()
+ if backend is None:
+ return None
+ token = await backend.extract_token(request)
+ if token is None:
+ return None
+ return await backend.decode_token(token)
</code_context>
<issue_to_address>
**Backend errors abort anonymous calls**
When the registered backend raises while extracting or decoding a request token, `resolve_principal_from_request()` lets the backend exception escape, so the generic HTTP endpoint never reaches `supervisor.call()` and returns an error instead of dispatching anonymously.
Catch extraction and decoding errors in the resolver and return `None` so the call proceeds without a principal.
Also at `xcore/kernel/api/router.py:168`.
</issue_to_address>
### Comment 5
<location path="xcore/kernel/runtime/supervisor.py" line_range="348" />
<code_context>
+ # Même pattern que le ContextVar tenant : posé une fois ici pour l'appel
+ # en cours, lu par get_current_principal() dans le plugin — jamais de
+ # mutation d'un objet PluginContext partagé entre requêtes concurrentes.
+ principal_token = _current_principal.set(principal)
+ try:
+ # Les services du plugin sont déjà wrappés (TenantAwareDB/Cache) au
</code_context>
<issue_to_address>
**Sandboxed plugins lose caller identity**
When an authenticated call targets a sandboxed plugin, `PluginSupervisor._dispatch()` sets `_current_principal` in the supervisor process, but the sandbox worker receives only the action and payload. Its `get_current_principal()` therefore returns `None`, so sandboxed plugin code cannot access the caller identity.
Propagate the principal to the sandbox worker and set it in the worker’s context for the call.
</issue_to_address>
### Comment 6
<location path="makefile" line_range="50-52" />
<code_context>
@$(MAKE) clean
@poetry run python -m uvicorn main:app --reload --host 0.0.0.0 --port 8000
-st: ## Lancer en mode production (sans reload)
- @$(MAKE) clean
- @poetry run python -m uvicorn main:app --host 0.0.0.0 --port 8000
-
-
</code_context>
<issue_to_address>
**Documented server commands do nothing**
When an operator follows either documented command, `make st` and `make restart` are still advertised in `GEMINI.md` and `doc/cli/commands/health.md`, but their recipes are removed while the goals remain phony, so Make exits successfully without starting or restarting the server.
Update the documentation to use supported commands, or restore the removed Make targets.
Also at `makefile:248-252`.
</issue_to_address>
### Comment 7
<location path="makefile" line_range="64-66" />
<code_context>
# ============================================================
lint-check: ## Vérifier le style sans modifier (CI)
- @poetry run black xcore/ --check
+ @poetry run black xcore/ --check 2>/dev/null || true
@poetry run isort xcore/ --check-only
- @poetry run flake8 xcore/
+ @poetry run flake8 xcore/ || { echo "⚠️ flake8 warnings (non-fatales en CI)"; exit 0; }
lint-fix: ## Corriger automatiquement le style de xcore/ (black → isort → flake8 check)
</code_context>
<issue_to_address>
**Lint targets ignore style failures**
When black or Flake8 reports an error, `lint-check` and `ci-lint` suppress Black and Flake8 failures, so these targets return success even when formatting or lint checks fail; callers relying on them receive a false passing result.
Propagate the Black and Flake8 exit codes, or make these targets explicitly warning-only and provide a separate enforcing lint target.
Also at `makefile:121-123`.
</issue_to_address>
### Comment 8
<location path="xcore/sdk/decorators.py" line_range="75" />
<code_context>
+ Utilisable seul (sur n'importe quelle méthode) ou empilé avec @action — dans
+ ce dernier cas l'ordre n'importe pas, les deux décorateurs posent des
+ attributs distincts (_xcore_schema / _xcore_action) sur la même fonction.
+ @action accepte aussi directement ces mêmes paramètres pour l'usage fusionné.
+
Le format du dict `input` suit la convention pydantic create_model :
</code_context>
<issue_to_address>
**Documented decorator usage raises TypeError**
When a plugin author follows the `@schema` docstring and passes schema parameters directly to `@action`, `action()` accepts only `name` and `permissions`, so passing documented schema arguments such as `version=` raises `TypeError` while importing the plugin.
Make `action()` accept and apply the documented schema parameters, or revise the docstring to describe a supported usage.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 8 findings to address first, and a token can be decoded into, or an already-supplied principal can be propagated as, the caller identity exposed to plugin code; if that identity is wrong, downstream authorization or business actions may run for the wrong user. Reverting restores the previous behavior but cannot undo any data access or other side effects that occurred while the incorrect principal was in use.
Blocking findings: xcore/kernel/api/auth.py:78, xcore/kernel/runtime/supervisor.py:348, makefile:129, xcore/kernel/api/auth.py:94, xcore/kernel/runtime/supervisor.py:348, and 3 more
| Principal (sub/roles/permissions) résolu pour l'appel IPC en cours via | ||
| l'AuthBackend enregistré — None si aucun token n'a été transmis ou résolu. | ||
| """ | ||
| return _current_principal.get() |
Contributor
There was a problem hiding this comment.
🟡 Medium · Nested plugins trust altered identity
When plugin code mutates the current principal before making a nested call, get_current_principal() exposes a mutable AuthPayload, and TrustedBase.call_plugin() forwards the same object. Mutations alter the current identity and reach downstream plugins, allowing forged roles or permissions and cross-call interference.
Make principals immutable or return and forward defensive deep copies at both boundaries.
Also at xcore/kernel/api/contract.py:171, xcore/kernel/runtime/supervisor.py:348.
Prompt for AI agents
In `xcore/kernel/api/auth.py` at line 78:
**Nested plugins trust altered identity**
When plugin code mutates the current principal before making a nested call, `get_current_principal()` exposes a mutable `AuthPayload`, and `TrustedBase.call_plugin()` forwards the same object. Mutations alter the current identity and reach downstream plugins, allowing forged roles or permissions and cross-call interference.
Make principals immutable or return and forward defensive deep copies at both boundaries.
Also at `xcore/kernel/api/contract.py:171`, `xcore/kernel/runtime/supervisor.py:348`.| # Même pattern que le ContextVar tenant : posé une fois ici pour l'appel | ||
| # en cours, lu par get_current_principal() dans le plugin — jamais de | ||
| # mutation d'un objet PluginContext partagé entre requêtes concurrentes. | ||
| principal_token = _current_principal.set(principal) |
Contributor
There was a problem hiding this comment.
🟡 Medium · Background tasks retain caller identity
When a handler starts a detached task during an authenticated IPC call and that task later makes a nested plugin call, _dispatch() resets _current_principal only in its own task context; the detached task keeps its inherited copy. TrustedBase.call_plugin() then forwards that stale principal, so the downstream plugin sees the original user after the request ends.
Prevent detached tasks from retaining or forwarding the request principal after _dispatch() completes.
Also at xcore/kernel/api/contract.py:171.
Prompt for AI agents
In `xcore/kernel/runtime/supervisor.py` at line 348:
**Background tasks retain caller identity**
When a handler starts a detached task during an authenticated IPC call and that task later makes a nested plugin call, `_dispatch()` resets `_current_principal` only in its own task context; the detached task keeps its inherited copy. `TrustedBase.call_plugin()` then forwards that stale principal, so the downstream plugin sees the original user after the request ends.
Prevent detached tasks from retaining or forwarding the request principal after `_dispatch()` completes.
Also at `xcore/kernel/api/contract.py:171`.
Comment on lines
-122
to
-129
| security-check: ## Vérifications rapides (.env, mots de passe en dur) | ||
| @if git ls-files | grep -q "\.env$$"; then \ | ||
| echo "❌ .env est tracké par git !"; exit 1; \ | ||
| else echo "✅ .env correctement ignoré"; fi | ||
| @if grep -rn "password\s*=\s*[\"'][^\"']*[\"']" xcore/ 2>/dev/null; then \ | ||
| echo "❌ Mots de passe potentiels détectés !"; \ | ||
| else echo "✅ Aucun mot de passe en dur"; fi | ||
|
|
Contributor
There was a problem hiding this comment.
🟡 Medium · CI security check becomes a no-op
When the CI security job or ci-security target runs, ci-security and the GitHub security job still invoke make security-check, but the target's recipe is removed while security-check remains in .PHONY; Make therefore succeeds without checking whether .env is tracked or passwords are hard-coded.
Restore the security-check recipe or remove its callers and replace the checks with an equivalent active target.
Prompt for AI agents
In `makefile` at lines 122-129:
**CI security check becomes a no-op**
When the CI security job or `ci-security` target runs, `ci-security` and the GitHub security job still invoke `make security-check`, but the target's recipe is removed while `security-check` remains in `.PHONY`; Make therefore succeeds without checking whether `.env` is tracked or passwords are hard-coded.
Restore the `security-check` recipe or remove its callers and replace the checks with an equivalent active target.
Comment on lines
+92
to
+94
| token = await backend.extract_token(request) | ||
| if token is None: | ||
| return None |
Contributor
There was a problem hiding this comment.
🟠 High · Backend errors abort anonymous calls
When the registered backend raises while extracting or decoding a request token, resolve_principal_from_request() lets the backend exception escape, so the generic HTTP endpoint never reaches supervisor.call() and returns an error instead of dispatching anonymously.
Catch extraction and decoding errors in the resolver and return None so the call proceeds without a principal.
Also at xcore/kernel/api/router.py:168.
Prompt for AI agents
In `xcore/kernel/api/auth.py` at lines 92-94:
**Backend errors abort anonymous calls**
When the registered backend raises while extracting or decoding a request token, `resolve_principal_from_request()` lets the backend exception escape, so the generic HTTP endpoint never reaches `supervisor.call()` and returns an error instead of dispatching anonymously.
Catch extraction and decoding errors in the resolver and return `None` so the call proceeds without a principal.
Also at `xcore/kernel/api/router.py:168`.| # Même pattern que le ContextVar tenant : posé une fois ici pour l'appel | ||
| # en cours, lu par get_current_principal() dans le plugin — jamais de | ||
| # mutation d'un objet PluginContext partagé entre requêtes concurrentes. | ||
| principal_token = _current_principal.set(principal) |
Contributor
There was a problem hiding this comment.
🟡 Medium · Sandboxed plugins lose caller identity
When an authenticated call targets a sandboxed plugin, PluginSupervisor._dispatch() sets _current_principal in the supervisor process, but the sandbox worker receives only the action and payload. Its get_current_principal() therefore returns None, so sandboxed plugin code cannot access the caller identity.
Propagate the principal to the sandbox worker and set it in the worker’s context for the call.
Prompt for AI agents
In `xcore/kernel/runtime/supervisor.py` at line 348:
**Sandboxed plugins lose caller identity**
When an authenticated call targets a sandboxed plugin, `PluginSupervisor._dispatch()` sets `_current_principal` in the supervisor process, but the sandbox worker receives only the action and payload. Its `get_current_principal()` therefore returns `None`, so sandboxed plugin code cannot access the caller identity.
Propagate the principal to the sandbox worker and set it in the worker’s context for the call.
Comment on lines
-50
to
-52
| st: ## Lancer en mode production (sans reload) | ||
| @$(MAKE) clean | ||
| @poetry run python -m uvicorn main:app --host 0.0.0.0 --port 8000 |
Contributor
There was a problem hiding this comment.
🟡 Medium · Documented server commands do nothing
When an operator follows either documented command, make st and make restart are still advertised in GEMINI.md and doc/cli/commands/health.md, but their recipes are removed while the goals remain phony, so Make exits successfully without starting or restarting the server.
Update the documentation to use supported commands, or restore the removed Make targets.
Also at makefile:248-252.
Prompt for AI agents
In `makefile` at lines 50-52:
**Documented server commands do nothing**
When an operator follows either documented command, `make st` and `make restart` are still advertised in `GEMINI.md` and `doc/cli/commands/health.md`, but their recipes are removed while the goals remain phony, so Make exits successfully without starting or restarting the server.
Update the documentation to use supported commands, or restore the removed Make targets.
Also at `makefile:248-252`.
Comment on lines
+64
to
+66
| @poetry run black xcore/ --check 2>/dev/null || true | ||
| @poetry run isort xcore/ --check-only | ||
| @poetry run flake8 xcore/ | ||
| @poetry run flake8 xcore/ || { echo "⚠️ flake8 warnings (non-fatales en CI)"; exit 0; } |
Contributor
There was a problem hiding this comment.
🟡 Medium · Lint targets ignore style failures
When black or Flake8 reports an error, lint-check and ci-lint suppress Black and Flake8 failures, so these targets return success even when formatting or lint checks fail; callers relying on them receive a false passing result.
Propagate the Black and Flake8 exit codes, or make these targets explicitly warning-only and provide a separate enforcing lint target.
Also at makefile:121-123.
Prompt for AI agents
In `makefile` at lines 64-66:
**Lint targets ignore style failures**
When black or Flake8 reports an error, `lint-check` and `ci-lint` suppress Black and Flake8 failures, so these targets return success even when formatting or lint checks fail; callers relying on them receive a false passing result.
Propagate the Black and Flake8 exit codes, or make these targets explicitly warning-only and provide a separate enforcing lint target.
Also at `makefile:121-123`.| Utilisable seul (sur n'importe quelle méthode) ou empilé avec @action — dans | ||
| ce dernier cas l'ordre n'importe pas, les deux décorateurs posent des | ||
| attributs distincts (_xcore_schema / _xcore_action) sur la même fonction. | ||
| @action accepte aussi directement ces mêmes paramètres pour l'usage fusionné. |
Contributor
There was a problem hiding this comment.
🟡 Medium · Documented decorator usage raises TypeError
When a plugin author follows the @schema docstring and passes schema parameters directly to @action, action() accepts only name and permissions, so passing documented schema arguments such as version= raises TypeError while importing the plugin.
Make action() accept and apply the documented schema parameters, or revise the docstring to describe a supported usage.
Prompt for AI agents
In `xcore/sdk/decorators.py` at line 75:
**Documented decorator usage raises TypeError**
When a plugin author follows the `@schema` docstring and passes schema parameters directly to `@action`, `action()` accepts only `name` and `permissions`, so passing documented schema arguments such as `version=` raises `TypeError` while importing the plugin.
Make `action()` accept and apply the documented schema parameters, or revise the docstring to describe a supported usage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
supervisor.call()/TrustedBase.call_plugin()) can now resolve the calling user via the registeredAuthBackend, separately from the existing HTTP-route RBAC. NewAuthResolverMiddleware,get_current_principal(),resolve_principal_from_request()— resolution only, nothing is enforced yet.@action(name, permissions=None)— optional declarative permissions list, mirroring@route's existing param.@schemastays fully independent of@action(usable standalone or stacked, either order).xcdk>=0.2.0/xcorecli>=2.3.0instead of only the vendoredxcore/sdk/fallback; trimmedmakefiletargets superseded byxcorecli; dropped stale tracked report files underreports/.Test plan
poetry run pytest tests/unit tests/integration -q— 1387 passed, 1 pre-existing unrelated skippoetry run mkdocs build --strict— clean (pre-existing unrelated warnings only)make lint-check— passedSummary by Sourcery
Add best-effort authenticated-user resolution to IPC calls while preparing declarative per-action authorization and modernizing project dependencies and tooling.
New Features:
@actiondecorator while keeping it independent from@schema.xcdkandxcoreclipackages as runtime dependencies.Enhancements:
Build:
Documentation:
Chores: