Skip to content

The audit log is an in-memory rolling buffer — entries are lost on restart and silently evicted under load #224

Description

@Jagadeeshftw

Priority: Medium  ·  Area: Audit logging / durability  ·  Est. effort: 7–10 h

📌 Problem

src/middleware/auditLog.ts describes itself as a "rolling buffer, so operators can see recent write activity", implemented as an in-process array:

history.push(redactSensitiveData(entry));   // auditLog.ts:103

Exposed at GET /api/v1/audit per src/openapi.ts:35.

As a debugging aid this is reasonable. As an audit log it has two disqualifying properties:

  1. No durability. A restart destroys the record of every write the service performed. There is no other copy.
  2. Silent eviction. A rolling buffer drops the oldest entries once full. Under sustained write activity — exactly when an audit trail matters most — the earliest evidence is discarded first, with no signal that it happened.

The redaction step (redactSensitiveData) shows the entries are considered sensitive enough to filter, which reinforces that they are meant to be a real record.

🎯 Design decision required

State and defend:

  1. What this is. Decide whether it is an operator convenience or a genuine audit trail, and make the code and docs say so unambiguously. If it stays a convenience buffer, GET /api/v1/audit and its documentation must stop implying completeness. Either answer is acceptable; ambiguity is not.
  2. Durability, if it is an audit trail. Structured stdout for external collection, or the database (tracked separately — the service currently has no persistence)? Structured logging is cheap and needs no schema; a table gives queryability. Argue one.
  3. Eviction visibility. However it ends up, dropping an entry must be observable — a counter, a warning, or a gap marker — not silent.

🧩 Requirements and context

  • redactSensitiveData must apply to any new sink. Verify what it actually redacts and report whether the coverage is adequate — anything reaching a durable sink is far harder to retract than something in a ring buffer.
  • The GET /api/v1/audit response shape should stay stable unless you argue otherwise.
  • Tests must run with no external dependency.
  • Do not silently increase the buffer size and call it fixed.
  • src/middleware/auditLog.test.ts exists — extend it.

🛠️ Suggested execution

  1. Read redactSensitiveData and report its actual coverage.
  2. Write a test proving entries are lost on eviction and that nothing reports it.
  3. Implement your decision.
  4. Add a test for eviction/overflow visibility.
  5. Update src/openapi.ts if the endpoint's guarantees change.

✅ Acceptance criteria

  • The PR states whether this is a convenience buffer or an audit trail, and the code and docs agree.
  • A test proves the current silent-eviction behaviour.
  • Eviction or overflow is observable after the change, with a test.
  • redactSensitiveData coverage is reported, with any gap called out.
  • If it became durable, the sink is documented for operators.
  • GET /api/v1/audit documentation matches actual guarantees.
  • npm run lint, npm run build and npm test pass.

🚫 Out of scope

  • Building a log-shipping pipeline.
  • The persistence layer — separate issue, though you may build on it.
  • Adding new audited events.

🧪 Verification

npm ci
npm test src/auditLog.test.ts src/middleware/auditLog.test.ts
npm run lint && npm run build && npm test

📤 What your PR must include

  1. What redactSensitiveData actually covers, and any gap.
  2. Your convenience-vs-audit-trail decision.
  3. Your durability choice and its reasoning.
  4. The eviction-visibility test.
  5. Closes #<n>.

🔒 Security notes

An audit log that silently discards its oldest entries under load fails exactly when it is needed: sustained anomalous write activity is both the thing worth investigating and the thing that evicts the evidence of how it started. If the log becomes durable, redaction becomes materially more important — a sensitive value in a ring buffer disappears on its own, whereas one written to a durable sink persists and propagates.

📋 Guidelines

  • Minimum 95% test coverage on changed lines
  • Clear documentation
  • Timeframe: 96 hours from assignment
  • One logical change per commit; no merge commits

💬 Join our community

Working on this, or want to sanity-check your approach before you start? Come and ask — the maintainers are there and happy to help.

Telegram: https://t.me/Grainlify

Activity

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

Metadata

Metadata

Labels

GrantFox OSSGrantFox open-source programMaybe RewardedGrantFox: potentially rewarded contributionThird CampaignGrantFox third campaign issuepriority:mediumMedium difficulty / self-contained but non-trivialsecuritySecurity hardening

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions