Skip to content

fix(registry): Ensure up-to-date remote before pushing#187

Merged
BYK merged 1 commit into
masterfrom
byk/fix/concurrent-registry
Mar 16, 2021
Merged

fix(registry): Ensure up-to-date remote before pushing#187
BYK merged 1 commit into
masterfrom
byk/fix/concurrent-registry

Conversation

@BYK

@BYK BYK commented Mar 12, 2021

Copy link
Copy Markdown
Member

@BYK BYK enabled auto-merge (squash) March 12, 2021 18:23
Comment thread src/targets/registry.ts
await git.commit(`craft: release "${canonicalName}", version "${version}"`);

// Ensure we are still up to date with upstream
await git.pull('origin', 'master', { rebase: true });

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.

There's still a chance for a race condition. What if you put this in a bounded loop?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think, at our scale it is very unlikely that we'll hit the condition you mention and a bounded loop is still a band-aid fix. When that time comes, we should look into proper queues, not more band aids.

@BYK BYK requested a review from jan-auer March 15, 2021 09:47
@BYK BYK closed this Mar 16, 2021
auto-merge was automatically disabled March 16, 2021 09:03

Pull request was closed

@BYK BYK reopened this Mar 16, 2021
@BYK BYK enabled auto-merge (squash) March 16, 2021 09:03
@BYK BYK merged commit 4c861a7 into master Mar 16, 2021
@BYK BYK deleted the byk/fix/concurrent-registry branch March 16, 2021 09:36
BYK added a commit that referenced this pull request Jul 1, 2026
## Summary

Resolves 3 Dependabot alerts (**#186, #187, #188**), all the same
advisory across both projects.

- **Advisory:**
[GHSA-h67p-54hq-rp68](GHSA-h67p-54hq-rp68)
/ CVE-2026-53550 — *js-yaml: Quadratic-complexity DoS in merge key
handling via repeated aliases* (medium severity)
- **Vulnerable:** \`>= 4.0.0, <= 4.1.1\` → **patched:** \`4.2.0\`

These alerts were opened by the post-merge rescan right after #839
landed (newly-published advisory), not caused by that change.

## Fix

| Alert | Manifest | Relationship | Change |
|---|---|---|---|
| #187 | root \`package.json\` | direct devDep | bump pinned \`js-yaml\`
\`4.1.1\` → \`4.2.0\` |
| #188 | root \`pnpm-lock.yaml\` | direct devDep | regenerated →
\`4.2.0\` |
| #186 | \`docs/pnpm-lock.yaml\` | transitive (astro/starlight) |
\`pnpm.overrides\` \`js-yaml: ^4.2.0\` → resolves to \`4.3.0\` |

- Root uses an exact pin (matching the repo's pinned-devDep convention,
e.g. \`tar\`); docs uses an override (matching the docs override style)
since js-yaml is transitive there.
- Both lockfile diffs are 100% js-yaml-scoped — no unrelated drift. No
\`@babel/core\`/other packages touched.

## Verification

- Root: \`pnpm build\` ✅, \`pnpm test\` ✅ (1025 passed, 1 skipped),
\`pnpm lint\` ✅ (0 errors), \`pnpm format:check\` ✅ (tracked files)
- Docs: \`pnpm build\` ✅ (27 pages built)
- \`grep "js-yaml@" pnpm-lock.yaml docs/pnpm-lock.yaml\` — only
\`4.2.0\`/\`4.3.0\` remain; \`4.1.1\` gone. (\`@types/js-yaml\` is
type-defs only, not the vulnerable package.)
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.

Multiple parallel publishes at the same time conflict release registry

3 participants