mirror of
https://github.com/FOSSBilling/FOSSBilling.git
synced 2026-10-23 03:37:01 +09:00
Add security-advisory-triage skill (#4232)
* Add security-advisory-triage skill Codifies the two-stage workflow for handling a reported GitHub Security Advisory: verify it against current code with file:line evidence, then refine its title/description/CVSS/CWE metadata to match this repo's actual publish conventions before applying it. Includes a living house-style reference derived from this repo's own published advisories. Lives at .agents/skills/ with .claude/skills symlinked to it, matching the existing AGENTS.md/.claude/CLAUDE.md convention for cross-tool agent support. * Fix spellcheck: add GHSA/macrovector/rubygems, ignore skills reference dir The skills reference file legitimately contains GHSA advisory IDs as provenance examples; those are opaque identifiers, not real words, and new ones will keep appearing as more advisories get referenced there over time, so excluding the directory from spellcheck avoids repeated dictionary churn rather than fixing it once. * Address CodeRabbit review on security-advisory-triage skill - Stage 1's current-code check now requires an exact revision match (fetch + clean worktree + exact rev-parse comparison) instead of only an ancestor check, which could pass against a stale or divergent base. - Stage 1's related-history step now queries each advisory state explicitly with --method GET, and calls out that gh api silently sends a POST (hitting the advisory-creation endpoint) when -f is used without --method GET. Verified directly against this repo: an unfiltered call does return every state for a maintainer-authenticated token, but explicit per-state queries are more portable. - Confidentiality note broadened to cover chat/review/tool output, not just committed artifacts. - CVSS guidance now distinguishes the nested read path (cvss_severities.cvss_v3/cvss_v4) from the flat write field (cvss_vector_string), and notes severity/cvss_vector_string are mutually exclusive on write. - The live-advisory PATCH checklist now lists the actual top-level fields (summary, description, cwe_ids, cvss_vector_string/severity) instead of vaguely saying 'the description body', notes vulnerabilities is an array rather than assuming index [0] is everything, and says to PATCH the complete object per entry rather than a partial one. - Added language tags to two fenced code blocks (shell, markdown) per markdownlint MD040. * Fix data-loss gap in vulnerabilities PATCH guidance The 'send the complete object' instruction added last round listed only four fields (package.ecosystem/name, vulnerable_version_range, patched_versions) and omitted vulnerable_functions, which is a real field on the object (confirmed: this session's own successful PATCH sent it as an empty array). Following that guidance literally on an advisory where vulnerable_functions is actually populated would erase it. Added the missing field.
This commit is contained in:
1 parent
84ed803bb7
commit
1420584f6d
4 files changed
+466
-2
No files matched your search
@@ -0,0 +1,189 @@
|
||||
---
|
||||
name: security-advisory-triage
|
||||
description: Verify a reported GitHub Security Advisory (GHSA) against this repo's actual current code, then refine its title/description/CVSS/CWE metadata into this project's house style and apply it to the live advisory. Use this whenever the user gives you a GHSA ID, a security advisory number, a security report to "check", "verify", "triage", or "confirm", or asks whether a vulnerability report is valid/real/still exploitable on main. Also use it when the user wants an advisory's writeup improved, its CVSS score computed or recalculated, its CWEs corrected, or its title/description brought in line with how this repo normally publishes advisories, even if they only ask for one of those pieces (e.g. "recompute the CVSS for GHSA-xxxx") rather than the full workflow. Don't wait for the user to ask for "the process" by name; if they hand you a GHSA id or a raw vulnerability report against this codebase, this skill is almost certainly what they want.
|
||||
---
|
||||
|
||||
# Security advisory triage
|
||||
|
||||
This is a two-stage workflow: first establish, with evidence, whether a reported
|
||||
vulnerability is real against the code as it stands today; only then move on to
|
||||
making the advisory itself good. Don't skip straight to writing prose about a bug
|
||||
you haven't confirmed exists.
|
||||
|
||||
Both stages are collaborative and iterative, not something to run to completion
|
||||
and dump on the user at the end. The reason is simple: writing a security advisory
|
||||
involves a lot of small judgment calls (how far back does "affected versions" really
|
||||
go, is this workaround actually true, does this deserve its own CWE), and the user
|
||||
catching a wrong assumption early is much cheaper than catching it after you've
|
||||
built five more paragraphs on top of it. Work through it piece by piece, show your
|
||||
reasoning, and let the user correct course before you continue.
|
||||
|
||||
## Stage 1: Verify the report against current code
|
||||
|
||||
Goal: reach a confident, evidence-backed valid/invalid verdict, not "this looks
|
||||
plausible."
|
||||
|
||||
1. **Fetch the advisory.** `gh api /repos/{owner}/{repo}/security-advisories/{ghsa_id}`
|
||||
gives you the reporter's summary, description, severity, CVSS, CWEs, and state
|
||||
(`triage`/`draft`/`published`/`closed`). Read the whole thing before touching code.
|
||||
|
||||
2. **Confirm you're looking at current code, exactly.** Fetch the branch the
|
||||
report claims to target (`git fetch origin main`, or whatever branch is
|
||||
relevant) and require a clean worktree (`git status --porcelain` empty).
|
||||
Then compare revisions exactly: `git rev-parse HEAD` must equal
|
||||
`git rev-parse origin/main`. An ancestor check alone (`git merge-base
|
||||
--is-ancestor origin/main HEAD`) isn't enough. It passes even if
|
||||
`origin/main` is stale relative to the actual remote, or if your checkout
|
||||
has extra local commits the report never saw, and it doesn't catch
|
||||
uncommitted changes either. A "verified" conclusion built on the wrong code
|
||||
state isn't a verification at all, so make the exact check a reflex.
|
||||
|
||||
3. **Trace the full path, not just the quoted lines.** Reporters usually quote two
|
||||
or three lines as evidence. Read those lines for real, but don't stop there.
|
||||
Follow the code end to end: where does the request enter, what's the actual
|
||||
vulnerable logic, and how does the response or exception actually get back to
|
||||
the caller? That last part is often what reports skip, and a bug that looks
|
||||
real at the "sink" can turn out to be redacted, caught, or rate-limited
|
||||
somewhere in the response path the reporter never looked at. In this codebase
|
||||
specifically, API exceptions flow through `Box\Mod\Api\Controller\Client::tryCall()`
|
||||
into `FOSSBilling\Http\ApiResponseFactory`, which is worth checking on any
|
||||
report about information disclosure via error messages. Also check whatever
|
||||
the report claims is *missing* (rate limiting, CAPTCHA, timing normalization,
|
||||
authorization checks) by grepping/reading for it yourself. Absence claims are
|
||||
exactly the kind of thing worth double-checking, not assuming.
|
||||
|
||||
4. **Pull related history for context.** Query every advisory state
|
||||
explicitly rather than trusting an unfiltered call: `gh api --method GET
|
||||
/repos/{owner}/{repo}/security-advisories -f state=<published|draft|triage|closed>
|
||||
--paginate`, once per state. A maintainer-authenticated session may find the
|
||||
unfiltered call already returns everything (it did when this was tested
|
||||
directly against this repo), but that can depend on token permissions, so
|
||||
don't rely on it. `--method GET` matters here: `gh api` defaults to POST
|
||||
whenever you pass `-f` without it, which for this endpoint hits the
|
||||
advisory-*creation* path instead and fails outright, so it's worth getting
|
||||
right the first time rather than debugging a confusing error. Skim titles
|
||||
for anything thematically close to the current report and read the close
|
||||
ones in full. This surfaces real things: a prior closed report about a
|
||||
related bug class, or a PR that hardened sibling code but happened to miss
|
||||
the exact path being reported now. That kind of context turns "here's a
|
||||
bug" into "here's why it slipped through," which is both more convincing
|
||||
and useful for Stage 2's Details section later.
|
||||
|
||||
5. **If unit tests exist for the relevant code, read them.** They're often the
|
||||
fastest, most authoritative way to confirm exact behavior (exception messages,
|
||||
config-flag semantics, order of operations) instead of inferring it from
|
||||
reading application code alone.
|
||||
|
||||
6. **Conclude with an explicit verdict**, laid out so each of the reporter's
|
||||
claims is checked against a specific file:line, not just asserted as "confirmed."
|
||||
A table works well for this. If something in the report doesn't hold up
|
||||
(wrong line numbers, a claimed-missing mitigation that actually exists,
|
||||
a severity that seems off), say so plainly. The point of this stage is an
|
||||
honest verdict, not validating the reporter's framing.
|
||||
|
||||
**Confidentiality note:** if the advisory is unpublished (`published_at: null`,
|
||||
state `triage`/`draft`), treat the GHSA ID and exploit details as sensitive
|
||||
everywhere, not just in committed artifacts. Don't put them in public commit
|
||||
messages, PR titles/bodies, branch names, or CI run titles; this has actually
|
||||
leaked before. The same caution applies to chat, code review comments, and any
|
||||
other tool output: only share specifics with a requester and channel you've
|
||||
actually confirmed are private to this advisory's disclosure. Keep working
|
||||
notes in a scratchpad or a private location instead.
|
||||
|
||||
## Stage 2: Refine the advisory and apply it
|
||||
|
||||
Only start this once Stage 1 has produced a real "yes, this is valid" verdict.
|
||||
|
||||
### Use the house style, and keep it current
|
||||
|
||||
`references/house-style.md` already has the FOSSBilling template (section
|
||||
order, per-section rules, voice, CVSS/CWE/title conventions), derived by
|
||||
sampling this repo's own *published* advisories rather than assuming the
|
||||
reporter's raw submission format is the target. Read it before drafting
|
||||
anything; it holds the detail so this file doesn't have to repeat it.
|
||||
|
||||
It's a living document, not a fixed spec. If a new advisory surfaces
|
||||
something it doesn't cover, or the user corrects an assumption it makes,
|
||||
update it in place and log the change under its Revisions section, the same
|
||||
way it got built in the first place. If you ever suspect it's drifted from
|
||||
reality (a new published advisory breaks one of its rules), re-derive by
|
||||
sampling directly:
|
||||
|
||||
```shell
|
||||
gh api /repos/{owner}/{repo}/security-advisories --paginate
|
||||
# then fetch several state=published ones in full and diff their structure
|
||||
```
|
||||
|
||||
### Work section by section
|
||||
|
||||
Draft one section, show it, take the feedback, apply it, move to the next.
|
||||
Don't write the whole thing and ask for one blanket approval. By the time you've
|
||||
written seven sections, an early wrong assumption has propagated through all of
|
||||
them, and the user has to hold the whole draft in their head to catch it. A
|
||||
tight loop on individual sections is much cheaper to fix and cheaper to review.
|
||||
|
||||
Things worth doing before the user has to point them out, because the pattern
|
||||
already caught real mistakes when it wasn't done:
|
||||
|
||||
- **Verify every factual claim in the writeup against real code or tests**,
|
||||
don't just write what sounds plausible and flag it for the user to check.
|
||||
Config key names, exact admin-UI label text, and whether a suggested
|
||||
workaround genuinely behaves as described are all things you can grep and
|
||||
read yourself. If you catch yourself writing "I believe this is called X,"
|
||||
stop and go check.
|
||||
- **Ground "Affected Versions" in real git history**, not guesswork. Reporters
|
||||
usually only tested the current release. `git fetch --unshallow` first if
|
||||
the checkout is shallow (cheap, do it by default, worth checking with
|
||||
`git rev-parse --is-shallow-repository`), then `git log -S"<distinguishing
|
||||
string>" -- <file>` to find where the vulnerable logic actually appeared.
|
||||
Also check whether a later, unrelated hardening pass should have covered
|
||||
this exact path but missed it. That's both a strong version-range signal
|
||||
and good material for the Details section. If history hits a genuine wall
|
||||
(a rename/rewrite commit that shows the whole file as newly added) and the
|
||||
pattern already existed at that wall, that's a legitimate stopping point:
|
||||
state the range as confirmed rather than manufacturing false precision by
|
||||
digging further with no possible payoff.
|
||||
- **Compute CVSS with an actual tool**, not by hand. CVSS v4 in particular is
|
||||
scored via a nested lookup table (the "macrovector"), not a simple formula,
|
||||
and is easy to get wrong from memory. Python's `cvss` package
|
||||
(`from cvss import CVSS4`) is normally available; use it. Reason through
|
||||
each base metric explicitly before computing (see `references/house-style.md`
|
||||
for FOSSBilling's specific answer on which CVSS version this repo publishes).
|
||||
- **Don't restate CVSS in the description body.** It belongs solely in the
|
||||
advisory's dedicated `cvss_vector_string` field.
|
||||
- **Pick CWEs by mechanism, not vibes.** A message-content oracle and a
|
||||
timing oracle are different CWEs (204 vs. 208) even though they're both
|
||||
"enumeration" bugs in casual conversation. Get the mechanism right. Check
|
||||
whether a similar past advisory in this repo used a particular CWE pairing
|
||||
worth following as precedent.
|
||||
|
||||
### Voice
|
||||
|
||||
Technical but approachable, precise on file paths/method names/code, not
|
||||
written like a jargon-dense internal analyst note. See `references/house-style.md`
|
||||
for the specifics (jargon substitutions, AI-writing tics to avoid, how to open
|
||||
the Impact section). Worth internalizing this one before drafting: it's the
|
||||
kind of thing the user will keep correcting a little at a time if you don't.
|
||||
|
||||
### Apply it, but only once approved
|
||||
|
||||
Applying a refined advisory is normally two separate PATCH calls, and it's
|
||||
easy to only remember the first:
|
||||
|
||||
1. The top-level fields: `summary` (title), `description` (the Markdown
|
||||
sections above), `cwe_ids`, and either `cvss_vector_string` or `severity`
|
||||
(the API rejects a request setting both).
|
||||
2. The separate structured `vulnerabilities` field: an array, not a single
|
||||
object. Don't assume index `[0]` is the whole story; check how many
|
||||
entries actually exist. FOSSBilling is one application rather than a
|
||||
multi-package repo, so one entry is the norm here, but confirm it rather
|
||||
than assume it. See `references/house-style.md` for what belongs in each
|
||||
entry and the ecosystem-enum gotcha that will otherwise produce a
|
||||
confusing 422.
|
||||
|
||||
Treat `gh api --method PATCH .../security-advisories/{ghsa_id}` as a real,
|
||||
external, semi-irreversible action, even while the advisory is still
|
||||
unpublished/triage. Only run it after the user has explicitly said to apply
|
||||
the changes, not proactively once you think it's ready. If they've approved
|
||||
individual sections along the way, that's still not the same as approval to
|
||||
push to the live advisory; ask once the full set of changes is ready to go.
|
||||
@@ -0,0 +1,270 @@
|
||||
# FOSSBilling security advisory house style
|
||||
|
||||
Derived by sampling this repo's own *published* advisories directly (not
|
||||
assumed), originally from 6/6 that were 100% consistent on structure:
|
||||
GHSA-3g7m-ph2r-jhcx, GHSA-rrp7-fvc6-xxrw, GHSA-cqqm-p3x5-9fqg,
|
||||
GHSA-x7p2-xhvc-cfp9, GHSA-q4rq-9844-r9w2, GHSA-5493-9m76-2qrr.
|
||||
|
||||
This is a *living* reference: it should keep accumulating rules and
|
||||
revisions as more advisories get triaged, the same way it was built in the
|
||||
first place. If the user corrects something during a future advisory, update
|
||||
this file, don't just fix that one draft and move on. Log the change under
|
||||
Revisions at the bottom so the reasoning behind a rule stays visible, not
|
||||
just the rule itself.
|
||||
|
||||
This is distinct from the *reporter's* raw submission format (GitHub's
|
||||
default Summary/Details/PoC/Impact template): this file describes the
|
||||
maintainer-authored rewrite that actually gets published.
|
||||
|
||||
## Section order
|
||||
|
||||
```markdown
|
||||
## Summary
|
||||
## Impact
|
||||
## Affected Versions
|
||||
## Patched Versions
|
||||
## Details
|
||||
## Workarounds
|
||||
## Acknowledgements
|
||||
```
|
||||
|
||||
### Summary
|
||||
1-3 sentences: what the flaw is, where it lives (endpoint/class/method), and
|
||||
the immediate consequence. State the mechanism, not just the outcome label.
|
||||
|
||||
### Impact
|
||||
Lead with one or two plain-language sentences stating the real-world effect,
|
||||
what a person or the business actually experiences, before any technical
|
||||
framing. Don't open with a CIA-triad label ("Confidentiality: ...") as the
|
||||
first thing the reader sees; save classification framing, if used at all,
|
||||
for after the plain statement. Then list realistic follow-on scenarios in
|
||||
concrete terms (what an attacker would actually do with this), not the
|
||||
abstract class of bug.
|
||||
|
||||
### Affected Versions
|
||||
Keep this to a single brief version-range line, nothing else:
|
||||
- `FOSSBilling version 0.1.0 through 0.7.2 (all releases to date).`
|
||||
- `FOSSBilling versions >= X and <= 0.7.2.`
|
||||
|
||||
Required, not optional: confirm this range to a reasonable degree via code
|
||||
and git history before writing it down. Don't just trust the reporter's
|
||||
guess; reporters usually only tested the current release. In practice:
|
||||
- `git log -S"<distinguishing string/symbol>" -- <file>` to find when the
|
||||
vulnerable logic was introduced.
|
||||
- Unshallow the clone first if needed (`git fetch --unshallow`). A shallow
|
||||
worktree will silently truncate the search at its shallow boundary and
|
||||
give a false "this is where it was introduced" answer. Check with
|
||||
`git rev-parse --is-shallow-repository`. This is cheap (took under a
|
||||
minute the one time it was needed) and worth doing by default.
|
||||
- Check whether a later, unrelated hardening/refactor pass should have
|
||||
covered this code path but didn't. That's a strong signal for the true
|
||||
affected range, and good Details material (see the worked example below).
|
||||
- If history genuinely can't be pinned down past a certain point (e.g. a
|
||||
rename/rewrite commit that shows the whole file as newly added), that's a
|
||||
legitimate place to stop. State the range as bounded by what's confirmed
|
||||
rather than chasing false precision that can't change the answer anyway.
|
||||
|
||||
Put any narrative about *why* that range is what it is in **Details**, not
|
||||
here. This field is a fact, not an explanation.
|
||||
|
||||
### Patched Versions
|
||||
Just the version, or `TBD` if no fix exists yet. Don't write a sentence
|
||||
here either:
|
||||
- `0.8.7`
|
||||
- `TBD`
|
||||
|
||||
### Details
|
||||
Technical root-cause walkthrough:
|
||||
- Exact vulnerable entry point(s), by file/method.
|
||||
- The specific logic responsible (quote it).
|
||||
- Contrast with sibling code that handles the same concern correctly.
|
||||
FOSSBilling's own advisories consistently point out when "the fix pattern
|
||||
already exists elsewhere in the codebase"; it reads as more convincing
|
||||
than describing the bug in isolation.
|
||||
- History where relevant: which PR/commit introduced the gap, or which
|
||||
later hardening pass should have covered this path but missed it. This
|
||||
turns "here's a bug" into "here's why it slipped through," which reads as
|
||||
far more credible and helps prevent recurrence.
|
||||
|
||||
### Workarounds
|
||||
Practical steps an operator can take before a patch ships. If there is no
|
||||
real workaround, say so plainly ("There is no complete workaround without
|
||||
patching") and then list partial mitigations (WAF/reverse-proxy rate
|
||||
limiting, disabling the affected feature, config hardening).
|
||||
|
||||
### Acknowledgements
|
||||
`Thanks to <reporter> for responsibly reporting this vulnerability.`
|
||||
Exact phrasing used in every sample checked; keep it consistent.
|
||||
|
||||
## Voice
|
||||
|
||||
Technical but approachable. Stay precise on file paths, method names, and
|
||||
code (don't soften those), but avoid security-jargon labels and
|
||||
internal-analyst asides where a plain description works ("the existence
|
||||
check" over "sink", "how the message reaches the attacker" over "why the
|
||||
message reaches the attacker verbatim", no "noted in passing"-style
|
||||
meta-commentary). The reader is a technical person, not necessarily a
|
||||
security specialist.
|
||||
|
||||
Avoid em dashes and other tics that read as AI-generated: triads of
|
||||
adjectives, "it's not just X, it's Y" constructions, excessive bolded
|
||||
lead-ins. Rewrite with plain periods/commas or restructure the sentence
|
||||
instead.
|
||||
|
||||
## Severity / CVSS
|
||||
|
||||
This repo scores **CVSS v4 only**. On a GET, every published advisory
|
||||
checked has a null `cvss_severities.cvss_v3` and a populated
|
||||
`cvss_severities.cvss_v4` (GitHub nests both under `cvss_severities` on
|
||||
read); don't carry over a v3.1 vector just because the reporter submitted
|
||||
one. Do NOT restate the vector or score anywhere in the description body.
|
||||
On a PATCH, it's a different shape than the read: the top-level
|
||||
`cvss_vector_string` field, not the nested read path. The API also rejects
|
||||
a request that sets both `severity` and `cvss_vector_string` at once; pick
|
||||
one (this repo's convention is to always send `cvss_vector_string` and let
|
||||
GitHub derive `severity` from it).
|
||||
|
||||
Compute the score with a real tool rather than by hand. CVSS v4's base
|
||||
score comes from a nested lookup table (the "macrovector"), not a formula,
|
||||
and is easy to get wrong from memory. Python's `cvss` package supports it;
|
||||
the vector string below is just an example of the syntax, not a default to
|
||||
reuse:
|
||||
|
||||
```python
|
||||
from cvss import CVSS4
|
||||
c = CVSS4("CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:L/VI:N/VA:N/SC:N/SI:N/SA:N")
|
||||
c.base_score, c.severity
|
||||
```
|
||||
|
||||
Reason through each metric explicitly for the actual bug in front of you
|
||||
before computing, rather than guessing the vector wholesale:
|
||||
- `AV` (Attack Vector, N/Adjacent/Local/Physical): how does the attacker
|
||||
reach the vulnerable component?
|
||||
- `AC` (Attack Complexity): does the attacker need to actively evade some
|
||||
built-in defense to get a working exploit, or does it just work?
|
||||
- `AT` (Attack Requirements, new in v4, distinct from AC): does exploitation
|
||||
need a specific pre-existing state (a race window, being on-path, prior
|
||||
reconnaissance)? Most straightforward unauthenticated endpoint bugs are
|
||||
`AT:N`.
|
||||
- `PR` (Privileges Required) / `UI` (User Interaction): self-explanatory.
|
||||
- `VC`/`VI`/`VA` (Confidentiality/Integrity/Availability impact to the
|
||||
**Vulnerable** System): what does this specific vulnerability actually
|
||||
expose or corrupt on the system where it lives?
|
||||
- `SC`/`SI`/`SA` (impact to a **Subsequent** System): only non-`N` when the
|
||||
impact genuinely extends to a separate downstream system, not just
|
||||
"an attacker could theoretically use this to attack something else later."
|
||||
|
||||
## CWEs
|
||||
|
||||
Pick by the precise technical mechanism, not the closest-sounding label.
|
||||
CWE-204 (Observable Response Discrepancy, a message/content oracle) and
|
||||
CWE-208 (Observable Timing Discrepancy) are both "enumeration" bugs in
|
||||
casual conversation but are different root causes; don't default to
|
||||
whichever sounds closer. Check whether a similar past advisory in this repo
|
||||
used a particular CWE pairing worth following as precedent (this repo has
|
||||
done that: an oracle-endpoint-plus-weak-rate-limiting bug used CWE-204
|
||||
paired with CWE-307).
|
||||
|
||||
## Title
|
||||
|
||||
Avoid restating a CWE's official name as the title (e.g. don't write "via
|
||||
observable response discrepancy" just because that's literally what
|
||||
CWE-204 is called; it reads as jargon lifted from a taxonomy rather than a
|
||||
plain description of the bug). Match the plain subject/verb/object pattern
|
||||
used throughout this repo's real titles, e.g. "Unauthenticated X via Y
|
||||
endpoint" or "Missing Z allows W". Use the project's own domain vocabulary
|
||||
for entities: this app calls a customer record a "client" (`Box\Mod\Client`)
|
||||
rather than a generic "user" or "account", so prefer that term where it
|
||||
fits naturally.
|
||||
|
||||
## Applying changes to the live advisory
|
||||
|
||||
Two PATCH calls' worth of fields, and it's easy to only remember the prose:
|
||||
|
||||
1. **Top-level fields**: `summary` (title), `description` (the Markdown
|
||||
sections above), `cwe_ids`, and either `cvss_vector_string` or `severity`
|
||||
(the API rejects setting both at once; see Severity / CVSS above).
|
||||
2. A **separate structured field**, `vulnerabilities`: an array, not a
|
||||
single object. Don't assume index `[0]` is the whole story; check how
|
||||
many entries actually exist. FOSSBilling is a single application rather
|
||||
than a multi-package repo, so one entry is the norm, but confirm it. Each
|
||||
entry is shaped `package.ecosystem` / `package.name` /
|
||||
`vulnerable_version_range` / `patched_versions` / `vulnerable_functions`,
|
||||
which is what machine-readable consumers (OSV, Dependabot-style tooling)
|
||||
actually key off. It's easy to leave this silently inheriting whatever
|
||||
the original reporter typed if you only ever edit the prose; explicitly
|
||||
review and PATCH it too, every time, even if the values turn out
|
||||
unchanged from the reporter's submission. That should be a deliberate
|
||||
confirmation, not an accident of omission. When you do PATCH it, send the
|
||||
complete object for each entry, including `vulnerable_functions` even
|
||||
when it's an empty array, not just the one field you're changing; treat
|
||||
it as a full replacement rather than assume it merges, so anything you
|
||||
omit is anything you erase.
|
||||
|
||||
**Ecosystem enum gotcha:** FOSSBilling is an application, not a
|
||||
package-registry library, so the correct `package.ecosystem` value is the
|
||||
literal string `other`. The write API rejects an empty string outright
|
||||
(422: `` `` is not a possible value ``); the valid enum is `rubygems, npm,
|
||||
pip, maven, nuget, composer, go, rust, erlang, actions, pub, other, swift`.
|
||||
`package.name` stays `FOSSBilling` (not blank); every published advisory
|
||||
checked uses that name.
|
||||
|
||||
## Worked example (provenance for the rules above)
|
||||
|
||||
An earlier advisory triaged through this process is where most of these
|
||||
rules were established (kept unnamed here since it was still unpublished at
|
||||
the time of writing; see this repo's private working notes from that triage
|
||||
if you need the specifics):
|
||||
|
||||
- The Affected Versions git-archaeology rule came from tracing a reported
|
||||
bug back through an old rename commit, and separately discovering (via
|
||||
`git log -S` on the relevant symbol) that an unrelated PR had added a
|
||||
mitigation to sibling code in the same class but missed the exact path
|
||||
being reported, which became both the version-range justification and the
|
||||
strongest paragraph in Details.
|
||||
- The "verify every factual claim" rule came from initially writing a
|
||||
Workarounds bullet that described a config toggle's UI label from memory;
|
||||
it was wrong, and grepping the actual Twig template caught it.
|
||||
- The CVSS-v4-only and ecosystem-enum facts above came from directly
|
||||
inspecting other published advisories' API responses rather than assuming.
|
||||
|
||||
## Revisions
|
||||
|
||||
- v1: initial template derived from 6 published advisories.
|
||||
- v1.1: "Affected Versions"/"Patched Versions" corrected to single-line,
|
||||
fact-only fields (no inline narrative; narrative belongs in Details).
|
||||
- v1.2: confirmed CVSS is never restated in the body.
|
||||
- v1.3: added the Voice section after feedback that a draft read as
|
||||
slightly over-technical; reworded jargon labels ("Sink" → "the existence
|
||||
check") and dropped internal-analyst-note phrasing.
|
||||
- v1.4: Impact rewritten to lead with plain-language real-world effect
|
||||
instead of a CIA-triad-labeled bullet list.
|
||||
- v1.5: confirmed git/code-history verification of Affected Versions is
|
||||
required, not optional, to a reasonable (not unlimited) degree.
|
||||
- v1.6: added em-dash/AI-tic avoidance to Voice. Added the structured
|
||||
`vulnerabilities` API field as a required, separate PATCH step from the
|
||||
description prose, plus the `ecosystem: "other"` enum gotcha.
|
||||
- v1.7: caught the skill itself violating its own em-dash/filler-word rule
|
||||
throughout (SKILL.md and this file). Rewritten. Lesson: apply the Voice
|
||||
section to the skill's own prose, not just to advisory drafts it produces.
|
||||
- v1.8: after external review, corrected several claims that hadn't been
|
||||
verified directly. CVSS section now distinguishes the nested read path
|
||||
(`cvss_severities.cvss_v3`/`cvss_v4`) from the flat write field
|
||||
(`cvss_vector_string`), and notes `severity`/`cvss_vector_string` are
|
||||
mutually exclusive on write. The live-advisory PATCH checklist now lists
|
||||
the actual top-level fields (`summary`, `description`, `cwe_ids`,
|
||||
`cvss_vector_string`/`severity`) instead of just "the description body",
|
||||
notes `vulnerabilities` is an array (don't assume `[0]` is everything),
|
||||
and says to send the complete object per entry on write rather than a
|
||||
partial one. Confirmed directly against this repo that an unfiltered
|
||||
`security-advisories` call already returns every state for a
|
||||
maintainer-authenticated token, but the guidance still recommends explicit
|
||||
per-state queries for portability, plus the `gh api -f` without
|
||||
`--method GET` footgun (silently POSTs to the advisory-creation endpoint
|
||||
instead) is now called out explicitly, since it's real and easy to hit.
|
||||
- v1.9: the v1.8 "send the complete object" instruction listed only four
|
||||
`vulnerabilities` entry fields and omitted `vulnerable_functions`, which
|
||||
is a real field on the object (this session's own successful PATCH sent
|
||||
it as `[]`). Following the v1.8 guidance literally would have wiped
|
||||
`vulnerable_functions` on any advisory where it's populated. Added it to
|
||||
the field list.
|
||||
Symlink
+1
@@ -0,0 +1 @@
|
||||
../.agents/skills
|
||||
+6
-2
@@ -80,7 +80,10 @@
|
||||
"entrancy",
|
||||
"writability",
|
||||
"rediss",
|
||||
"dedup"
|
||||
"dedup",
|
||||
"GHSA",
|
||||
"macrovector",
|
||||
"rubygems"
|
||||
],
|
||||
|
||||
"ignoreWords": [
|
||||
@@ -638,6 +641,7 @@
|
||||
".gitignore",
|
||||
"src/.htaccess",
|
||||
"Dockerfile",
|
||||
".ddev/**"
|
||||
".ddev/**",
|
||||
".agents/skills/**"
|
||||
]
|
||||
}
|
||||
Reference in new issue
Block a user