Skip to content

TICKET-535: Add audit logging - #17

Open
Naragod wants to merge 2 commits into
mainfrom
TICKET-535_log_intrusion_detection
Open

TICKET-535: Add audit logging#17
Naragod wants to merge 2 commits into
mainfrom
TICKET-535_log_intrusion_detection

Conversation

@Naragod

@Naragod Naragod commented Jul 2, 2026

Copy link
Copy Markdown

Description

Three issues with the current implementation:

  1. There is zero monitoring. If someone calls AI-Server server many times with incorrect API keys, we would never know as nothing is recorded.
  2. The server throws away the instructions MarkUs sends with each request. The AI never sees them, meaning teachers' feedback is ignored.
  3. A wrong API key makes the server crash with 500 errors.

Implementation

  1. The server now writes every failed key try into a log book (Loki). Each entry has the caller's address, but never the key itself. Two alarms watch the log book and send an email:
    1. One address tries more than 10 wrong keys in 5 minutes
    2. More than 50 wrong keys arrive from anywhere in 5 minutes.
    3. A new setting (TRUSTED_PROXY_HOPS) stops callers from faking their address.
  2. The server now reads the system_instructions field that MarkUs sends and passes it to the AI.
  3. A wrong key now gets a clean 401 Invalid API key answer.

@Naragod
Naragod requested a review from donny-wong July 2, 2026 02:36

@donny-wong donny-wong 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.

Excellent work @Naragod. I see this is configured/developed for local testing — anonymous Grafana admin, insecure TLS, Mailpit SMTP, service endpoints on Compose DNS names. How do you plan on configuring for production? Should we have separate settings files for that, or is prod deployment handled elsewhere?

@donny-wong donny-wong 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.

Clarified with @Naragod that we will need to work with sysadmin for production deployment configurations.

@donny-wong
donny-wong requested a review from david-yz-liu July 10, 2026 16:09

@david-yz-liu david-yz-liu 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.

@Naragod this PR incorporates a few different changes that I would like you to pull out into separate PRs, making them easier to review.

  1. The fix for passing in system_instructions.
  2. The fix for the API key.
  3. The change to the debug mode for running the server.
  4. Changes to the README for using uv instead of pip (which is a good change to make!).
  5. Adding pytest-cov as a dependency.

After those get merged in I'll review the meat of this PR.

@david-yz-liu david-yz-liu mentioned this pull request Jul 13, 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