Skip to content

Add an MCP server for askgod at <askgod_server_address>/mcp.#9

Closed
Res260 wants to merge 9 commits intonsec:mainfrom
Res260:mcp
Closed

Add an MCP server for askgod at <askgod_server_address>/mcp.#9
Res260 wants to merge 9 commits intonsec:mainfrom
Res260:mcp

Conversation

@Res260
Copy link
Copy Markdown
Contributor

@Res260 Res260 commented Mar 15, 2026

Add an MCP server for askgod at <askgod_server_address>/mcp.

It has two methods: one to submit flags and one to list the scoreboard.
The list scoreboard method contains a small prompt injection to troll the players.

A new column is added to the scores table: source. Its purpose is to track if the flag was submitted using the REST API or using MCP.

The askgod admin list-scores aud askgod history commands were changed to display the source of each flag.

Note: This PR was mainly vibecoded using Claude Code, but I reviewed all of it. However, I'm not a golang expert so a thorough review would be appreciated.

@Res260 Res260 force-pushed the mcp branch 5 times, most recently from 2b3a570 to fe84c16 Compare March 15, 2026 21:25
@Res260
Copy link
Copy Markdown
Contributor Author

Res260 commented Mar 15, 2026

@stgraber Le lint semble chialer sur des fichiers que j'ai pas touchés, sais-tu pourquoi?

@stgraber
Copy link
Copy Markdown
Member

@Res260 try rebasing now. I've just pushed a whole bunch of fixes to modernize things and sort out all the newer lint.

@Res260 Res260 force-pushed the mcp branch 6 times, most recently from 2b402c2 to 43f79a0 Compare March 21, 2026 18:34
@Res260
Copy link
Copy Markdown
Contributor Author

Res260 commented Mar 21, 2026

@stgraber everything should be good now

Comment thread api/flag.go Outdated
@stgraber
Copy link
Copy Markdown
Member

Other than the API change mentioned above, this is going to need quite a few updates:

  • This should be split into quite a few separate commits, off the top of my head, I'd say:
    • Docker compose change
    • Moving of the IP logic into utils
    • Updating of existing code following the move of the function to the utils package
    • Introduction of the new API fields in the API package
    • DB schema update
    • DB functions update
    • CLI update for the new field
    • Introduction of the MCP logic

On the styling front, our normal pattern for commands is to have them start with a capital letter and end with a period. It looks like all the newly introduced comments fail to follow the existing pattern.

We've also recently been putting http.MaxBytesReader in place everywhere we read user provided data, the MCP endpoints should use that too.

I also am quite concerned about having validation logic duplicated in both the REST and MCP endpoints, especially things like scoreboard filtering or the validity of a team who's attempting to submit a flag.

As someone with zero interest in the MCP stuff, I'm not likely to remember to update that code so we may well end up with submissions allowed under MCP which aren't on REST, that would be bad. My preference would be for the MCP stuff to put together a regular http.Request and call the REST endpoint internally so it would be guaranteed to go through the exact same logic as a normal REST query.

If that's impractical, then the logic should be extracted from the REST endpoint and be put somewhere that can be called from both REST and MCP with neither of the handlers performing any additional validation on the submitted data.

I also think that the trolling/nagging logic probably ought to be configurable.
I'm not aware of any use of Askgod outside of NorthSec but if it is used externally, the nagging/trolling may not be desired in that environment.

Oh and speaking of configuration, the entire MCP stuff should be an optional feature that needs to be enabled in the server configuration.

@Res260 Res260 force-pushed the mcp branch 3 times, most recently from 53a5e31 to 5ae26c3 Compare March 22, 2026 23:47
@Res260
Copy link
Copy Markdown
Contributor Author

Res260 commented Apr 2, 2026

Update: J'ai refactor le tout. Le au lieu de Source dans le score table, ça sera un bool ai_agent qui peut aussi être activé via le askgod client via --agent ou autodétecté via des variables d'environnement communes d'AI Agents (comme CLAUDECODE=1). L'autodétection est désactivable via variable d'environnement ou --no-ai-autodetect.

This should be split into quite a few separate commits

Still todo, je ferai à la fin quand le review sera fini, idem pour les signoff

On the styling front, our normal pattern for commands is to have them start with a capital letter and end with a period. It looks like all the newly introduced comments fail to follow the existing pattern.

De quelle commande on parle? Je sais pas si c'est encore pertinent suite au refactor.

We've also recently been putting http.MaxBytesReader in place everywhere we read user provided data, the MCP endpoints should use that too.

Fixed

I also am quite concerned about having validation logic duplicated in both the REST and MCP endpoints, especially things like scoreboard filtering or the validity of a team who's attempting to submit a flag.
As someone with zero interest in the MCP stuff, I'm not likely to remember to update that code so we may well end up with submissions allowed under MCP which aren't on REST, that would be bad. My preference would be for the MCP stuff to put together a regular http.Request and call the REST endpoint internally so it would be guaranteed to go through the exact same logic as a normal REST query.

Done, c'est en effet plus clean

I also think that the trolling/nagging logic probably ought to be configurable. I'm not aware of any use of Askgod outside of NorthSec but if it is used externally, the nagging/trolling may not be desired in that environment.

Retiré altogether

Oh and speaking of configuration, the entire MCP stuff should be an optional feature that needs to be enabled in the server configuration.

Done

Donc j'attend le review final et je retravaille les commits

@stgraber
Copy link
Copy Markdown
Member

stgraber commented Apr 3, 2026

Ça me prend le re-shuffle des commits pour pouvoir faire le review.

Je review toujours commit par commit dans l'ordre et m'attend généralement à ce que le codebase soit compilable à chacun de ces points pour pouvoir être capable de bisect dans le future sans avoir une batch de commits brisés qui font skipper le bisect.

Res260 added 7 commits April 3, 2026 13:42
…e `COPY` step.

docker-compose: restart askgod-server on failure
seed-data: fix seeded IPs so they work with Docker

Signed-off-by: Émilio Gonzalez <little.moon6016@fastmail.com>
Signed-off-by: Émilio Gonzalez <little.moon6016@fastmail.com>
Signed-off-by: Émilio Gonzalez <little.moon6016@fastmail.com>
…rver

Signed-off-by: Émilio Gonzalez <little.moon6016@fastmail.com>
…od history`

Signed-off-by: Émilio Gonzalez <little.moon6016@fastmail.com>
… to `askgod submit` (based on various AI Agent’s environment variables, this can be disabled using `--no-ai-autodetect` or using the `ASKGOD_DISABLE_AGENT_AUTODETECT` environment variable).

Signed-off-by: Émilio Gonzalez <little.moon6016@fastmail.com>
It has only one tool: Submit a flag.
The MCP server is disabled by default and can be enabled with `mcp: true` in the askgod config.
Internally, it uses the REST method calls to limit code duplication to a minimum.

Signed-off-by: Émilio Gonzalez <little.moon6016@fastmail.com>
@Res260
Copy link
Copy Markdown
Contributor Author

Res260 commented Apr 3, 2026

@stgraber C’est fait! J’ai également retiré le refactor de la méthode qui retournait le Team selon le IP, les diffs sont pas mal plus clean sans ce refactor.

Res260 added 2 commits April 5, 2026 17:08
Signed-off-by: Émilio Gonzalez <little.moon6016@fastmail.com>
Signed-off-by: Émilio Gonzalez <little.moon6016@fastmail.com>
value INTEGER NOT NULL DEFAULT 0,
submit_time TIMESTAMP WITH TIME ZONE,
notes VARCHAR,
ai_agent BOOLEAN NOT NULL DEFAULT FALSE,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think a source field as a string would be more flexible in case we ever want to differentiate other flag submission sources.

Comment thread api/flag.go
Description string `json:"description" yaml:"description"`
ReturnString string `json:"return_string" yaml:"return_string"`
Value int64 `json:"value" yaml:"value"`
AIAgent bool `json:"ai_agent" yaml:"ai_agent"`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd make that to be Source here too as a string.

Comment thread api/score.go
AdminScorePost `yaml:",inline"`

ID int64 `json:"id" yaml:"id"`
AIAgent bool `json:"ai_agent" yaml:"ai_agent"`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should be Source on this one too

Comment thread api/flag.go

Flag string `json:"flag" yaml:"flag"`
Flag string `json:"flag" yaml:"flag"`
AIAgent bool `json:"ai_agent" yaml:"ai_agent"`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you make this Source as well but we'll want to limit the number of valid values to:

  • "" => compatibility mechanism, we'll translate it to "unknown" internally
  • "cli" => human submission through CLI
  • "cli+agent" => agent submission through CLI
  • "mcp" => submitted through the MCP server
  • "web" => human submission through website
  • "web+agent" => agent submission through website

That should cover all the cases we care about while giving us some flexibility in the future.

metricSubmitTeam.WithLabelValues(strconv.FormatInt(team.ID, 10), "invalid").Inc()
_ = r.eventSend("flags", api.EventFlag{Team: *team, Input: flag.Flag, Type: "invalid"})
logger.Info("Invalid flag submitted", log15.Ctx{"teamid": team.ID, "flag": flag.Flag})
logger.Info("Invalid flag submitted", log15.Ctx{"teamid": team.ID, "ai-agent": flag.AIAgent, "flag": flag.Flag})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

all of those will want to use the "source" field instead


table := tablewriter.NewWriter(os.Stdout)
table.SetHeader([]string{"ID", "TeamID", "FlagID", "Value", "Submit time", "Notes"})
table.SetHeader([]string{"ID", "TeamID", "FlagID", "Value", "Submit time", "AI Agent", "Notes"})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will be Source too

Comment thread cmd/askgod/cmd_history.go

table := tablewriter.NewWriter(os.Stdout)
table.SetHeader([]string{"ID", "Description", "Value", "Timestamp", "Message", "Notes"})
table.SetHeader([]string{"ID", "Description", "Value", "Timestamp", "AI Agent", "Message", "Notes"})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same

Comment thread cmd/askgod/main.go
Comment on lines +238 to +241
&cli.BoolFlag{
Name: "no-ai-autodetect",
Sources: cli.EnvVars("ASKGOD_DISABLE_AGENT_AUTODETECT"),
Usage: "Disable automatic AI agent detection",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wouldn't do that

Comment thread cmd/askgod/main.go
Comment on lines +234 to +237
&cli.BoolFlag{
Name: "agent",
Usage: "Flag was found mostly or entirely with an AI agent",
},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't remember this particular CLI library but with spf13/cobra at least, you can make a flag as a tri-state. So being able to check if:

  • Flag was passed (value is true)
  • Flag was passed with =false (value is false)
  • Flag wasn't passed (value is false)

So you'd then use --agent=false as the way to do what your separate --no-ai-autodetect is currently doing

Comment thread cmd/askgod/cmd_submit.go
flag := api.FlagPost{}
flag.Flag = cmd.Args().Get(0)
flag.Notes = cmd.String("notes")
flag.AIAgent = cmd.Bool("agent")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Given the previous changes I requested, we can probably track that as just flag.Agent instead of flag.AIAgent so we use more consistent terminology

Comment thread api/score.go
FlagID int64 `json:"flag_id" yaml:"flag_id"`
TeamID int64 `json:"team_id" yaml:"team_id"`
FlagID int64 `json:"flag_id" yaml:"flag_id"`
AIAgent bool `json:"ai_agent" yaml:"ai_agent"`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Source

@stgraber
Copy link
Copy Markdown
Member

stgraber commented May 1, 2026

@Res260 is not interested in doing this the way that's been suggested above, closing and taking over

@stgraber stgraber closed this May 1, 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.

2 participants