You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tooling.apache.org by "asf-tooling (via GitHub)" <gi...@apache.org> on 2026/04/03 18:39:54 UTC

[I] Dynamic Field Assignment Without Explicit Allowlist in Policy Updates (tooling-trusted-releases)

asf-tooling opened a new issue, #1047:
URL: https://github.com/apache/tooling-trusted-releases/issues/1047

   **ASVS Level(s):** L2-only
   
   **Description:**
   
   ### Summary
   The policy update function uses dynamic field assignment via `setattr()` loop without an explicit allowlist, creating risk that future model expansions could inadvertently expose additional writable fields. This violates BOPLA protection principles where field-level write access should be explicit, not derived from model structure. Sibling methods (`edit_compose`, `edit_vote`) use explicit field assignment, creating an inconsistency.
   
   ### Details
   In `atr/storage/writers/policy.py` lines 117-140, the update function iterates over `update.model_fields_set` and applies changes via `setattr()` without checking against an explicit allowlist of editable fields. If the Pydantic model is expanded with new fields in the future, those fields would automatically become editable through this endpoint without explicit security review.
   
   ### Recommended Remediation
   Define an explicit allowlist and enforce it:
   
   ```python
   _EDITABLE_POLICY_FIELDS = frozenset({
       'manual_vote',
       'min_hours',
       'github_repository_name',
       'github_workflow_path',
       # Add other intentionally editable fields
   })
   
   def edit_policy(self, policy_id: int, update: PolicyUpdate) -> Outcome:
       policy = self._get_policy(policy_id)
       
       # Intersect with allowlist
       requested_fields = update.model_fields_set
       disallowed_fields = requested_fields - _EDITABLE_POLICY_FIELDS
       
       if disallowed_fields:
           return Outcome.err(f"Cannot edit fields: {disallowed_fields}")
       
       for field_name in requested_fields & _EDITABLE_POLICY_FIELDS:
           setattr(policy, field_name, getattr(update, field_name))
   ```
   
   This ensures field editability is explicitly controlled rather than implicitly derived from the Pydantic model structure.
   
   ### Acceptance Criteria
   - [ ] `_EDITABLE_POLICY_FIELDS` allowlist defined as frozenset
   - [ ] Update function validates requested fields against allowlist
   - [ ] Disallowed fields rejected with clear error message
   - [ ] Unit tests verify allowlist enforcement
   - [ ] Unit tests verify future model additions don't automatically become editable
   - [ ] Documentation updated to reflect explicit field control
   
   ### References
   - Source reports: L2:8.2.3.md
   - Related findings: None
   - ASVS sections: 8.2.3
   
   ### Priority
   Medium
   
   ---
   
   ---
   
   **Triage notes:** audit_guidance we give room to committees to modify their policy, and anything that is editable is allowed to be


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@tooling.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tooling.apache.org
For additional commands, e-mail: dev-help@tooling.apache.org

Re: [I] Dynamic Field Assignment Without Explicit Allowlist in Policy Updates (tooling-trusted-releases)

Posted by "andrewmusselman (via GitHub)" <gi...@apache.org>.
andrewmusselman closed issue #1047: Dynamic Field Assignment Without Explicit Allowlist in Policy Updates
URL: https://github.com/apache/tooling-trusted-releases/issues/1047


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@tooling.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tooling.apache.org
For additional commands, e-mail: dev-help@tooling.apache.org

Re: [I] Dynamic Field Assignment Without Explicit Allowlist in Policy Updates (tooling-trusted-releases)

Posted by "andrewmusselman (via GitHub)" <gi...@apache.org>.
andrewmusselman commented on issue #1047:
URL: https://github.com/apache/tooling-trusted-releases/issues/1047#issuecomment-4241317921

   Closed by https://github.com/apache/tooling-runbooks/commit/0a268942efcd90c3fe99d5901f7e59022e34c95f


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@tooling.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tooling.apache.org
For additional commands, e-mail: dev-help@tooling.apache.org