[18.0][ADD] cetmix_tower_jet_isolation: base module add new module#486
[18.0][ADD] cetmix_tower_jet_isolation: base module add new module#486marionumza wants to merge 12 commits intocetmix:18.0from
Conversation
Introduces a strict isolation mode for Jet Templates to restrict command and flight plan execution. Key features: - Adds `isolation_mode` to `cx.tower.jet.template` to enforce applicability and tags. - Injects template restrictions directly into `cx.tower.command.run.wizard` and `cx.tower.plan.run.wizard` via `default_get`. - Visually locks the applicability and tag fields in the run wizards (readonly) to prevent users from bypassing isolation filters. - Reuses the existing `cx.tower.tag` model for seamless integration without modifying Odoo core domains. Signed-off-by: Mario Osvaldo Nuñez <marionumza@gmail.com>
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an Odoo addon that enables "isolation mode" for jets: new template fields, wizard subclasses that detect isolated jets and prefill/lock applicability/tags, XML view inheritances to enforce UI restrictions, package manifest/pyproject, uninstall hook, and README. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cetmix_tower_jet_isolation/__manifest__.py`:
- Around line 1-17: The manifest currently sets "author" to "Antigravity" and is
missing "website" and "maintainers", and the repo requires a README.rst; update
the manifest in __manifest__.py by replacing the "author" value with the correct
project author (e.g., "Cetmix" if that is intended), add a "website" key with
the project URL, and add a "maintainers" list with the appropriate usernames;
also add a top-level README.rst describing the module (short description,
install/usage notes) to satisfy pylint_odoo's missing-readme check and repo
conventions.
In `@cetmix_tower_jet_isolation/models/cx_tower_jet_template.py`:
- Around line 10-13: Add a server-side constraint to enforce that
forced_applicability is set when isolation_mode is True: in the model class
(e.g., CxTowerJetTemplate) add an `@api.constrains`('isolation_mode',
'forced_applicability') method that checks if record.isolation_mode and not
record.forced_applicability then raise ValidationError with a clear message;
ensure you import from odoo import api and from odoo.exceptions import
ValidationError so RPC writes trigger the same validation as the UI.
- Around line 10-13: Replace the hardcoded selection on forced_applicability
with a dynamic selection that reads the existing applicability selection from
the cx.tower.plan.run.wizard field to avoid duplication; update the
forced_applicability field definition (in the cx_tower_jet_template model) so
its selection is provided as a lambda that returns
self.env["cx.tower.plan.run.wizard"]._fields["applicability"].selection and keep
the string parameter (e.g., "Forced Applicability") unchanged.
- Around line 15-25: Update the Many2many definitions for forced_command_tag_ids
and forced_plan_tag_ids to explicitly declare column1 and column2 and rename the
relation tables to include "jet" to match repo conventions; specifically, for
the field forced_command_tag_ids (comodel_name='cx.tower.tag', relation
currently 'cx_tower_template_forced_command_tag_rel') add
column1='jet_template_id' and column2='tag_id' and rename the relation to
'cx_tower_jet_template_forced_command_tag_rel', and for forced_plan_tag_ids add
column1='jet_template_id' and column2='tag_id' and rename its relation to
'cx_tower_jet_template_forced_plan_tag_rel' so the DB columns and table name
match other cx.tower.tag Many2manys and avoid auto-generated fragile names.
- Around line 1-25: Update the model to follow repo style: change all single
quotes to double quotes, add a class docstring and the _description attribute on
class CxTowerJetTemplate describing the purpose, and ensure trailing commas on
multiline literals (e.g., the forced_applicability selection list and the
Many2many field definitions forced_command_tag_ids and forced_plan_tag_ids).
Specifically, add a top-of-class docstring string, set _description = "..."
under the class, replace string='...' occurrences on fields (isolation_mode,
forced_applicability, forced_command_tag_ids, forced_plan_tag_ids) with
double-quoted values, and ensure trailing commas remain on multi-line arguments
and lists.
In `@cetmix_tower_jet_isolation/views/cx_tower_jet_template_views.xml`:
- Around line 11-13: The fields forced_applicability, forced_command_tag_ids and
forced_plan_tag_ids remain populated when isolation_mode is turned off; add
logic to clear them when isolation_mode becomes False by implementing either an
onchange_isolation_mode handler or overriding write/create to set those fields
to False/empty when isolation_mode is False. Specifically, in the model
containing these fields implement an `@api.onchange`('isolation_mode') that clears
forced_applicability and empties forced_command_tag_ids / forced_plan_tag_ids
when isolation_mode is falsy, and/or add a write method that detects
isolation_mode being set to False and updates those same fields to
False/[(6,0,[])] (or empty) before calling super so stale values are removed
from the DB.
In `@cetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.py`:
- Around line 8-14: Replace the loop in _compute_is_isolated_context by
assigning the boolean expression directly to is_isolated_context and add a
dependency on the template field; specifically, change the compute to set
record.is_isolated_context = any(j.jet_template_id.id and
j.jet_template_id.isolation_mode for j in record.jet_ids) (this guards against
NewId/unsaved jet_template_id by checking its id) and update the `@api.depends` to
include 'jet_ids.jet_template_id.isolation_mode' so the flag recomputes when the
underlying template toggles.
- Around line 16-29: default_get currently only inspects
jet_ids[0].jet_template_id which can miss isolation info used by
_compute_is_isolated_context; update default_get to iterate all jets in jet_ids,
collect templates where template.isolation_mode is True, then apply a
deterministic rule (e.g., pick the first isolated template) or merge strictest
values: set res['applicability'] from the chosen template.forced_applicability
(if present) and res['tag_ids'] to the union of all chosen templates'
forced_command_tag_ids.ids (if any) so forced_applicability and
forced_command_tag_ids are applied whenever any selected jet enforces isolation;
reference variables/functions: default_get, jet_ids, template,
template.isolation_mode, template.forced_applicability,
template.forced_command_tag_ids, res['applicability'], res['tag_ids'].
In `@cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py`:
- Around line 16-29: default_get currently only pre-fills applicability/tag_ids
but does not enforce them server-side, so override create and write on the
wizard model (the same pattern should be applied to
cx_tower_command_run_wizard.py) to enforce the template's forced values: when
is_isolated_context is true (use the wizard model's field), fetch the related
cx.tower.jet -> jet_template_id and, if template.isolation_mode, set/replace
record values for applicability with template.forced_applicability and tag_ids
with template.forced_plan_tag_ids.ids (or refuse the operation) before saving;
alternatively add an `@api.constrains` that validates incoming
applicability/tag_ids against template.forced_* and raises an error if they
diverge.
- Around line 19-22: The current default_get uses only the first selected jet's
template (jet_ids[0].jet_template_id), which is inconsistent with
_compute_is_isolated_context (which aggregates across jets) and silently ignores
mixed templates; update default_get to either (a) enforce that all selected jets
(self.env.context['default_jet_ids']) share the same jet_template_id and raise a
UserError if they do not, or (b) compute the union/strictest combination of
forced tags/applicability across all jet_ids and derive defaults from that
union; ensure you reference and align behavior with _compute_is_isolated_context
so the default flag semantics match the applied values (use symbols:
default_jet_ids, jet_ids, jet_template_id, _compute_is_isolated_context,
default_get) and raise UserError when choosing the enforcement option.
- Around line 18-28: The override in default_get unconditionally applies
template.forced_applicability and can widen the base wizard's non-privileged
restriction (res['applicability']) which is a privilege-escalation risk; modify
default_get so that before assigning res['applicability'] =
template.forced_applicability you compare the existing res.get('applicability')
and only change it when the new value does not relax access for the current user
(e.g., only set to a broader scope like 'shared' if the current user is
privileged via an explicit check such as self.env.user.has_group(...) or another
project-specific privilege helper), otherwise leave res['applicability'] as
returned by super(); keep the existing logic for tag population
(template.forced_plan_tag_ids -> res['tag_ids']) unchanged.
- Around line 1-14: Update style and naming: reorder and use imports as "from
odoo import api, fields, models" and switch string quotes to double quotes;
rename class CxTowerPlanRunWizardFilter to CxTowerPlanRunWizard to reflect that
it extends the existing model (keep _inherit='cx.tower.plan.run.wizard'); and
simplify the compute method _compute_is_isolated_context to assign the boolean
directly (e.g., record.is_isolated_context = bool(record.jet_ids and
any(j.jet_template_id.isolation_mode for j in record.jet_ids))) while keeping
the field name is_isolated_context and the `@api.depends`('jet_ids') decorator.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 303e7003-642a-4f92-9cb5-76957fbccd3c
📒 Files selected for processing (10)
cetmix_tower_jet_isolation/__init__.pycetmix_tower_jet_isolation/__manifest__.pycetmix_tower_jet_isolation/models/__init__.pycetmix_tower_jet_isolation/models/cx_tower_jet_template.pycetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xmlcetmix_tower_jet_isolation/views/cx_tower_jet_template_views.xmlcetmix_tower_jet_isolation/views/cx_tower_plan_run_wizard_views.xmlcetmix_tower_jet_isolation/wizards/__init__.pycetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.pycetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py
| from odoo import models, fields | ||
|
|
||
| class CxTowerJetTemplate(models.Model): | ||
| _inherit = 'cx.tower.jet.template' | ||
|
|
||
| isolation_mode = fields.Boolean( | ||
| string='Isolation Mode', | ||
| help='When active, prevents users from changing applicability or tags when running commands/plans.' | ||
| ) | ||
| forced_applicability = fields.Selection([ | ||
| ('this', 'For selected server(s)'), | ||
| ('shared', 'Non server restricted') | ||
| ], string='Forced Applicability') | ||
|
|
||
| forced_command_tag_ids = fields.Many2many( | ||
| comodel_name='cx.tower.tag', | ||
| relation='cx_tower_template_forced_command_tag_rel', | ||
| string='Forced Command Tags', | ||
| ) | ||
|
|
||
| forced_plan_tag_ids = fields.Many2many( | ||
| comodel_name='cx.tower.tag', | ||
| relation='cx_tower_template_forced_plan_tag_rel', | ||
| string='Forced Flight Plan Tags', | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Missing module/class docstring and style inconsistencies.
This file uses single quotes and omits a class docstring, diverging from the _description/docstring conventions in cetmix_tower_server/models/cx_tower_jet_template.py. Please align with repo style (double quotes, docstring, trailing commas on multiline literals).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cetmix_tower_jet_isolation/models/cx_tower_jet_template.py` around lines 1 -
25, Update the model to follow repo style: change all single quotes to double
quotes, add a class docstring and the _description attribute on class
CxTowerJetTemplate describing the purpose, and ensure trailing commas on
multiline literals (e.g., the forced_applicability selection list and the
Many2many field definitions forced_command_tag_ids and forced_plan_tag_ids).
Specifically, add a top-of-class docstring string, set _description = "..."
under the class, replace string='...' occurrences on fields (isolation_mode,
forced_applicability, forced_command_tag_ids, forced_plan_tag_ids) with
double-quoted values, and ensure trailing commas remain on multi-line arguments
and lists.
| forced_applicability = fields.Selection([ | ||
| ('this', 'For selected server(s)'), | ||
| ('shared', 'Non server restricted') | ||
| ], string='Forced Applicability') |
There was a problem hiding this comment.
Enforce forced_applicability presence when isolation_mode=True on the server.
The XML view declares required="isolation_mode", but this is only a UI constraint — RPC writes can leave forced_applicability empty while isolation_mode=True, which makes the wizard's if template.forced_applicability: silently skip enforcement. Add a Python @api.constrains('isolation_mode', 'forced_applicability') (or a SQL/DB-level check) to make the rule authoritative.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cetmix_tower_jet_isolation/models/cx_tower_jet_template.py` around lines 10 -
13, Add a server-side constraint to enforce that forced_applicability is set
when isolation_mode is True: in the model class (e.g., CxTowerJetTemplate) add
an `@api.constrains`('isolation_mode', 'forced_applicability') method that checks
if record.isolation_mode and not record.forced_applicability then raise
ValidationError with a clear message; ensure you import from odoo import api and
from odoo.exceptions import ValidationError so RPC writes trigger the same
validation as the UI.
🧹 Nitpick | 🔵 Trivial
Avoid duplicating the applicability selection values.
forced_applicability hardcodes [('this', ...), ('shared', ...)], duplicating the selection on cx.tower.plan.run.wizard.applicability. If the base selection is ever extended (e.g., a new option), these will drift. Prefer deriving the selection from the base field, e.g.:
forced_applicability = fields.Selection(
selection=lambda self: self.env["cx.tower.plan.run.wizard"]._fields["applicability"].selection,
string="Forced Applicability",
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cetmix_tower_jet_isolation/models/cx_tower_jet_template.py` around lines 10 -
13, Replace the hardcoded selection on forced_applicability with a dynamic
selection that reads the existing applicability selection from the
cx.tower.plan.run.wizard field to avoid duplication; update the
forced_applicability field definition (in the cx_tower_jet_template model) so
its selection is provided as a lambda that returns
self.env["cx.tower.plan.run.wizard"]._fields["applicability"].selection and keep
the string parameter (e.g., "Forced Applicability") unchanged.
| forced_command_tag_ids = fields.Many2many( | ||
| comodel_name='cx.tower.tag', | ||
| relation='cx_tower_template_forced_command_tag_rel', | ||
| string='Forced Command Tags', | ||
| ) | ||
|
|
||
| forced_plan_tag_ids = fields.Many2many( | ||
| comodel_name='cx.tower.tag', | ||
| relation='cx_tower_template_forced_plan_tag_rel', | ||
| string='Forced Flight Plan Tags', | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Specify column1/column2 for the Many2many relations and follow repo naming.
Other cx.tower.tag Many2many relations in cetmix_tower_server/models/cx_tower_tag.py declare explicit column1/column2 (e.g., tag_id / <target>_id). Omitting them lets Odoo auto-generate columns from field names, which is fragile across renames. Also, the relation table name cx_tower_template_forced_command_tag_rel elides the "jet" segment — consider cx_tower_jet_template_forced_command_tag_rel for clarity and to avoid collisions with other template models.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cetmix_tower_jet_isolation/models/cx_tower_jet_template.py` around lines 15 -
25, Update the Many2many definitions for forced_command_tag_ids and
forced_plan_tag_ids to explicitly declare column1 and column2 and rename the
relation tables to include "jet" to match repo conventions; specifically, for
the field forced_command_tag_ids (comodel_name='cx.tower.tag', relation
currently 'cx_tower_template_forced_command_tag_rel') add
column1='jet_template_id' and column2='tag_id' and rename the relation to
'cx_tower_jet_template_forced_command_tag_rel', and for forced_plan_tag_ids add
column1='jet_template_id' and column2='tag_id' and rename its relation to
'cx_tower_jet_template_forced_plan_tag_rel' so the DB columns and table name
match other cx.tower.tag Many2manys and avoid auto-generated fragile names.
| <field name="forced_applicability" invisible="not isolation_mode" required="isolation_mode"/> | ||
| <field name="forced_command_tag_ids" widget="many2many_tags" invisible="not isolation_mode"/> | ||
| <field name="forced_plan_tag_ids" widget="many2many_tags" invisible="not isolation_mode"/> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clear forced values when isolation is disabled.
forced_applicability, forced_command_tag_ids, and forced_plan_tag_ids are only hidden when isolation_mode is false, but stale values persist in the database. Consider either clearing them in a Python write/onchange override or relying on them only while isolation_mode=True in the wizards (the current wizards already do the latter, so this is mainly UI hygiene).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cetmix_tower_jet_isolation/views/cx_tower_jet_template_views.xml` around
lines 11 - 13, The fields forced_applicability, forced_command_tag_ids and
forced_plan_tag_ids remain populated when isolation_mode is turned off; add
logic to clear them when isolation_mode becomes False by implementing either an
onchange_isolation_mode handler or overriding write/create to set those fields
to False/empty when isolation_mode is False. Specifically, in the model
containing these fields implement an `@api.onchange`('isolation_mode') that clears
forced_applicability and empties forced_command_tag_ids / forced_plan_tag_ids
when isolation_mode is falsy, and/or add a write method that detects
isolation_mode being set to False and updates those same fields to
False/[(6,0,[])] (or empty) before calling super so stale values are removed
from the DB.
| @api.model | ||
| def default_get(self, fields_list): | ||
| res = super().default_get(fields_list) | ||
| if 'default_jet_ids' in self.env.context: | ||
| jet_ids = self.env['cx.tower.jet'].browse(self.env.context['default_jet_ids']) | ||
| if jet_ids: | ||
| template = jet_ids[0].jet_template_id | ||
|
|
||
| if template.isolation_mode: | ||
| if template.forced_applicability: | ||
| res['applicability'] = template.forced_applicability | ||
| if template.forced_command_tag_ids: | ||
| res['tag_ids'] = [(6, 0, template.forced_command_tag_ids.ids)] | ||
| return res |
There was a problem hiding this comment.
Inconsistency between _compute_is_isolated_context and default_get — isolation can be bypassed.
_compute_is_isolated_context activates when any selected jet's template has isolation_mode enabled (Line 11), but default_get only inspects jet_ids[0].jet_template_id (Line 22). If the first selected jet's template has no isolation, but a later one does, the wizard enters isolated (read-only) context without the forced applicability/tag_ids being applied — defeating the isolation guarantees described in the PR objectives.
Iterate over all jets and apply the strictest forced values, or otherwise define a deterministic selection rule (e.g., the first isolated template).
🛠️ Proposed fix
- if 'default_jet_ids' in self.env.context:
- jet_ids = self.env['cx.tower.jet'].browse(self.env.context['default_jet_ids'])
- if jet_ids:
- template = jet_ids[0].jet_template_id
-
- if template.isolation_mode:
- if template.forced_applicability:
- res['applicability'] = template.forced_applicability
- if template.forced_command_tag_ids:
- res['tag_ids'] = [(6, 0, template.forced_command_tag_ids.ids)]
+ jet_ids_ctx = self.env.context.get('default_jet_ids')
+ if jet_ids_ctx:
+ jets = self.env['cx.tower.jet'].browse(jet_ids_ctx)
+ isolated_templates = jets.jet_template_id.filtered('isolation_mode')
+ if isolated_templates:
+ template = isolated_templates[0]
+ if template.forced_applicability:
+ res['applicability'] = template.forced_applicability
+ forced_tags = isolated_templates.mapped('forced_command_tag_ids')
+ if forced_tags:
+ res['tag_ids'] = [(6, 0, forced_tags.ids)]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.py` around
lines 16 - 29, default_get currently only inspects jet_ids[0].jet_template_id
which can miss isolation info used by _compute_is_isolated_context; update
default_get to iterate all jets in jet_ids, collect templates where
template.isolation_mode is True, then apply a deterministic rule (e.g., pick the
first isolated template) or merge strictest values: set res['applicability']
from the chosen template.forced_applicability (if present) and res['tag_ids'] to
the union of all chosen templates' forced_command_tag_ids.ids (if any) so
forced_applicability and forced_command_tag_ids are applied whenever any
selected jet enforces isolation; reference variables/functions: default_get,
jet_ids, template, template.isolation_mode, template.forced_applicability,
template.forced_command_tag_ids, res['applicability'], res['tag_ids'].
| from odoo import models, fields, api | ||
|
|
||
| class CxTowerPlanRunWizardFilter(models.TransientModel): | ||
| _inherit = 'cx.tower.plan.run.wizard' | ||
|
|
||
| is_isolated_context = fields.Boolean(compute='_compute_is_isolated_context') | ||
|
|
||
| @api.depends('jet_ids') | ||
| def _compute_is_isolated_context(self): | ||
| for record in self: | ||
| if record.jet_ids and any(j.jet_template_id.isolation_mode for j in record.jet_ids): | ||
| record.is_isolated_context = True | ||
| else: | ||
| record.is_isolated_context = False |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Style/consistency nits.
- The rest of the repo uses double quotes and
from odoo import api, fields, modelsordering; this file uses single quotes and a different import order. - Class name
CxTowerPlanRunWizardFilteris misleading since it is not a filter but an extension — considerCxTowerPlanRunWizard(matching the base class name) for consistency with other_inheritextensions in the codebase. - The compute can be simplified:
Proposed simplification
- `@api.depends`('jet_ids')
- def _compute_is_isolated_context(self):
- for record in self:
- if record.jet_ids and any(j.jet_template_id.isolation_mode for j in record.jet_ids):
- record.is_isolated_context = True
- else:
- record.is_isolated_context = False
+ `@api.depends`("jet_ids.jet_template_id.isolation_mode")
+ def _compute_is_isolated_context(self):
+ for record in self:
+ record.is_isolated_context = any(
+ record.jet_ids.mapped("jet_template_id.isolation_mode")
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from odoo import models, fields, api | |
| class CxTowerPlanRunWizardFilter(models.TransientModel): | |
| _inherit = 'cx.tower.plan.run.wizard' | |
| is_isolated_context = fields.Boolean(compute='_compute_is_isolated_context') | |
| @api.depends('jet_ids') | |
| def _compute_is_isolated_context(self): | |
| for record in self: | |
| if record.jet_ids and any(j.jet_template_id.isolation_mode for j in record.jet_ids): | |
| record.is_isolated_context = True | |
| else: | |
| record.is_isolated_context = False | |
| from odoo import models, fields, api | |
| class CxTowerPlanRunWizardFilter(models.TransientModel): | |
| _inherit = 'cx.tower.plan.run.wizard' | |
| is_isolated_context = fields.Boolean(compute='_compute_is_isolated_context') | |
| `@api.depends`("jet_ids.jet_template_id.isolation_mode") | |
| def _compute_is_isolated_context(self): | |
| for record in self: | |
| record.is_isolated_context = any( | |
| record.jet_ids.mapped("jet_template_id.isolation_mode") | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py` around lines
1 - 14, Update style and naming: reorder and use imports as "from odoo import
api, fields, models" and switch string quotes to double quotes; rename class
CxTowerPlanRunWizardFilter to CxTowerPlanRunWizard to reflect that it extends
the existing model (keep _inherit='cx.tower.plan.run.wizard'); and simplify the
compute method _compute_is_isolated_context to assign the boolean directly
(e.g., record.is_isolated_context = bool(record.jet_ids and
any(j.jet_template_id.isolation_mode for j in record.jet_ids))) while keeping
the field name is_isolated_context and the `@api.depends`('jet_ids') decorator.
| @api.model | ||
| def default_get(self, fields_list): | ||
| res = super().default_get(fields_list) | ||
| if 'default_jet_ids' in self.env.context: | ||
| jet_ids = self.env['cx.tower.jet'].browse(self.env.context['default_jet_ids']) | ||
| if jet_ids: | ||
| template = jet_ids[0].jet_template_id | ||
|
|
||
| if template.isolation_mode: | ||
| if template.forced_applicability: | ||
| res['applicability'] = template.forced_applicability | ||
| if template.forced_plan_tag_ids: | ||
| res['tag_ids'] = [(6, 0, template.forced_plan_tag_ids.ids)] | ||
| return res |
There was a problem hiding this comment.
Server-side enforcement is missing — isolation is bypassable.
default_get only prefills applicability/tag_ids on form open; the view's readonly + force_save="1" is a client-side restriction. A user can still submit arbitrary values via RPC (e.g., create/write/direct button call) and bypass the isolation filter entirely, defeating the stated security goal of this module.
Enforce the isolation constraints on the server side by overriding create/write (or the action that launches the plan) to re-apply forced_applicability and forced_plan_tag_ids whenever is_isolated_context is true, or validate them in a @api.constrains. The same concern applies symmetrically in cetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.py.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py` around lines
16 - 29, default_get currently only pre-fills applicability/tag_ids but does not
enforce them server-side, so override create and write on the wizard model (the
same pattern should be applied to cx_tower_command_run_wizard.py) to enforce the
template's forced values: when is_isolated_context is true (use the wizard
model's field), fetch the related cx.tower.jet -> jet_template_id and, if
template.isolation_mode, set/replace record values for applicability with
template.forced_applicability and tag_ids with template.forced_plan_tag_ids.ids
(or refuse the operation) before saving; alternatively add an `@api.constrains`
that validates incoming applicability/tag_ids against template.forced_* and
raises an error if they diverge.
| res = super().default_get(fields_list) | ||
| if 'default_jet_ids' in self.env.context: | ||
| jet_ids = self.env['cx.tower.jet'].browse(self.env.context['default_jet_ids']) | ||
| if jet_ids: | ||
| template = jet_ids[0].jet_template_id | ||
|
|
||
| if template.isolation_mode: | ||
| if template.forced_applicability: | ||
| res['applicability'] = template.forced_applicability | ||
| if template.forced_plan_tag_ids: | ||
| res['tag_ids'] = [(6, 0, template.forced_plan_tag_ids.ids)] |
There was a problem hiding this comment.
Privilege-escalation risk: isolation may override the base wizard's non-privileged applicability='this' restriction.
The base cx.tower.plan.run.wizard.default_get forces applicability='this' for non-privileged users. This override runs after super() and unconditionally writes res['applicability'] = template.forced_applicability, so a template configured with forced_applicability='shared' effectively grants non-privileged users the ability to run non-server-restricted plans — the opposite of the isolation intent.
Consider respecting the base restriction (e.g., only widen to 'shared' when the user is privileged, or intersect with the base result).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py` around lines
18 - 28, The override in default_get unconditionally applies
template.forced_applicability and can widen the base wizard's non-privileged
restriction (res['applicability']) which is a privilege-escalation risk; modify
default_get so that before assigning res['applicability'] =
template.forced_applicability you compare the existing res.get('applicability')
and only change it when the new value does not relax access for the current user
(e.g., only set to a broader scope like 'shared' if the current user is
privileged via an explicit check such as self.env.user.has_group(...) or another
project-specific privilege helper), otherwise leave res['applicability'] as
returned by super(); keep the existing logic for tag population
(template.forced_plan_tag_ids -> res['tag_ids']) unchanged.
| if 'default_jet_ids' in self.env.context: | ||
| jet_ids = self.env['cx.tower.jet'].browse(self.env.context['default_jet_ids']) | ||
| if jet_ids: | ||
| template = jet_ids[0].jet_template_id |
There was a problem hiding this comment.
Using only the first jet's template is fragile.
jet_ids[0].jet_template_id silently ignores other jets. If the selection mixes jets from different templates (some with isolation, some without), the user either gets partial enforcement or none. Either enforce that all selected jets share the same isolation template (raise UserError otherwise) or iterate and apply the strictest union of forced tags/applicability.
Also, _compute_is_isolated_context uses any(...) across jets while default_get uses only the first — inconsistent semantics between the flag and the applied values.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py` around lines
19 - 22, The current default_get uses only the first selected jet's template
(jet_ids[0].jet_template_id), which is inconsistent with
_compute_is_isolated_context (which aggregates across jets) and silently ignores
mixed templates; update default_get to either (a) enforce that all selected jets
(self.env.context['default_jet_ids']) share the same jet_template_id and raise a
UserError if they do not, or (b) compute the union/strictest combination of
forced tags/applicability across all jet_ids and derive defaults from that
union; ensure you reference and align behavior with _compute_is_isolated_context
so the default flag semantics match the applied values (use symbols:
default_jet_ids, jet_ids, jet_template_id, _compute_is_isolated_context,
default_get) and raise UserError when choosing the enforcement option.
marionumza
changed the title
… and trailing whitespaces
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
cetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.py (1)
29-31:⚠️ Potential issue | 🟠 MajorForced
applicabilitycan bypass non-privileged-user restriction.The parent
default_getforcesres["applicability"] = "this"for non-privileged users (seecetmix_tower_server/wizards/cx_tower_command_run_wizard.pylines 122–127). This override unconditionally replaces it withtemplate.forced_applicability, which may be"shared", granting a non-privileged user broader scope than the base module permits. Consider preserving the privilege cap:🔧 Proposed fix
if template.isolation_mode: - if template.forced_applicability: + if template.forced_applicability and self._is_privileged_user(): res["applicability"] = template.forced_applicability🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.py` around lines 29 - 31, The code unconditionally replaces res["applicability"] with template.forced_applicability, which can escalate a non-privileged default ("this") to a broader scope; change the logic in default_get so that when template.isolation_mode and template.forced_applicability exist you only assign res["applicability"] = template.forced_applicability if the current res.get("applicability") is not the non-privileged value ("this") (or otherwise ensure forced_applicability is not broader than the existing value); reference the default_get flow, the template.isolation_mode / template.forced_applicability check and the res["applicability"] assignment to implement this conditional guard.cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py (4)
9-17: 🧹 Nitpick | 🔵 TrivialSimplify the compute and tighten dependencies.
Depend on the actual leaf (
jet_ids.jet_template_id.isolation_mode) so the flag recomputes when the template flag changes, and collapse the branch.♻️ Proposed refactor
- `@api.depends`("jet_ids") - def _compute_is_isolated_context(self): - for record in self: - if record.jet_ids and any( - j.jet_template_id.isolation_mode for j in record.jet_ids - ): - record.is_isolated_context = True - else: - record.is_isolated_context = False + `@api.depends`("jet_ids.jet_template_id.isolation_mode") + def _compute_is_isolated_context(self): + for record in self: + record.is_isolated_context = any( + record.jet_ids.mapped("jet_template_id.isolation_mode") + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py` around lines 9 - 17, Change the compute decorator to depend on the leaf field "jet_ids.jet_template_id.isolation_mode" and simplify _compute_is_isolated_context to assign the boolean expression directly; e.g., in _compute_is_isolated_context set record.is_isolated_context = any(j.jet_template_id.isolation_mode for j in record.jet_ids) for each record so it recomputes when the template isolation_mode changes and removes the explicit if/else branch (references: _compute_is_isolated_context, is_isolated_context, jet_ids.jet_template_id.isolation_mode).
19-34:⚠️ Potential issue | 🔴 CriticalServer-side enforcement is missing — isolation is bypassable via RPC.
default_getonly prefills values on form open; view-levelreadonly/force_saveis purely client-side. A user can submit arbitraryapplicability/tag_idsvia RPCcreate/writeor by invoking the run action directly, defeating the isolation goal. Enforce the forced values server-side increate/write(or the launch action), or add an@api.constrainsthat rejects divergence fromforced_applicability/forced_plan_tag_idswhenis_isolated_contextis true. The same applies tocx_tower_command_run_wizard.py.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py` around lines 19 - 34, default_get only pre-fills isolation fields on the client, so isolation can be bypassed via RPC; enforce the forced values server-side by applying them in the model's create and write paths (or the run/launch action) for cx.tower.plan.run.wizard: in create() and write() check the related template (jet_template_id from the wizard or the jet(s) on launch) and when template.isolation_mode is set override/force applicability and tag_ids to template.forced_applicability and template.forced_plan_tag_ids.ids respectively (or raise an exception if they differ). Do the same symmetric enforcement for the analogous model/handlers in cx_tower_command_run_wizard.py so client-side fields cannot be submitted with divergent values.
29-31:⚠️ Potential issue | 🟠 MajorPrivilege-escalation risk: overrides the base wizard's non-privileged
applicability='this'restriction.The parent
default_get(seecetmix_tower_server/wizards/cx_tower_plan_run_wizard.py:78-83) forcesapplicability='this'for non-privileged users. This override runs aftersuper()and unconditionally assignstemplate.forced_applicability, so a template configured withforced_applicability='shared'grants non-privileged users broader access than the base enforces — the inverse of the isolation intent. Only widen applicability when the current user passes the same privilege check used by the base, or intersect withres['applicability']rather than overwriting it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py` around lines 29 - 31, The current change in default_get unconditionally sets res["applicability"] = template.forced_applicability when template.isolation_mode is true, which can widen access for non-privileged users; modify the logic in cx_tower_plan_run_wizard.py's default_get so you only apply template.forced_applicability when the caller passes the same privilege check used by the parent default_get (i.e., the non-privileged check from the parent method), or otherwise compute the intersection between the existing res["applicability"] and template.forced_applicability rather than overwriting it; reference template.isolation_mode, template.forced_applicability and res["applicability"] to locate and update the conditional.
26-33:⚠️ Potential issue | 🟡 MinorInconsistent semantics: using only the first jet's template.
_compute_is_isolated_contextaggregates across all jets (any(...)), butdefault_gettakes onlyjet_ids[0].jet_template_id. Mixed selections silently get partial enforcement (or none if the first jet happens to be non-isolated while others are). Either raiseUserErrorwhen selected jets span different isolation templates, or iterate all jets and apply the strictest union offorced_applicability/forced_plan_tag_idsso the flag and the applied values stay in sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py` around lines 26 - 33, default_get currently only inspects jet_ids[0].jet_template_id which causes inconsistent behavior with _compute_is_isolated_context; update default_get to iterate all provided jet_ids: gather each jet_template_id, if multiple distinct templates have differing isolation rules then either raise a UserError (preferred) or else compute the strict/combined values by unioning forced_plan_tag_ids.ids and merging forced_applicability values across templates and set res["applicability"] and res["tag_ids"] accordingly; reference the symbols jet_ids, jet_template_id, template.isolation_mode, template.forced_applicability, template.forced_plan_tag_ids and the methods _compute_is_isolated_context and default_get when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xml`:
- Around line 15-18: The override of the applicability field replaced the
original readonly condition and removed the parent condition show_servers;
update the readonly attribute on the applicability field (the xpath targeting
//field[`@name`='applicability']) to combine both conditions instead of replacing
them—e.g. set readonly to an expression that ORs the two flags like
"show_servers or is_isolated_context" so the field remains readonly when either
condition applies, and keep the existing force_save="1" attribute.
In `@cetmix_tower_jet_isolation/views/cx_tower_plan_run_wizard_views.xml`:
- Around line 15-18: The xpath that sets attributes on the field applicability
currently replaces the parent readonly condition; update the readonly attribute
to combine both guards by setting readonly="show_servers or is_isolated_context"
(keep the existing force_save attribute unchanged) so the field honors both the
parent show_servers and the local is_isolated_context conditions.
---
Duplicate comments:
In `@cetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.py`:
- Around line 29-31: The code unconditionally replaces res["applicability"] with
template.forced_applicability, which can escalate a non-privileged default
("this") to a broader scope; change the logic in default_get so that when
template.isolation_mode and template.forced_applicability exist you only assign
res["applicability"] = template.forced_applicability if the current
res.get("applicability") is not the non-privileged value ("this") (or otherwise
ensure forced_applicability is not broader than the existing value); reference
the default_get flow, the template.isolation_mode /
template.forced_applicability check and the res["applicability"] assignment to
implement this conditional guard.
In `@cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py`:
- Around line 9-17: Change the compute decorator to depend on the leaf field
"jet_ids.jet_template_id.isolation_mode" and simplify
_compute_is_isolated_context to assign the boolean expression directly; e.g., in
_compute_is_isolated_context set record.is_isolated_context =
any(j.jet_template_id.isolation_mode for j in record.jet_ids) for each record so
it recomputes when the template isolation_mode changes and removes the explicit
if/else branch (references: _compute_is_isolated_context, is_isolated_context,
jet_ids.jet_template_id.isolation_mode).
- Around line 19-34: default_get only pre-fills isolation fields on the client,
so isolation can be bypassed via RPC; enforce the forced values server-side by
applying them in the model's create and write paths (or the run/launch action)
for cx.tower.plan.run.wizard: in create() and write() check the related template
(jet_template_id from the wizard or the jet(s) on launch) and when
template.isolation_mode is set override/force applicability and tag_ids to
template.forced_applicability and template.forced_plan_tag_ids.ids respectively
(or raise an exception if they differ). Do the same symmetric enforcement for
the analogous model/handlers in cx_tower_command_run_wizard.py so client-side
fields cannot be submitted with divergent values.
- Around line 29-31: The current change in default_get unconditionally sets
res["applicability"] = template.forced_applicability when
template.isolation_mode is true, which can widen access for non-privileged
users; modify the logic in cx_tower_plan_run_wizard.py's default_get so you only
apply template.forced_applicability when the caller passes the same privilege
check used by the parent default_get (i.e., the non-privileged check from the
parent method), or otherwise compute the intersection between the existing
res["applicability"] and template.forced_applicability rather than overwriting
it; reference template.isolation_mode, template.forced_applicability and
res["applicability"] to locate and update the conditional.
- Around line 26-33: default_get currently only inspects
jet_ids[0].jet_template_id which causes inconsistent behavior with
_compute_is_isolated_context; update default_get to iterate all provided
jet_ids: gather each jet_template_id, if multiple distinct templates have
differing isolation rules then either raise a UserError (preferred) or else
compute the strict/combined values by unioning forced_plan_tag_ids.ids and
merging forced_applicability values across templates and set
res["applicability"] and res["tag_ids"] accordingly; reference the symbols
jet_ids, jet_template_id, template.isolation_mode,
template.forced_applicability, template.forced_plan_tag_ids and the methods
_compute_is_isolated_context and default_get when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 78939af7-2d2b-47c0-aba5-0e047b3cd332
📒 Files selected for processing (9)
cetmix_tower_jet_isolation/README.rstcetmix_tower_jet_isolation/__manifest__.pycetmix_tower_jet_isolation/models/cx_tower_jet_template.pycetmix_tower_jet_isolation/pyproject.tomlcetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xmlcetmix_tower_jet_isolation/views/cx_tower_jet_template_views.xmlcetmix_tower_jet_isolation/views/cx_tower_plan_run_wizard_views.xmlcetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.pycetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py
| <xpath expr="//field[@name='applicability']" position="attributes"> | ||
| <attribute name="readonly">is_isolated_context</attribute> | ||
| <attribute name="force_save">1</attribute> | ||
| </xpath> |
There was a problem hiding this comment.
Overriding readonly drops existing show_servers condition on applicability.
The parent view sets readonly="show_servers" on applicability. Replacing the attribute with readonly="is_isolated_context" silently removes the original condition, so applicability will become editable again in contexts where it previously wasn't (e.g., after servers are computed). Combine both conditions.
🔧 Proposed fix
<xpath expr="//field[`@name`='applicability']" position="attributes">
- <attribute name="readonly">is_isolated_context</attribute>
+ <attribute name="readonly">is_isolated_context or show_servers</attribute>
<attribute name="force_save">1</attribute>
</xpath>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <xpath expr="//field[@name='applicability']" position="attributes"> | |
| <attribute name="readonly">is_isolated_context</attribute> | |
| <attribute name="force_save">1</attribute> | |
| </xpath> | |
| <xpath expr="//field[`@name`='applicability']" position="attributes"> | |
| <attribute name="readonly">is_isolated_context or show_servers</attribute> | |
| <attribute name="force_save">1</attribute> | |
| </xpath> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xml`
around lines 15 - 18, The override of the applicability field replaced the
original readonly condition and removed the parent condition show_servers;
update the readonly attribute on the applicability field (the xpath targeting
//field[`@name`='applicability']) to combine both conditions instead of replacing
them—e.g. set readonly to an expression that ORs the two flags like
"show_servers or is_isolated_context" so the field remains readonly when either
condition applies, and keep the existing force_save="1" attribute.
| <xpath expr="//field[@name='applicability']" position="attributes"> | ||
| <attribute name="readonly">is_isolated_context</attribute> | ||
| <attribute name="force_save">1</attribute> | ||
| </xpath> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'cx_tower_plan_run_wizard_view.xml' | xargs -I{} rg -n -C2 "name=['\"]applicability['\"]" {}Repository: cetmix/cetmix-tower
Length of output: 299
🏁 Script executed:
# Find the parent view by examining the file and locating the parent reference
fd -t f '*.xml' -p 'cetmix_tower_jet_isolation/views/*' | head -20Repository: cetmix/cetmix-tower
Length of output: 184
🏁 Script executed:
# Search for the parent view name referenced in the file being reviewed
rg -A5 -B5 "cx_tower_plan_run_wizard_view" cetmix_tower_jet_isolation/views/cx_tower_plan_run_wizard_views.xml | head -40Repository: cetmix/cetmix-tower
Length of output: 658
🏁 Script executed:
# Look for any parent view definition files that might contain applicability field
fd -t f '*.xml' cetmix_tower_jet_isolation/views/ | xargs grep -l "applicability" | head -10Repository: cetmix/cetmix-tower
Length of output: 347
🏁 Script executed:
# List all XML files to understand the structure
find . -name "*.xml" -type f | grep -E "(cetmix_tower_server|cetmix_tower_jet)" | head -20Repository: cetmix/cetmix-tower
Length of output: 1292
🏁 Script executed:
# Search for the parent view definition across the entire repo
rg -l "cx_tower_plan_run_wizard_view_form" --type xmlRepository: cetmix/cetmix-tower
Length of output: 192
🏁 Script executed:
# Search for applicability field with readonly attribute
rg "readonly.*applicability|applicability.*readonly" --type xml -A2 -B2Repository: cetmix/cetmix-tower
Length of output: 45
🏁 Script executed:
# Read the parent view file to find applicability field definition
cat -n cetmix_tower_server/wizards/cx_tower_plan_run_wizard_view.xml | grep -A10 -B10 "applicability"Repository: cetmix/cetmix-tower
Length of output: 1462
Combine readonly conditions instead of replacing: The parent view already sets readonly="show_servers" on the applicability field (line 31 of cetmix_tower_server/wizards/cx_tower_plan_run_wizard_view.xml). The xpath with position="attributes" will override this, causing the field to ignore the show_servers guard. Change the attribute to readonly="show_servers or is_isolated_context" to preserve both conditions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cetmix_tower_jet_isolation/views/cx_tower_plan_run_wizard_views.xml` around
lines 15 - 18, The xpath that sets attributes on the field applicability
currently replaces the parent readonly condition; update the readonly attribute
to combine both guards by setting readonly="show_servers or is_isolated_context"
(keep the existing force_save attribute unchanged) so the field honors both the
parent show_servers and the local is_isolated_context conditions.
…rsion to 18.0.1.0.2 - UI: hide the div that contains and when the jet is in isolated mode and the user is not a manager. - XML: add to the field. - Python: re‑format and simplify the computation. - Manifest: version bumped to 18.0.1.0.2. - Pre‑commit: all hooks pass without errors.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py (1)
30-45:⚠️ Potential issue | 🔴 CriticalEnforce isolation server-side and derive forced values from all isolated jets.
default_getonly pre-fills form values, while the view restrictions are client-side. A caller can still create/write the transient record or invoke the run action with arbitraryapplicability/tag_ids. Also,_compute_is_restricted_contextchecks any isolated jet, but Lines 38-44 apply forced values from only the first jet, so mixed selections can miss the actual isolated template.At minimum, centralize the forced-value calculation across all isolated templates and call it from
default_getplus the server-side execution path (run_flight_plan, or create/write validation). Preserve any narrowerapplicabilityreturned bysuper()rather than widening it.#!/bin/bash # Verify whether plan isolation values are enforced outside default_get. fd -t f 'cx_tower_plan_run_wizard.py' | xargs -r rg -n -C4 \ 'def (default_get|create|write|run_flight_plan)|forced_plan_tag_ids|forced_applicability|is_restricted_context'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py` around lines 30 - 45, The current default_get uses only the first selected jet to apply forced_applicability/forced_plan_tag_ids and does not enforce these values server-side; extract the forced-value logic into a single helper method (e.g., _compute_forced_plan_values) that iterates over all jets returned by the browse of context["default_jet_ids"], computes the intersection/narrowest applicability and union/combined tag ids according to template.isolation_mode and template.forced_* fields, and returns {'applicability': ..., 'tag_ids': ...} while preserving any narrower applicability returned by super(); call this helper from default_get and also invoke it in the server-side execution paths (run_flight_plan and create/write validation) to enforce the forced values before proceeding.cetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xml (1)
19-22:⚠️ Potential issue | 🟠 MajorPreserve the parent
show_serversreadonly guard.Line 20 replaces the parent
readonly="show_servers"onapplicability, so the field becomes editable again in server-selection contexts wheneveris_restricted_contextis false. Combine both guards.Proposed fix
<xpath expr="//field[`@name`='applicability']" position="attributes"> - <attribute name="readonly">is_restricted_context</attribute> + <attribute name="readonly">show_servers or is_restricted_context</attribute> <attribute name="force_save">1</attribute> </xpath>#!/bin/bash # Verify the parent and inherited readonly expressions for command applicability. rg -n -C4 'name="applicability"|readonly="show_servers"|readonly">is_restricted_context' \ cetmix_tower_server/wizards/cx_tower_command_run_wizard_view.xml \ cetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xml` around lines 19 - 22, The xpath override for the field 'applicability' replaced the parent readonly guard 'show_servers' with 'is_restricted_context', making the field editable in server-selection contexts; update the readonly attribute on the xpath that targets field name='applicability' so it combines both guards (e.g. use a boolean expression that includes both is_restricted_context and show_servers such as "is_restricted_context or show_servers") and keep the existing force_save attribute unchanged.cetmix_tower_jet_isolation/views/cx_tower_plan_run_wizard_views.xml (1)
19-22:⚠️ Potential issue | 🟠 MajorPreserve the parent
show_serversreadonly guard.Line 20 replaces the parent
readonly="show_servers"onapplicability, so the field can become editable again in server-selection contexts wheneveris_restricted_contextis false. Combine both guards.Proposed fix
<xpath expr="//field[`@name`='applicability']" position="attributes"> - <attribute name="readonly">is_restricted_context</attribute> + <attribute name="readonly">show_servers or is_restricted_context</attribute> <attribute name="force_save">1</attribute> </xpath>#!/bin/bash # Verify the parent and inherited readonly expressions for plan applicability. rg -n -C4 'name="applicability"|readonly="show_servers"|readonly">is_restricted_context' \ cetmix_tower_server/wizards/cx_tower_plan_run_wizard_view.xml \ cetmix_tower_jet_isolation/views/cx_tower_plan_run_wizard_views.xml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cetmix_tower_jet_isolation/views/cx_tower_plan_run_wizard_views.xml` around lines 19 - 22, The current xpath override replaces the parent readonly="show_servers" on the field applicability with readonly="is_restricted_context", which removes the original server-selection guard; update the xpath attribute on field 'applicability' (the <xpath expr="//field[`@name`='applicability']" ...> override) so the readonly combines both guards (e.g. readonly="show_servers or is_restricted_context" or an equivalent boolean expression) and preserve force_save="1" so the field remains readonly when either condition applies.cetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.py (1)
30-45:⚠️ Potential issue | 🔴 CriticalEnforce isolation server-side and don’t rely on the first jet.
default_getonly initializes the form; it does not protect the execution path. A caller can still submit arbitraryapplicability/tag_idsthrough RPC or direct button calls. Also,_compute_is_restricted_contextchecks any isolated jet, but Lines 38-44 only usejet_ids[0], so forced command tags can be skipped for mixed selections.Apply the forced command restrictions from all isolated templates in a shared helper and invoke it from
default_getplus the server-side run/write path.#!/bin/bash # Verify whether command isolation values are enforced outside default_get. fd -t f 'cx_tower_command_run_wizard.py' | xargs -r rg -n -C4 \ 'def (default_get|create|write|run_command_on_server|run_command_in_wizard|action_run_command)|forced_command_tag_ids|forced_applicability|is_restricted_context'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.py` around lines 30 - 45, default_get currently only applies the first jet's forced restrictions and does not enforce them on server writes/runs; add a shared helper (e.g. a private method) that, given a recordset of jets (via jet_ids / jet_template_id) aggregates forced_applicability and the union of forced_command_tag_ids from all templates where isolation_mode is true, and returns the effective applicability and tag_ids; call this helper from default_get and in all server-side mutation/execution entrypoints (create, write, run_command_on_server, run_command_in_wizard, action_run_command) to overwrite/validate incoming vals so submitted applicability/tag_ids cannot bypass isolation; reference _compute_is_restricted_context, forced_applicability, forced_command_tag_ids, jet_template_id and ensure the helper is used wherever values are persisted or executed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xml`:
- Around line 24-28: Remove the static groups restriction on the tag_ids field
by deleting the attribute line that sets
groups="cetmix_tower_server.group_manager" inside the xpath targeting
//field[`@name`='tag_ids']; keep the existing invisible="is_restricted_context or
result" and force_save attributes so visibility remains dynamically controlled
by is_restricted_context and result logic (i.e., remove the <attribute
name="groups"> entry in the xpath for tag_ids).
- Around line 36-39: The xpath currently clearing attributes for the field
rendered_code removes the manager-only restriction; update the xpath so it
preserves the original groups restriction while adding readonly by setting the
groups attribute back to cetmix_tower_server.group_manager and setting
readonly="1" on the field (targeting the same xpath
expr="//field[`@name`='rendered_code']"). Ensure you do not leave <attribute
name="groups" /> empty—explicitly assign cetmix_tower_server.group_manager—so
rendered_code remains manager-only and readonly in the inherited view.
---
Duplicate comments:
In `@cetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xml`:
- Around line 19-22: The xpath override for the field 'applicability' replaced
the parent readonly guard 'show_servers' with 'is_restricted_context', making
the field editable in server-selection contexts; update the readonly attribute
on the xpath that targets field name='applicability' so it combines both guards
(e.g. use a boolean expression that includes both is_restricted_context and
show_servers such as "is_restricted_context or show_servers") and keep the
existing force_save attribute unchanged.
In `@cetmix_tower_jet_isolation/views/cx_tower_plan_run_wizard_views.xml`:
- Around line 19-22: The current xpath override replaces the parent
readonly="show_servers" on the field applicability with
readonly="is_restricted_context", which removes the original server-selection
guard; update the xpath attribute on field 'applicability' (the <xpath
expr="//field[`@name`='applicability']" ...> override) so the readonly combines
both guards (e.g. readonly="show_servers or is_restricted_context" or an
equivalent boolean expression) and preserve force_save="1" so the field remains
readonly when either condition applies.
In `@cetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.py`:
- Around line 30-45: default_get currently only applies the first jet's forced
restrictions and does not enforce them on server writes/runs; add a shared
helper (e.g. a private method) that, given a recordset of jets (via jet_ids /
jet_template_id) aggregates forced_applicability and the union of
forced_command_tag_ids from all templates where isolation_mode is true, and
returns the effective applicability and tag_ids; call this helper from
default_get and in all server-side mutation/execution entrypoints (create,
write, run_command_on_server, run_command_in_wizard, action_run_command) to
overwrite/validate incoming vals so submitted applicability/tag_ids cannot
bypass isolation; reference _compute_is_restricted_context,
forced_applicability, forced_command_tag_ids, jet_template_id and ensure the
helper is used wherever values are persisted or executed.
In `@cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py`:
- Around line 30-45: The current default_get uses only the first selected jet to
apply forced_applicability/forced_plan_tag_ids and does not enforce these values
server-side; extract the forced-value logic into a single helper method (e.g.,
_compute_forced_plan_values) that iterates over all jets returned by the browse
of context["default_jet_ids"], computes the intersection/narrowest applicability
and union/combined tag ids according to template.isolation_mode and
template.forced_* fields, and returns {'applicability': ..., 'tag_ids': ...}
while preserving any narrower applicability returned by super(); call this
helper from default_get and also invoke it in the server-side execution paths
(run_flight_plan and create/write validation) to enforce the forced values
before proceeding.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 73b1aa14-1fed-42d4-9e0f-5b3b9a5f90ab
📒 Files selected for processing (5)
cetmix_tower_jet_isolation/__manifest__.pycetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xmlcetmix_tower_jet_isolation/views/cx_tower_plan_run_wizard_views.xmlcetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.pycetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py
| <xpath expr="//field[@name='tag_ids']" position="attributes"> | ||
| <attribute name="groups">cetmix_tower_server.group_manager</attribute> | ||
| <attribute name="invisible">is_restricted_context or result</attribute> | ||
| <attribute name="force_save">1</attribute> | ||
| </xpath> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify static group narrowing on tag_ids versus dynamic restriction logic.
rg -n -C4 'name="tag_ids"|is_restricted_context|manager_ids|has_group\("cetmix_tower_server.group_manager"\)' \
cetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xml \
cetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.pyRepository: cetmix/cetmix-tower
Length of output: 7612
Remove static group restriction on tag_ids field.
Line 25 unnecessarily restricts tag_ids visibility to global managers only using groups="cetmix_tower_server.group_manager". This contradicts the dynamic is_restricted_context logic, which allows jet managers (non-global) to see the field when managing their own jets. The existing invisible="is_restricted_context or result" attribute already provides the correct visibility control.
Proposed fix
<xpath expr="//field[`@name`='tag_ids']" position="attributes">
- <attribute name="groups">cetmix_tower_server.group_manager</attribute>
<attribute name="invisible">is_restricted_context or result</attribute>
<attribute name="force_save">1</attribute>
</xpath>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <xpath expr="//field[@name='tag_ids']" position="attributes"> | |
| <attribute name="groups">cetmix_tower_server.group_manager</attribute> | |
| <attribute name="invisible">is_restricted_context or result</attribute> | |
| <attribute name="force_save">1</attribute> | |
| </xpath> | |
| <xpath expr="//field[`@name`='tag_ids']" position="attributes"> | |
| <attribute name="invisible">is_restricted_context or result</attribute> | |
| <attribute name="force_save">1</attribute> | |
| </xpath> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xml`
around lines 24 - 28, Remove the static groups restriction on the tag_ids field
by deleting the attribute line that sets
groups="cetmix_tower_server.group_manager" inside the xpath targeting
//field[`@name`='tag_ids']; keep the existing invisible="is_restricted_context or
result" and force_save attributes so visibility remains dynamically controlled
by is_restricted_context and result logic (i.e., remove the <attribute
name="groups"> entry in the xpath for tag_ids).
|
I was really worried that a non-manager user could modify the code and remove the tags, so I made the options invisible and set the code to read-only. |
marionumza
changed the title
…on data When the module is uninstalled Odoo does not drop the columns it added to cx.tower.jet.template, so existing records may keep isolation_mode=True and silently continue to restrict commands/plans even with the module gone. The new uninstall_hook resets all isolation-related fields to their neutral state (isolation_mode=False, forced_applicability=False, both tag M2M cleared) on every jet template before the module is removed from the registry. * hooks.py - new file with uninstall_hook function * __init__.py - import hooks so Odoo can resolve the hook entry point * __manifest__.py - register uninstall_hook key
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cetmix_tower_jet_isolation/__init__.py`:
- Line 1: The package-level module currently only imports submodules, so Odoo
cannot find the uninstall_hook callable; re-export the uninstall function by
importing uninstall_hook from the hooks submodule into the package namespace
(e.g., add a statement to __init__.py to import uninstall_hook from .hooks) and
optionally include "uninstall_hook" in the module's __all__ so getattr(package,
"uninstall_hook") will succeed; reference the symbol uninstall_hook and the
__init__.py module to locate where to add this import.
In `@cetmix_tower_jet_isolation/hooks.py`:
- Around line 10-17: The hook only resets active templates with
isolation_mode=True and thus skips archived records and templates that already
have isolation_mode=False but stale forced_* tag values; change the search call
on env["cx.tower.jet.template"] to select all records (no isolation_mode domain)
and pass active_test=False (e.g., search([], active_test=False)) so archived
templates are included, then call write(...) with the same payload to clear
isolation_mode, forced_applicability and force-clear
forced_command_tag_ids/forced_plan_tag_ids for every template.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2060c09b-8b43-420a-ad97-74b8a5c36e6e
📒 Files selected for processing (3)
cetmix_tower_jet_isolation/__init__.pycetmix_tower_jet_isolation/__manifest__.pycetmix_tower_jet_isolation/hooks.py
| env["cx.tower.jet.template"].search([("isolation_mode", "=", True)]).write( | ||
| { | ||
| "isolation_mode": False, | ||
| "forced_applicability": False, | ||
| "forced_command_tag_ids": [(5,)], | ||
| "forced_plan_tag_ids": [(5,)], | ||
| } | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the hooks.py file
find . -name "hooks.py" -path "*cetmix_tower_jet_isolation*" -type fRepository: cetmix/cetmix-tower
Length of output: 100
🏁 Script executed:
# Also search for the model definition to understand the structure
find . -path "*cetmix_tower_jet_isolation*" -name "*.py" -type f | head -20Repository: cetmix/cetmix-tower
Length of output: 477
🏁 Script executed:
# Check for model definitions with cx.tower.jet.template
rg "cx\.tower\.jet\.template" --type py -A 5 -B 5Repository: cetmix/cetmix-tower
Length of output: 50375
🏁 Script executed:
cat -n ./cetmix_tower_jet_isolation/hooks.pyRepository: cetmix/cetmix-tower
Length of output: 937
🏁 Script executed:
cat -n ./cetmix_tower_jet_isolation/models/cx_tower_jet_template.pyRepository: cetmix/cetmix-tower
Length of output: 882
🌐 Web query:
Odoo active_test default behavior models with active field
💡 Result:
In Odoo, models that define an 'active' field (typically BooleanField with default=True) have automatic filtering behavior in ORM methods like search, read_group, etc. The parameter 'active_test' controls whether this filtering is applied. Default behavior: active_test defaults to True when not specified in the context. This means searches and similar operations automatically filter to only include records where active=True (active records), excluding archived/inactive ones (active=False). To include inactive records, explicitly set active_test=False in the context, e.g.: self.with_context(active_test=False).search(domain) or in views: This applies to any model with an 'active' field (or custom _active_name field). Official docs confirm the 'active' field "toggles the global visibility of the record, if active is set to False the record is invisible in most searches and listing." Multiple secondary sources and code examples confirm the default is True, requiring explicit False to bypass filtering.
Citations:
- 1: https://www.odoo.com/documentation/19.0/developer/reference/backend/orm.html
- 2: https://www.dasolo.ai/blog/odoo-data-api-5/odoo-context-fields-guide-132
- 3: https://www.odoo.com/documentation/17.0/developer/reference/backend/orm.html
- 4: https://stackoverflow.com/questions/53708159/odoo-10-search-active-and-inactive-records-using-search-method
- 5: https://github.com/odoo/odoo/blob/17.0/odoo/models.py
🏁 Script executed:
# Find the parent cx.tower.jet.template model definition
rg "_name = \"cx\.tower\.jet\.template\"" --type py -A 20 -B 2Repository: cetmix/cetmix-tower
Length of output: 3753
🏁 Script executed:
# Check if the parent model has an active field
rg "active.*Boolean" cetmix_tower_server/models/cx_tower_jet_template.py -A 2 -B 2Repository: cetmix/cetmix-tower
Length of output: 190
Reset all isolation fields on both active and archived templates.
The current search only updates active templates with isolation_mode=True. Odoo's active_test parameter defaults to True, which automatically excludes archived records from all searches. Additionally, templates with isolation_mode=False but stale forced_* values are not reset at all. To ensure a truly clean state on module reinstall as documented in the hook docstring, clear isolation fields for all templates regardless of state.
🛠️ Proposed fix
- env["cx.tower.jet.template"].search([("isolation_mode", "=", True)]).write(
+ env["cx.tower.jet.template"].with_context(active_test=False).search([]).write(
{
"isolation_mode": False,
"forced_applicability": False,
"forced_command_tag_ids": [(5,)],
"forced_plan_tag_ids": [(5,)],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cetmix_tower_jet_isolation/hooks.py` around lines 10 - 17, The hook only
resets active templates with isolation_mode=True and thus skips archived records
and templates that already have isolation_mode=False but stale forced_* tag
values; change the search call on env["cx.tower.jet.template"] to select all
records (no isolation_mode domain) and pass active_test=False (e.g., search([],
active_test=False)) so archived templates are included, then call write(...)
with the same payload to clear isolation_mode, forced_applicability and
force-clear forced_command_tag_ids/forced_plan_tag_ids for every template.
…t level Odoo resolves the hook with getattr(py_module, 'uninstall_hook'), so the function must be a direct attribute of the package, not just a submodule. Replace 'from . import hooks' with 'from .hooks import uninstall_hook'.
…olation mode In restricted context (isolation mode, non-manager user), tag_ids was completely hidden which also caused the command/plan selector to show 'No records' since the forced tags were set but not visible. Now tag_ids is always visible but readonly when is_restricted_context=True, so users can see which tags are filtering their available commands/plans.
…for restricted users The many2many_tags widget in Odoo 18 does not enforce visual readonly state via the readonly XML modifier alone. Replace the readonly approach with a dedicated computed field 'isolated_tag_ids' that mirrors tag_ids. Computed fields are inherently readonly in Odoo's OWL frontend (no add/remove buttons rendered), guaranteeing that restricted users see the forced tags but cannot modify them. - Restricted users: see isolated_tag_ids (computed, always readonly) - Managers / jet managers: see tag_ids (editable, manager group only)
There was a problem hiding this comment.
♻️ Duplicate comments (6)
cetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xml (3)
24-28:⚠️ Potential issue | 🟠 MajorDo not make
tag_idsglobally manager-only.Line 25 blocks non-global per-jet managers even when
is_restricted_contextis false and the dynamic wizard logic should allow them to edit tags. Keep visibility controlled byis_restricted_context/result.Proposed fix
<xpath expr="//field[`@name`='tag_ids']" position="attributes"> - <attribute name="groups">cetmix_tower_server.group_manager</attribute> <attribute name="invisible">is_restricted_context or result</attribute> <attribute name="force_save">1</attribute> </xpath>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xml` around lines 24 - 28, The xpath for the field tag_ids is incorrectly forcing global manager-only access via the attribute name="groups" value cetmix_tower_server.group_manager; remove that groups attribute (or stop setting it) so visibility remains controlled solely by the existing invisible expression (is_restricted_context or result) and force_save stays intact; update the xpath that targets field[`@name`='tag_ids'] to only set the invisible and force_save attributes, not the groups attribute.
44-47:⚠️ Potential issue | 🟠 MajorKeep
rendered_codemanager-only.Line 45 clears the parent
groups="cetmix_tower_server.group_manager"restriction, exposing rendered command code to users who could not see it in the base wizard.Proposed fix
<xpath expr="//field[`@name`='rendered_code']" position="attributes"> - <attribute name="groups" /> + <attribute name="groups">cetmix_tower_server.group_manager</attribute> <attribute name="readonly">1</attribute> </xpath>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xml` around lines 44 - 47, The xpath override on the field rendered_code removes the parent groups restriction, exposing sensitive command output; restore the manager-only restriction by setting the groups attribute back to cetmix_tower_server.group_manager on the rendered_code field (keep the readonly attribute as-is) — i.e. update the <xpath> that targets field name="rendered_code" to include attribute name="groups" with value "cetmix_tower_server.group_manager" (or remove the attribute override so the base view's groups remains).
19-22:⚠️ Potential issue | 🟠 MajorPreserve the parent
show_serversreadonly guard.Line 20 replaces the base
readonly="show_servers"with onlyis_restricted_context, soapplicabilitycan become editable again after the base view intended it to lock. Combine both conditions.Proposed fix
<xpath expr="//field[`@name`='applicability']" position="attributes"> - <attribute name="readonly">is_restricted_context</attribute> + <attribute name="readonly">show_servers or is_restricted_context</attribute> <attribute name="force_save">1</attribute> </xpath>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xml` around lines 19 - 22, The current xpath targeting field name="applicability" replaces the base readonly guard (readonly="show_servers") with only is_restricted_context, allowing the field to become editable when it should be locked; update the readonly attribute on that xpath to combine both conditions (e.g., use a logical OR between show_servers and is_restricted_context) so the field remains readonly when either guard applies, and leave the existing force_save attribute unchanged.cetmix_tower_jet_isolation/views/cx_tower_plan_run_wizard_views.xml (1)
19-22:⚠️ Potential issue | 🟠 MajorPreserve the parent
show_serversreadonly guard.Line 20 replaces the base
readonly="show_servers"with onlyis_restricted_context, soapplicabilitycan become editable again when the base wizard intended it to stay locked.Proposed fix
<xpath expr="//field[`@name`='applicability']" position="attributes"> - <attribute name="readonly">is_restricted_context</attribute> + <attribute name="readonly">show_servers or is_restricted_context</attribute> <attribute name="force_save">1</attribute> </xpath>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cetmix_tower_jet_isolation/views/cx_tower_plan_run_wizard_views.xml` around lines 19 - 22, The xpath replacing the applicability field removed the base readonly guard (`show_servers`); update the xpath attributes so the readonly combines both guards instead of replacing it — e.g. set the `readonly` attribute on the `<xpath>` for the `field name="applicability"` to include both `is_restricted_context` and `show_servers` (logical OR), leaving `force_save="1"` intact so the field stays readonly when either guard applies.cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py (1)
40-55:⚠️ Potential issue | 🔴 CriticalEnforce isolated plan constraints server-side and derive them from all selected jets.
default_getonly pre-fills the form; a caller can still submit arbitraryapplicability/tag_idsvia RPC. This also inspects onlyjet_ids[0]while_compute_is_restricted_contexttreats any isolated selected jet as restricted, and it can widen a baseapplicability="this"restriction to a template’s"shared"value.Enforce the forced plan values in
create/writeor the run action, derive tags/applicability from all isolated selected templates, and never relax the value returned bysuper().default_get().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py` around lines 40 - 55, default_get currently only pre-fills values from the first jet and can be bypassed; enforce isolation constraints server-side by applying forced_applicability and forced_plan_tag_ids during persistence or execution (implement in create and write of the cx.tower.plan.run.wizard model and/or in the run/action method that performs the plan run), derive the final applicability and tag_ids by iterating over all jets from the provided jet_ids (or context default_jet_ids) and combining their jet_template_id.forced_applicability/forced_plan_tag_ids such that any forced restriction wins (i.e., do not relax an applicability returned by super().default_get), and ensure tag_ids are set to the union of all forced_plan_tag_ids (use ORM operations to replace/merge tag_ids via [(6,0, ids)] where needed) before allowing the record to be created/updated or the run to proceed.cetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.py (1)
40-55:⚠️ Potential issue | 🔴 CriticalEnforce isolated command constraints server-side and derive them from all selected jets.
default_getonly pre-fills the form; a caller can still submit arbitraryapplicability/tag_idsvia RPC. It also inspects onlyjet_ids[0], while_compute_is_restricted_contextrestricts the wizard when any selected jet is isolated, so mixed selections can miss the forced command tags entirely.Enforce the forced command values in
create/writeor the run action, and compute the effective forced values from all isolated selected templates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.py` around lines 40 - 55, default_get only pre-fills values and looks at the first jet, so enforce isolation constraints server-side by overriding create and write (or the run action that executes commands) on the cx.tower.command.run.wizard model: compute the effective forced_applicability and forced_command_tag_ids by iterating all selected jets (from vals.get("jet_ids") or context["default_jet_ids"]), collecting templates where template.isolation_mode is True, then derive the effective applicability (use the template.forced_applicability when present) and the union of all template.forced_command_tag_ids.ids; if any isolated template exists, replace/force vals["applicability"] and vals["tag_ids"] (convert tag ids to Odoo command format [(6,0, ids)]) before calling super().create/super().write or before running the command, so submitted RPC payloads cannot bypass isolation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@cetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xml`:
- Around line 24-28: The xpath for the field tag_ids is incorrectly forcing
global manager-only access via the attribute name="groups" value
cetmix_tower_server.group_manager; remove that groups attribute (or stop setting
it) so visibility remains controlled solely by the existing invisible expression
(is_restricted_context or result) and force_save stays intact; update the xpath
that targets field[`@name`='tag_ids'] to only set the invisible and force_save
attributes, not the groups attribute.
- Around line 44-47: The xpath override on the field rendered_code removes the
parent groups restriction, exposing sensitive command output; restore the
manager-only restriction by setting the groups attribute back to
cetmix_tower_server.group_manager on the rendered_code field (keep the readonly
attribute as-is) — i.e. update the <xpath> that targets field
name="rendered_code" to include attribute name="groups" with value
"cetmix_tower_server.group_manager" (or remove the attribute override so the
base view's groups remains).
- Around line 19-22: The current xpath targeting field name="applicability"
replaces the base readonly guard (readonly="show_servers") with only
is_restricted_context, allowing the field to become editable when it should be
locked; update the readonly attribute on that xpath to combine both conditions
(e.g., use a logical OR between show_servers and is_restricted_context) so the
field remains readonly when either guard applies, and leave the existing
force_save attribute unchanged.
In `@cetmix_tower_jet_isolation/views/cx_tower_plan_run_wizard_views.xml`:
- Around line 19-22: The xpath replacing the applicability field removed the
base readonly guard (`show_servers`); update the xpath attributes so the
readonly combines both guards instead of replacing it — e.g. set the `readonly`
attribute on the `<xpath>` for the `field name="applicability"` to include both
`is_restricted_context` and `show_servers` (logical OR), leaving
`force_save="1"` intact so the field stays readonly when either guard applies.
In `@cetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.py`:
- Around line 40-55: default_get only pre-fills values and looks at the first
jet, so enforce isolation constraints server-side by overriding create and write
(or the run action that executes commands) on the cx.tower.command.run.wizard
model: compute the effective forced_applicability and forced_command_tag_ids by
iterating all selected jets (from vals.get("jet_ids") or
context["default_jet_ids"]), collecting templates where template.isolation_mode
is True, then derive the effective applicability (use the
template.forced_applicability when present) and the union of all
template.forced_command_tag_ids.ids; if any isolated template exists,
replace/force vals["applicability"] and vals["tag_ids"] (convert tag ids to Odoo
command format [(6,0, ids)]) before calling super().create/super().write or
before running the command, so submitted RPC payloads cannot bypass isolation.
In `@cetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py`:
- Around line 40-55: default_get currently only pre-fills values from the first
jet and can be bypassed; enforce isolation constraints server-side by applying
forced_applicability and forced_plan_tag_ids during persistence or execution
(implement in create and write of the cx.tower.plan.run.wizard model and/or in
the run/action method that performs the plan run), derive the final
applicability and tag_ids by iterating over all jets from the provided jet_ids
(or context default_jet_ids) and combining their
jet_template_id.forced_applicability/forced_plan_tag_ids such that any forced
restriction wins (i.e., do not relax an applicability returned by
super().default_get), and ensure tag_ids are set to the union of all
forced_plan_tag_ids (use ORM operations to replace/merge tag_ids via [(6,0,
ids)] where needed) before allowing the record to be created/updated or the run
to proceed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0f84621a-b02a-4cc3-b39f-5a221d087ca5
📒 Files selected for processing (5)
cetmix_tower_jet_isolation/__init__.pycetmix_tower_jet_isolation/views/cx_tower_command_run_wizard_views.xmlcetmix_tower_jet_isolation/views/cx_tower_plan_run_wizard_views.xmlcetmix_tower_jet_isolation/wizards/cx_tower_command_run_wizard.pycetmix_tower_jet_isolation/wizards/cx_tower_plan_run_wizard.py
2 participants
Introduces a strict isolation mode for Jet Templates to restrict command and flight plan execution.
Key features:
isolation_modetocx.tower.jet.templateto enforce applicability and tags.cx.tower.command.run.wizardandcx.tower.plan.run.wizardviadefault_get.cx.tower.tagmodel for seamless integration without modifying Odoo core domains.Summary by CodeRabbit