Skip to content

Adding feature to update rules in place rather than having to only create new ones - #253

Open
Pavornoc wants to merge 1 commit into
google:mainfrom
Pavornoc:feat/add_update_rule_ability
Open

Adding feature to update rules in place rather than having to only create new ones#253
Pavornoc wants to merge 1 commit into
google:mainfrom
Pavornoc:feat/add_update_rule_ability

Conversation

@Pavornoc

@Pavornoc Pavornoc commented May 7, 2026

Copy link
Copy Markdown

Summary

Adds an update_rule MCP tool to the SecOps server, filling a gap in the existing
rule management workflow.

Currently, the only way to modify an existing YARA-L detection rule via MCP is to
create a new rule and manually clean up the old version. update_rule wraps the
existing chronicle.update_rule() client method (PATCH rules/{rule_id} with
update_mask=text) to replace rule text in place, preserving the rule's ID,
deployment state, and version history.

Changes

  • server/secops/secops_mcp/tools/security_rules.py — new update_rule tool,
    inserted after create_rule. No existing code modified.
  • server/secops/tests/test_secops_rules_unit.py — new unit test file with 9 tests
    covering the success path, correct argument forwarding, version string surfacing,
    graceful handling of missing API fields, and error propagation.

Notes

  • The underlying secops library already exposes ChronicleClient.update_rule(rule_id, rule_text);
    this PR only adds the MCP tool layer.
  • The rule name parser in the response message uses the same replace("rule ", "")
    approach as create_rule for consistency. A fix to both functions would be a
    separate follow-on.

@Pavornoc
Pavornoc requested a review from a team May 7, 2026 20:52
@google-cla

google-cla Bot commented May 7, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@dandye dandye left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding the update_rule tool to the SecOps MCP server. This is a very helpful capability that fills an important gap in detection rule management.

Before merging, please address the following review feedback items:

  1. Use lazy logging formatting instead of eager f-strings:
    In update_rule:

    • Replace logger.info(f"Updating detection rule {rule_id}") with logger.info("Updating detection rule %s", rule_id).
    • Replace logger.error(f"Error updating rule {rule_id}: {str(e)}", exc_info=True) with logger.error("Error updating rule %s: %s", rule_id, e, exc_info=True).
      Using lazy %s formatting avoids eager string interpolation and conforms with logging standards across the repository.
  2. Use regex for YARA-L rule name extraction:
    Using line.strip().replace("rule ", "").replace(" {", "").strip() can corrupt rule names containing rule as a substring (e.g., rule rule_execution_detected { becomes _execution_detected) and can fail with non-standard spacing or leading comments.
    Please use regular expression matching instead:

    match = re.search(r"^\s*rule\s+([A-Za-z0-9_]+)", rule_text, re.MULTILINE)
    if match:
        result += f"Rule Name: {match.group(1)}\n"
  3. Safe resource name and version extraction:
    Ensure rule dictionary access handles unexpected types safely:

    resource_name = rule.get("name", "") if isinstance(rule, dict) else ""
    version_id = resource_name.split("/")[-1] if resource_name else ""
    if "@" in version_id:
        result += f"New version: {version_id}\n"
  4. Align docstrings with optional parameter defaults:
    In the docstring, project_id, customer_id, and region are marked as (required), but in the function signature they default to Optional[str] = None. Please update the docstring to specify (Optional[str]) and note that they default to environment configuration.

  5. Deduplicate response text:
    The return string repeats "Successfully updated detection rule." at the beginning and "Rule updated successfully." at the end. Please clean up the text so the confirmation message is not duplicated.

Once these updates are in place, we will be ready to approve and merge this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants