Skip to content

fix(audit): ensure valid payloads and resource attribution - #3143

Open
jordan-umusu wants to merge 1 commit into
mainfrom
fix/audit-log-validity
Open

fix(audit): ensure valid payloads and resource attribution#3143
jordan-umusu wants to merge 1 commit into
mainfrom
fix/audit-log-validity

Conversation

@jordan-umusu

@jordan-umusu jordan-umusu commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Why

The existing audit coverage established the main emission paths. Followup end-to-end QA surfaced a few edge cases where delivered events could be incomplete or use stale configuration.

This PR ensures:

  • Canonical fields cannot be overwritten by custom payloads.
  • Resource IDs are included across different service signatures.
  • Rejected schedule operations emit ATTEMPT and FAILURE.
  • Invalidate audit-setting caching correctly

Summary by cubic

Ensure audit events keep canonical fields and correct resource IDs, and make audit webhook setting changes apply immediately. Move schedule validation into the service to emit accurate ATTEMPT/FAILURE events with clear 404s.

  • Bug Fixes
    • Prevent custom payloads from overriding canonical fields; event values win (resource_type, resource_id, status).
    • Capture correct resource IDs across audit paths (keyword objects; case trigger updates; explicit resource_id_attr for workflow create, workspace delete, and invitation revoke).
    • Clear the audit-setting TTL cache after platform/org setting updates via clear_audit_setting_cache, so the next event uses the new webhook config.
    • Validate workflow existence and published state inside the schedules service; map errors to 404 and emit ATTEMPT then FAILURE.
    • Refresh updated_at on role and group updates to return fresh timestamps.

Written for commit c1f1043. Summary will update on new commits.

Review in cubic

@jordan-umusu
jordan-umusu marked this pull request as ready for review July 28, 2026 15:01
@zeropath-ai

zeropath-ai Bot commented Jul 28, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to c1f1043.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► tracecat/audit/service.py
    Add clear_audit_setting_cache to refresh cache
► tracecat/settings/service.py
    Refresh audit cache after audit-related settings updates
► tracecat/workflow/schedules/service.py
    Add _require_published_workflow precondition and integrate into create_schedule
► tracecat/workflow/management/management.py
    Audit integration for create_workflow with resource_id attr
► tracecat/workspaces/service.py
    Audit log updated to include resource_id for delete_workspace and related actions
Bug Fix ► tracecat/audit/logger.py
    Improve _extract_resource_id to consider first keyword argument object with matching attribute
► tracecat/audit/service.py
    Expose clear_audit_setting_cache function for tests and usage
► tests/unit/test_admin_settings_service.py
    Mock clear_audit_setting_cache in tests to verify cache clearing
► tests/unit/test_organization_settings.py
    Verify audit cache clear on update_audit_settings
► tracecat/settings/schemas.py
    Update description text for audit webhook custom payload to reflect field-based merging
► tracecat/settings/service.py
    Call clear_audit_setting_cache when audit-related keys updated or list/update operations occur
► tracecat/workflow/case_triggers/service.py
    Change _case_trigger_update_audit_details to async and include resource_id in audit data
► tracecat/workflow/schedules/router.py
    Map TracecatNotFoundError to HTTP 404 for schedule creation failures
Refactor ► tracecat/audit/service.py
    Move cache clearing utility and adjust related logic
► tracecat/tracecat_ee/rbac/service.py and related tests changes for updated refresh behavior
► tracecat/tracecat-ee/tracecat_ee/admin/settings/service.py
    Call cache clear after updates to audit-related settings
Other ► frontend/src/client/schemas.gen.ts and frontend/src/client/types.gen.ts
    Update audit webhook payload description text in comments

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a7f868cd17

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tracecat/audit/service.py
Comment thread tracecat/audit/service.py
@jordan-umusu
jordan-umusu force-pushed the fix/audit-log-validity branch from a7f868c to c1f1043 Compare July 28, 2026 16:13
@jordan-umusu
jordan-umusu requested a review from daryllimyt July 28, 2026 16:43
@jordan-umusu jordan-umusu added the priority:medium Medium priority ticket label Jul 28, 2026

@daryllimyt daryllimyt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm, couple of questions:
1. do we have to emit db queries inside the audit log handler? unsure if this is what we are already doing, probably fine.
2. instead of changing the service method shape maybe we can expose a context manager API to wrap a specific block? doesn't always have to be used as a decorator

VERSIONED_RESOURCE_RESOLUTION_STRATEGY_SETTING = (
"app_versioned_resource_resolution_strategy"
)
AUDIT_SETTINGS_KEYS = AuditSettingsUpdate.keys()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: frozenset

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

Labels

fix Bug fix logging priority:medium Medium priority ticket

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants