Skip to content

New adversary WIP#3252

Closed
deacon-mp wants to merge 16 commits into
masterfrom
newAdversary
Closed

New adversary WIP#3252
deacon-mp wants to merge 16 commits into
masterfrom
newAdversary

Conversation

@deacon-mp
Copy link
Copy Markdown
Contributor

Description

(insert summary)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

@deacon-mp deacon-mp requested review from Copilot and uruwhy February 10, 2026 15:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR appears to be a WIP toward supporting a new “automation/adversary” flow by allowing adversary atomic ordering steps to carry per-step metadata (e.g., executor-scoped facts), adding an automation import service/planner, and adjusting persistence/update flows for adversaries.

Changes:

  • Extend adversary and planning/link generation to support atomic_ordering entries as either strings or dicts with metadata, and inject facts into executor templates.
  • Add new automation components (automation_svc, automation_planner) for importing/playing back operations/actions.
  • Modify object update/save logic and plugin loading behavior; adjust config YAMLs and ignore patterns; add uuid dependency.

Reviewed changes

Copilot reviewed 35 out of 39 changed files in this pull request and generated 15 comments.

Show a summary per file
File Description
plugins/ssl Removes submodule pointer entry
plugins/sandcat Removes submodule pointer entry
plugins/response Removes submodule pointer entry
plugins/magma Updates submodule pointer commit
plugins/human Removes submodule pointer entry
plugins/gameboard Removes submodule pointer entry
plugins/fieldmanual Removes submodule pointer entry
plugins/emu Removes submodule pointer entry
plugins/debrief Removes submodule pointer entry
plugins/compass Removes submodule pointer entry
plugins/builder Removes submodule pointer entry
plugins/atomic Removes submodule pointer entry
plugins/access Removes submodule pointer entry
package.json Adds uuid dependency
conf/payloads.yml Populates payload registry with extensions and payload definitions
conf/default.yml Reorders/updates contact + plugin config entries
conf/agents.yml Adds deployments list and lowers sleep settings
app/service/planning_svc.py Adds step metadata parsing, fact injection, and extensive debug logging
app/service/file_svc.py Adds directory creation before file saves
app/service/data_svc.py Adds debug log for specific adversary filename
app/service/automation_svc.py New YAML import/validation service
app/planners/automation_planner.py New planner to execute “actions” derived from facts/relationships
app/planners/atomic.py Updates atomic step selection to prefer step_idx and adds print debug
app/objects/secondclass/c_link.py Adds metadata field to Link schema/model
app/objects/c_plugin.py Adds required plugins + skips loading/enabling plugins not configured
app/objects/c_adversary.py Allows atomic_ordering to be raw entries (string/dict) + adds debug logging
app/contacts/contact_http.py Adds exception stack trace logging
app/api/v2/managers/operation_api_manager.py Adds local variable assignment during operation setup
app/api/v2/managers/base_api_manager.py Alters on-disk update/replace behavior and adds verbose logging
app/api/v2/managers/adversary_api_manager.py Adds stack trace logging and try/except around verify
app/api/v2/handlers/base_object_api.py Changes existence check + changes update error handling/behavior
app/api/v2/handlers/adversary_api.py Refactors handler typing/docs and overrides update/create-or-update flows
AdversarySCHEMA.yml Adds sample adversary schema YAML with step metadata
.gitmodules Removes all submodule definitions
.gitignore Adds ignore rules for data/adversaries and various plugin dirs; duplicates patterns
.coveragerc Deletes coverage configuration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/service/file_svc.py
return '%s.xored' % filename

def _save(self, filename, content, encrypt=True):
os.makedirs(os.path.dirname(filename), exist_ok=True)
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

os.path.dirname(filename) can be an empty string (e.g., saving "foo.bin" in the current working directory). Calling os.makedirs('') raises FileNotFoundError. Guard by only creating directories when the dirname is non-empty (e.g., compute dirpath = os.path.dirname(filename) and mkdir only if dirpath).

Suggested change
os.makedirs(os.path.dirname(filename), exist_ok=True)
dirpath = os.path.dirname(filename)
if dirpath:
os.makedirs(dirpath, exist_ok=True)

Copilot uses AI. Check for mistakes.
Comment on lines +154 to 173
# 🧠 Update in-memory object fields
obj.update('name', data.get('name'))
obj.update('description', data.get('description'))

# ✅ THIS is the critical fix
obj.atomic_ordering = data.get('atomic_ordering', [])
# if any(isinstance(s, dict) and "metadata" in s for s in obj.atomic_ordering):
# obj.metadata = {}
self.log.debug(
"[REPLACE] Writing atomic_ordering (len=%s)",
len(obj.atomic_ordering)
)

obj.update('objective', data.get('objective'))
obj.update('tags', list(data.get('tags', [])))
obj.update('plugin', data.get('plugin'))

dumped = obj.schema.dump(obj)


Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

replace_on_disk_object is in the base manager and appears to be used for multiple object types, but it now unconditionally assigns adversary-specific fields (atomic_ordering/objective/tags/plugin). For non-adversary objects, this can raise AttributeError or silently corrupt state. Move this adversary-specific logic into AdversaryApiManager (override replace/update path there), or gate these assignments with hasattr(obj, ...) / schema field presence to keep BaseApiManager generic.

Suggested change
# 🧠 Update in-memory object fields
obj.update('name', data.get('name'))
obj.update('description', data.get('description'))
# ✅ THIS is the critical fix
obj.atomic_ordering = data.get('atomic_ordering', [])
# if any(isinstance(s, dict) and "metadata" in s for s in obj.atomic_ordering):
# obj.metadata = {}
self.log.debug(
"[REPLACE] Writing atomic_ordering (len=%s)",
len(obj.atomic_ordering)
)
obj.update('objective', data.get('objective'))
obj.update('tags', list(data.get('tags', [])))
obj.update('plugin', data.get('plugin'))
dumped = obj.schema.dump(obj)
# Update common in-memory object fields, if present on this object type
if hasattr(obj, 'name'):
obj.update('name', data.get('name'))
if hasattr(obj, 'description'):
obj.update('description', data.get('description'))
# Adversary-specific fields: only update if the object supports them
if hasattr(obj, 'atomic_ordering') and 'atomic_ordering' in data:
obj.atomic_ordering = data.get('atomic_ordering') or []
# if any(isinstance(s, dict) and "metadata" in s for s in obj.atomic_ordering):
# obj.metadata = {}
self.log.debug(
"[REPLACE] Writing atomic_ordering (len=%s)",
len(obj.atomic_ordering)
)
if hasattr(obj, 'objective') and 'objective' in data:
obj.update('objective', data.get('objective'))
if hasattr(obj, 'tags') and 'tags' in data:
obj.update('tags', list(data.get('tags', [])))
if hasattr(obj, 'plugin') and 'plugin' in data:
obj.update('plugin', data.get('plugin'))
dumped = obj.schema.dump(obj)

Copilot uses AI. Check for mistakes.
Comment on lines +154 to 173
# 🧠 Update in-memory object fields
obj.update('name', data.get('name'))
obj.update('description', data.get('description'))

# ✅ THIS is the critical fix
obj.atomic_ordering = data.get('atomic_ordering', [])
# if any(isinstance(s, dict) and "metadata" in s for s in obj.atomic_ordering):
# obj.metadata = {}
self.log.debug(
"[REPLACE] Writing atomic_ordering (len=%s)",
len(obj.atomic_ordering)
)

obj.update('objective', data.get('objective'))
obj.update('tags', list(data.get('tags', [])))
obj.update('plugin', data.get('plugin'))

dumped = obj.schema.dump(obj)


Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

replace_on_disk_object is in the base manager and appears to be used for multiple object types, but it now unconditionally assigns adversary-specific fields (atomic_ordering/objective/tags/plugin). For non-adversary objects, this can raise AttributeError or silently corrupt state. Move this adversary-specific logic into AdversaryApiManager (override replace/update path there), or gate these assignments with hasattr(obj, ...) / schema field presence to keep BaseApiManager generic.

Suggested change
# 🧠 Update in-memory object fields
obj.update('name', data.get('name'))
obj.update('description', data.get('description'))
# ✅ THIS is the critical fix
obj.atomic_ordering = data.get('atomic_ordering', [])
# if any(isinstance(s, dict) and "metadata" in s for s in obj.atomic_ordering):
# obj.metadata = {}
self.log.debug(
"[REPLACE] Writing atomic_ordering (len=%s)",
len(obj.atomic_ordering)
)
obj.update('objective', data.get('objective'))
obj.update('tags', list(data.get('tags', [])))
obj.update('plugin', data.get('plugin'))
dumped = obj.schema.dump(obj)
# Determine which fields are actually defined on this object's schema
schema = getattr(obj, 'schema', None)
schema_fields = set()
if schema is not None and isinstance(schema.__class__, SchemaMeta):
schema_fields = set(schema.fields.keys())
# 🧠 Update in-memory object fields that are expected to be common
obj.update('name', data.get('name'))
obj.update('description', data.get('description'))
# Only apply adversary-specific fields when the object supports them
if 'atomic_ordering' in schema_fields or hasattr(obj, 'atomic_ordering'):
obj.atomic_ordering = data.get('atomic_ordering', [])
self.log.debug(
"[REPLACE] Writing atomic_ordering (len=%s)",
len(obj.atomic_ordering)
)
if 'objective' in schema_fields:
obj.update('objective', data.get('objective'))
if 'tags' in schema_fields:
obj.update('tags', list(data.get('tags', [])))
if 'plugin' in schema_fields:
obj.update('plugin', data.get('plugin'))
dumped = obj.schema.dump(obj)

Copilot uses AI. Check for mistakes.
Comment on lines 90 to +92
async def update_on_disk_object(self, request: web.Request):
data, access, obj_id, query, search = await self._parse_common_data_from_request(request)
try:
data, access, obj_id, query, search = await self._parse_common_data_from_request(request)
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

This changes the API contract for all BaseObjectApi subclasses: when the object isn't found it now returns null (instead of a 404), and the 500 reason string is adversary-specific. Suggest restoring the previous 404 behavior (raise JsonHttpNotFound / web.HTTPNotFound) and using a generic error reason (e.g., "Internal error during object update") to avoid surprising clients and leaking handler-specific wording.

Copilot uses AI. Check for mistakes.
Comment on lines +105 to +110
# raise JsonHttpNotFound(f'{self.description.capitalize()} not found: {obj_id}')
return obj

except Exception as e:
self.log.exception('[update_on_disk_object] Exception occurred: %s', str(e))
raise web.HTTPInternalServerError(reason='Internal error during adversary update')
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

This changes the API contract for all BaseObjectApi subclasses: when the object isn't found it now returns null (instead of a 404), and the 500 reason string is adversary-specific. Suggest restoring the previous 404 behavior (raise JsonHttpNotFound / web.HTTPNotFound) and using a generic error reason (e.g., "Internal error during object update") to avoid surprising clients and leaking handler-specific wording.

Suggested change
# raise JsonHttpNotFound(f'{self.description.capitalize()} not found: {obj_id}')
return obj
except Exception as e:
self.log.exception('[update_on_disk_object] Exception occurred: %s', str(e))
raise web.HTTPInternalServerError(reason='Internal error during adversary update')
raise JsonHttpNotFound(f'{self.description.capitalize()} not found: {obj_id}')
return obj
except Exception as e:
self.log.exception('[update_on_disk_object] Exception occurred: %s', str(e))
raise web.HTTPInternalServerError(reason='Internal error during object update')

Copilot uses AI. Check for mistakes.
Comment thread app/planners/atomic.py

if links_to_use:
# Each agent will run the next available step.
print(f'[Atomic] Running {len(links_to_use)} links with links_to_use: {links_to_use}')
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

Avoid using print in application/server code paths; it bypasses configured logging handlers/levels and can leak operational details. Use the planner/service logger instead (e.g., self.log.debug/info) and consider redacting large/verbose objects from logs.

Copilot uses AI. Check for mistakes.
Comment on lines +431 to +439
# --- Build the link ---
link = Link.load(dict(
paw=agent.paw,
score=0,
ability=ability,
executor=executor,
status=link_status,
jitter=self.jitter(operation.jitter)
))
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

Previously, link creation was guarded by if executor.command:; this change always creates a link even when the executor may not have an executable command/template configured. If executor.command is semantically required for execution, restore the guard (or an equivalent check like if executor.test:) to avoid generating non-runnable links.

Copilot uses AI. Check for mistakes.
# self.log.debug('Unresolved placeholders remain after injection for step idx %s; skipping', idx)
# continue

link.command = self.encode_string(injected)
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

Previously, link creation was guarded by if executor.command:; this change always creates a link even when the executor may not have an executable command/template configured. If executor.command is semantically required for execution, restore the guard (or an equivalent check like if executor.test:) to avoid generating non-runnable links.

Copilot uses AI. Check for mistakes.
self.log.debug('Final injected command: %s', injected)
self.log.debug('Final command: %s', link.command)

links.append(link)
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

Previously, link creation was guarded by if executor.command:; this change always creates a link even when the executor may not have an executable command/template configured. If executor.command is semantically required for execution, restore the guard (or an equivalent check like if executor.test:) to avoid generating non-runnable links.

Copilot uses AI. Check for mistakes.
import marshmallow as ma
import logging
logger = logging.getLogger('adversary')
logger.setLevel(logging.DEBUG)
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

Setting the logger level at import time can unintentionally override application-level logging configuration (e.g., forcing DEBUG in production). Prefer leaving logger levels to centralized logging config and remove the setLevel here (or set it only in a controlled debug/dev configuration path).

Suggested change
logger.setLevel(logging.DEBUG)

Copilot uses AI. Check for mistakes.
@deacon-mp deacon-mp closed this Mar 15, 2026
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.

3 participants