fix: 同步master分支代码

[Mianhuatang8]

No description provided.

[github-actions]

Code Review Summary

PR: fix: 同步master分支代码
Scope: Large sync PR covering webhook integration at template level, loop execution for subprocess plugins, permission model expansion, Mako template name whitelist, multiple new APIGW endpoints, and task message enrichment.

Issues Found

---

🔴 Critical

1. get_plugin_outputs.py — Missing request body validation (bkflow/apigw/views/get_plugin_outputs.py:42-43)

data = json.loads(request.body)
plugin_id = data["plugin_id"]

Uses raw json.loads + direct dict access without validation. If request.body is empty/malformed or plugin_id key is missing, this produces an unhelpful 500 error (caught by return_json_response decorator, but returns a generic message). Should use a serializer for request body validation to return proper 400 responses with clear error messages.

Suggestion: Define a request body serializer (similar to other APIGW views) to validate plugin_id is present and is a string.

---

2. template.py & delete_template.py — Bare raise Exception inside transaction (bkflow/template/views/template.py:345, bkflow/apigw/views/delete_template.py:59)

raise Exception(result.get("message", "Failed to clear webhooks"))

Using bare Exception to trigger transaction rollback is an anti-pattern. It bypasses DRF's exception handling (won't produce a structured API response), and makes it impossible for callers to catch specific error types. In delete_template.py this results in the return_json_response decorator catching it and returning a 500 code — but this is actually a business logic failure, not a server error.

Suggestion: Define a custom exception (e.g., WebhookClearError(APIException)) or use an existing one from the project, and handle it appropriately.

---

🟡 Important

3. v1_0_0.py — No bounds check on self.inner_loop index access (bkflow/pipeline_plugins/components/collections/subprocess_plugin/v1_0_0.py:148,152)

loop_item_value = list(inputs)[self.inner_loop - 1]
# ...
loop_item_value = items[self.inner_loop - 1]

If self.inner_loop exceeds the length of inputs or items, this will raise an IndexError with no meaningful context. While the loop framework presumably calls within bounds, a defensive check with a clear error message would greatly aid debugging in edge cases (e.g., race conditions, data corruption).

Suggestion: Add a bounds check before indexing:

if self.inner_loop > len(inputs):
    raise ValidationError(f"inner_loop({self.inner_loop}) exceeds iterable length({len(inputs)}) for param {param_key}")

---

4. webhook.py — clear_scope_webhooks performs 3 independent DELETEs without its own transaction (bkflow/utils/webhook.py:87-96)

def clear_scope_webhooks(scope_code: list):
    try:
        WebhookModel.objects.filter(...).delete()
        ScopeModel.objects.filter(...).delete()
        Subscription.objects.filter(...).delete()
    except Exception as e:
  ...
        return {"result": False, ...}
    return {"result": True, ...}

If the second or third DELETE fails after the first succeeds, and this function is called outside a transaction (which is possible since it's a standalone utility), orphaned records will remain. Currently callers wrap it in transaction.atomic(), but the function itself does not enforce transactional safety.

Suggestion: Add @transaction.atomic to clear_scope_webhooks or document that callers must provide a transaction.

---

5. tasks.py — send_task_message signature change (added node_id) may break existing callers (bkflow/task/celery/tasks.py:100)

def send_task_message(task_id, node_id, msg_type):

The new node_id parameter is positional, inserted between task_id and msg_type. Any existing apply_async or .delay() calls using positional args in the old order (task_id, msg_type) will silently misassign msg_type to node_id. Verify all callers have been updated.

---

6. space/views.py — broadcast_task_events unconditionally accesses data["template_id"] (bkflow/space/views.py:317)

scopes = [Scope(type=WebhookScopeType.TEMPLATE.value, code=str(data["template_id"]))]

While current callers (celery tasks) always provide template_id, this endpoint is an internal API accessible by any authorized app. If template_id is missing, it raises an unhandled KeyError. Consider using data.get("template_id") with early return or validation.

---

🔵 Minor

7. get_plugin_outputs.py — Inconsistent error handling for single vs. multiple components (bkflow/apigw/views/get_plugin_outputs.py:48-68)

When plugin_version is provided, ComponentModel.DoesNotExist is caught. But when plugin_version is not provided, ComponentModel.objects.filter(code=plugin_id) returns an empty queryset silently (no error), which may confuse callers who get {"result": True, "data": []} instead of an explicit "not found".

---

8. config/default.py — Logging propagate changed from True to False

Changing propagate to False for pipeline, bamboo_engine, and pipeline_engine loggers means these logs will no longer bubble up to root logger handlers. Ensure dedicated handlers are configured for these loggers, otherwise log messages may be silently dropped.

---

Overall Assessment

The code changes are generally well-structured with good use of transactions for delete operations and proper validation on webhook configuration parameters. The main concerns are around defensive error handling (bare exceptions, unvalidated request bodies) and ensuring the function signature change in send_task_message doesn't break callers.