Guide 052 template

How to write a developer-friendly vulnerability comment

Write a vulnerability comment that explains the affected path, useful evidence and next action, then keep the discussion accurate as the candidate changes.

Start with the harmful action and affected asset, then show the exact path, the failed control, the conditions under which the problem matters, and the change that would close it. End with a check the developer and reviewer can perform on a new revision. If the path or deployment is still uncertain, write a review question instead of asserting a confirmed vulnerability. A developer should be able to locate the behavior and challenge the claim without decoding a rule ID or reading a scanner transcript. OWASP's Web Security Testing Guide says a useful report should identify the affected function, explain the issue in language developers understand and give enough detail to retest it.

This template is for a concise PR or ticket comment after the security engineer has checked the evidence. Keep sensitive source excerpts, tokens and reproduction detail in an authorized technical view when the comment's audience is broader.

The comment in six parts

Part Write this Avoid this
Title Actor + harmful action + asset: “A log reader may obtain a live password-reset token.” “Critical CWE in logReset().”
Scope Repository, immutable revision, operation and deployment evidence. A floating branch name or screenshot without revision.
Why Expected rule, observed path and exact failed control. “The scanner says this is exploitable.”
Effect and difficulty What could happen, who could do it and prerequisites still unknown. A severity label used as proof of access.
Fix direction The invariant the code must enforce, with a likely owner. A patch that assumes the engineer's architecture.
Acceptance Positive evidence at a new revision that the unsafe path is prevented. “Alert disappeared; close ticket.”

The first sentence should be readable without code knowledge. Add the technical trace beneath it. The OWASP reporting chapter separates management-level explanation from technical findings and asks for clear affected items, issue descriptions and remediation. A short developer comment can follow that order without becoming a full report.

Illustrative case: reset tokens in logs

Illustrative scenario, with invented organization, paths, revision and records: Harbor Desk lets a user request a password-reset link. At revision c8a42f, src/account/reset.ts:72-91 creates a single-use token and passes an object containing it to a generic event logger. The logger in src/telemetry/events.ts:30-43 serializes the whole object. For the writing exercise, assume an approved evidence record supplies the caller, serializer, production configuration and a controlled synthetic result: the synthetic token appears in the test log, and the production configuration uses the same serializer and sink without redaction. These are stipulated case facts, not checks performed for this article. No real token belongs in the comment. Historical token validity and log readership remain unknown.

OWASP's Logging Cheat Sheet identifies secrets and access tokens among data that should normally be excluded from application logs. Its Forgot Password Cheat Sheet treats reset tokens as one-use, expiring credentials. The comment therefore needs to distinguish the supplied synthetic logging observation from the conditional risk that someone could use a still-valid token.

Edit the report down to the developer's decision

A noisy first attempt might read: “Critical CWE-532, possible account takeover, 27 trace nodes, see attached JSON. Remove logging immediately.” It loses the actor, treats possible harm as observed harm, and suggests removing diagnostics the on-call team needs. Copying the entire evidence packet below it would make the comment longer without repairing those decisions.

Here is a concise restricted comment for Harbor Desk's already approved issue:

Reset credentials reach the event log at c8a42f. The reset handler passes token to the whole-object serializer; evidence record RST-12 contains the reviewed path and supplied synthetic result. A log reader could use a still-valid token; production readership and misuse are unknown. Please remove credentials from normal and error events while retaining a non-secret request ID and outcome. At the new revision, check both event paths and every configured sink with synthetic data: no token, useful event still present, legitimate reset still works. AppSec owns fix reassessment; Operations owns retained-log exposure separately.

That comment leaves three kinds of material in the restricted evidence record: exact source excerpts and complete trace, configuration and deployment provenance, and the controlled-check receipt. It retains the uncertainty about log access and token validity because that changes how the developer understands the harm. Shortening a comment must not turn conditional impact into certainty.

The expected implementation is an invariant, not a guessed patch: credentials must not enter any event sink. Removing one field at the main call is insufficient if the exception handler serializes the original request object. Conversely, the engineer can implement a safe event interface instead of following a suggested line edit. Keep the normal event's request ID and outcome; a check in which neither the token nor the event appears has not proved useful logging survives.

Continue the conversation without repeating the report

Developer: “These tokens expire after ten minutes. Can we close this without changing the logger?”

Reviewer: “The lifetime limits the window; it does not stop the credential reaching logs while valid. The fix is still to exclude it on normal and error paths. Operations will assess retained logs, actual access and token validity separately. We have not established misuse.”

Ten minutes is a supplied case assumption, not an OWASP recommendation. The source guidance requires appropriate expiry and single use; it does not make credential logging acceptable for a short period.

Developer: “I removed token from the success event. We still need the original error object for debugging.”

Reviewer: “Please inspect whether that error object contains the request or reset token before serialization. Preserve a safe error code, request ID and outcome instead. RST-12's acceptance sheet includes both a successful reset request and the error path; the first change covers only one.”

Developer: “The new event interface accepts only those safe fields. Here is the new revision and the proposed check record.”

Reviewer: “Ready for reassessment. I will check the interface's callers and configured sinks; the issue remains open until that evidence supports closure. This does not close Operations' historical-exposure work.”

Each reply changes one decision. None needs to paste the scanner transcript again or announce an unsupported account takeover.

Use a four-sentence drafting constraint

Try this structure for the first comment, then expand only where ambiguity would change the requested action:

  1. Problem: At this revision, this actor or data flow reaches this prohibited effect; here is the controlled evidence reference.
  2. Consequence and limit: This could affect this asset under these prerequisites; this material fact remains unknown.
  3. Requested behavior: Enforce this rule while preserving this legitimate operation.
  4. Closure: Inspect these changed paths and positive/negative cases at the new revision; this other response has a separate owner.

For Harbor Desk, the reader can locate the logger, understand why expiry is insufficient, preserve useful diagnostics and prepare a meaningful closure packet. A full accepted fix ticket can carry estimates, dependencies and deployment steps; the comment's job is to make the next engineering response obvious.

When the claim is not ready for a definitive comment

If you only see a token added to an event object but cannot inspect the serializer or sink, leave a narrow investigation question: “At revision c8a42f, the reset event includes token. Does the logger redact that field before any sink receives it? Please link the redaction rule and a synthetic check.” Attach the source path, assign an owner and set a review date. Keep the claim unresolved. If omitting the credential field is straightforward and preserves diagnostics, the developer can make that protective change while the sink question is answered.

If a mandatory redactor removes the token on every path, record that counterevidence and reject this specific claim. If the log contains the token but the token expires quickly, the lifetime reduces exposure without making the logging acceptable. If a patch removes the token from the main event but leaves it in error logs, the issue remains open. A code change, deleted file or missing alert does not prove the harmful flow is gone.

Keep one conversation attached to the changing evidence

The comment is also a small change-management record. In the HarborDesk example, the author should be able to tell whether the reviewer is discussing the original serializer or the latest replacement. Start a follow-up with the new revision and the remaining behavior: “At the revised handler, the success path now emits the approved event fields. The exception path still passes the response object to the diagnostic helper.” That acknowledges the repair without treating the whole concern as resolved.

Avoid repeating the original comment unchanged under every new diff. Repetition makes the developer reconstruct which portion still applies and can make a narrow residual concern look like several separate findings. Keep a stable reference to the original question, then update the current summary. If the platform hides an outdated line comment, place a brief current-state note in the review's main discussion with the evidence link. This is a communication convention, not a claim about a particular review tool's automation.

A useful final exchange has three parts:

Participant Concrete contribution Boundary of that contribution
Author Identifies the candidate revision and shows both success and exception events using the approved fields Reports what changed; does not infer production rollout from the diff
Reviewer Checks the credential-bearing values cannot enter the reviewed logging calls and records the limits of the check Supports the reviewed paths, not every log producer in the system
Release owner Connects the candidate to the deployed artifact and identifies remaining older instances Supplies rollout evidence, not a retrospective statement that old logs contained no credentials

For this exercise, those are requested contributions rather than completed observations. A review comment can be resolved once its agreed code-review question is answered, while a linked release or historical-data task remains open. State that distinction in the resolution note so a closed thread does not become accidental evidence that every consequence was addressed.

Make the requested reply safe and inexpensive

Ask for the smallest evidence that settles the question. A synthetic event with field names and harmless values can show the serializer's shape. The reviewer generally does not need a production password-reset URL pasted into a PR. If access to an existing log is necessary for a separate investigation, use the approved restricted evidence channel and describe the needed observation in the public thread.

The developer may discover that removing the response object also removes a field required for operational diagnosis. Treat that as a design constraint to solve: identify the specific non-secret field, its purpose and the approved event interface. “Keep everything for debugging” is too broad, but “delete every diagnostic event” may create avoidable operational harm. The useful comment gives the owner room to preserve necessary observability while excluding the credential.

Before resolving, read the comment as a future maintainer. It should explain why the event interface is intentionally limited, which paths were reviewed and what would reopen the question. A later developer adding a new exception handler then has a concrete rule to preserve rather than a mysterious instruction to appease a scanner.

For the next security comment, draft the plain first sentence before adding a rule or severity label. Then check that the short comment covers the necessary facts and lets the assigned developer locate the path and state the acceptance check. If the claim itself is still in doubt, start with the triage gate before a developer ticket. For a preliminary question on a PR, use the guide to an actionable code review comment.