Skip to content

Commit fed1bb1

Browse files
kovermierclaude
andcommitted
fix(policies): widen floating-pin detection + pin CycloneDX tool version
Address Codex review findings (stackbilt_llc#12): 1. Broaden floating-ref regex from @vN-only to any non-SHA ref. Old: `@v[\d]+` — missed @main, @master, semver tags (@v4.3.1) New: `@(?![0-9a-f]{40}(?:\s|$|#))` — flags everything except a 40-char hex SHA. Applies to detect.ts, patch.ts, and the embedded FLOATING_PIN_PATTERN constant in generate.ts. 2. resolveActionSha now also queries refs/heads/<tag> so branch refs (@main, @master) can be patched to commit SHAs. 3. Test mock updated to return correct ref lines for both tag and branch lookups; adds @main branch-ref patch test. Governed-By: Stackbilt-dev/stackbilt_llc#11 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 63958f6 commit fed1bb1

5 files changed

Lines changed: 40 additions & 18 deletions

File tree

packages/cli/src/__tests__/stamp-policies.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ function makeOptions(overrides: Partial<CLIOptions> = {}): CLIOptions {
2626
}
2727

2828
const FAKE_REF = 'a'.repeat(40);
29+
const fakeConfig = { packageManager: 'npm' as const, nodeVersion: '20', existingWorkflows: [], floatingPins: [], hasSupplyChainWorkflow: false };
2930

3031
beforeEach(() => {
3132
vi.clearAllMocks();
@@ -36,6 +37,7 @@ beforeEach(() => {
3637
});
3738
// Default: apply returns updated
3839
mockApply.mockResolvedValue({
40+
config: fakeConfig,
3941
pinsPatched: 2,
4042
workflowsPatched: ['.github/workflows/ci.yml'],
4143
supplyChainWorkflowAdded: true,
@@ -83,6 +85,7 @@ describe('stampPoliciesCommand', () => {
8385

8486
it('returns exit 0 when already compliant', async () => {
8587
mockApply.mockResolvedValue({
88+
config: fakeConfig,
8689
pinsPatched: 0,
8790
workflowsPatched: [],
8891
supplyChainWorkflowAdded: false,

packages/policies/src/__tests__/policies.test.ts

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,22 @@ function makeTempRepo(files: Record<string, string>): string {
2121
return dir;
2222
}
2323

24-
// Mock child_process.exec so tests never hit GitHub
24+
const FAKE_SHA = 'a'.repeat(40);
25+
26+
// Mock child_process.exec so tests never hit GitHub.
27+
// Returns a line for each ref type so both tag (@vN) and branch (@main) lookups resolve.
2528
vi.mock('node:child_process', () => ({
2629
exec: (
27-
_cmd: string,
30+
cmd: string,
2831
_opts: unknown,
2932
cb: (err: null, stdout: string) => void,
3033
) => {
31-
// Return a fake 40-char SHA for any tag lookup
32-
cb(null, 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\trefs/tags/v4\n');
34+
const tagM = cmd.match(/"refs\/tags\/([^"^]+)"/);
35+
const branchM = cmd.match(/"refs\/heads\/([^"]+)"/);
36+
const lines: string[] = [];
37+
if (tagM) lines.push(`${FAKE_SHA}\trefs/tags/${tagM[1]}`);
38+
if (branchM) lines.push(`${FAKE_SHA}\trefs/heads/${branchM[1]}`);
39+
cb(null, (lines.length ? lines.join('\n') : `${FAKE_SHA}\trefs/tags/v4`) + '\n');
3340
},
3441
}));
3542

@@ -59,17 +66,19 @@ jobs:
5966
expect(detectRepoConfig(dir).packageManager).toBe('pnpm');
6067
});
6168

62-
it('detects floating pins in workflow', () => {
69+
it('detects floating pins in workflow — tags and branch refs', () => {
6370
const dir = makeTempRepo({
6471
'.github/workflows/ci.yml': `
6572
steps:
6673
- uses: actions/checkout@v4
6774
- uses: actions/setup-node@v4
75+
- uses: actions/cache@main
6876
`,
6977
});
7078
const config = detectRepoConfig(dir);
71-
expect(config.floatingPins).toHaveLength(2);
79+
expect(config.floatingPins).toHaveLength(3);
7280
expect(config.floatingPins[0].tag).toBe('v4');
81+
expect(config.floatingPins[2].tag).toBe('main');
7382
});
7483

7584
it('does not flag SHA-pinned or local refs as floating', () => {
@@ -104,7 +113,14 @@ describe('patchFloatingActionPins', () => {
104113
const content = `steps:\n - uses: actions/checkout@v4\n`;
105114
const { patched, replacements } = await patchFloatingActionPins(content);
106115
expect(replacements).toHaveLength(1);
107-
expect(patched).toContain('actions/checkout@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # v4');
116+
expect(patched).toContain(`actions/checkout@${FAKE_SHA} # v4`);
117+
});
118+
119+
it('replaces @main branch ref with mocked SHA + comment', async () => {
120+
const content = `steps:\n - uses: actions/cache@main\n`;
121+
const { patched, replacements } = await patchFloatingActionPins(content);
122+
expect(replacements).toHaveLength(1);
123+
expect(patched).toContain(`actions/cache@${FAKE_SHA} # main`);
108124
});
109125

110126
it('leaves SHA-pinned lines unchanged', async () => {
@@ -203,7 +219,7 @@ describe('applyPolicies', () => {
203219
expect(fs.existsSync(path.join(dir, '.github/workflows/supply-chain.yml'))).toBe(true);
204220
// pin patched
205221
const ciContent = fs.readFileSync(path.join(dir, '.github/workflows/ci.yml'), 'utf-8');
206-
expect(ciContent).toContain('@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # v4');
222+
expect(ciContent).toContain(`@${FAKE_SHA} # v4`);
207223
// charter config created
208224
expect(fs.existsSync(path.join(dir, '.charter/config.json'))).toBe(true);
209225
// floating-action-pins pattern installed

packages/policies/src/detect.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@ export interface RepoConfig {
1616
hasSupplyChainWorkflow: boolean;
1717
}
1818

19-
// Matches `uses: owner/action@vN[.N.N]` — excludes local (./) and Stackbilt-dev refs
20-
const FLOATING_PIN_LINE_RE = /uses:\s+(?!Stackbilt-dev\/)(?!\.\/)[^\s@]+@(v[\d][\d.]*)/;
19+
// Matches any non-SHA ref (@vN, @main, @master, semver tags) — excludes local (./) and Stackbilt-dev refs.
20+
// A 40-char hex SHA followed by whitespace, EOL, or # comment is the only exempt form.
21+
const FLOATING_PIN_LINE_RE = /uses:\s+(?!Stackbilt-dev\/)(?!\.\/)[^\s@]+@(?![0-9a-f]{40}(\s|$|#))(\S+)/;
2122

2223
export function detectRepoConfig(repoPath: string): RepoConfig {
2324
const abs = path.resolve(repoPath);
@@ -44,7 +45,7 @@ export function detectRepoConfig(repoPath: string): RepoConfig {
4445
content.split('\n').forEach((line, idx) => {
4546
const m = line.match(FLOATING_PIN_LINE_RE);
4647
if (m) {
47-
floatingPins.push({ file: wfPath, line: idx + 1, uses: line.trim(), tag: m[1] });
48+
floatingPins.push({ file: wfPath, line: idx + 1, uses: line.trim(), tag: m[2] });
4849
}
4950
});
5051
}

packages/policies/src/generate.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,11 @@ export const FLOATING_PIN_PATTERN = {
2929
name: 'Floating Action Pins',
3030
category: 'SECURITY',
3131
status: 'ACTIVE',
32-
anti_patterns: 'uses: (?!Stackbilt-dev/)(?!\\./)[^@]+@v[\\d]',
32+
anti_patterns: 'uses: (?!Stackbilt-dev/)(?!\\./)[^\\s@]+@(?![0-9a-f]{40}(\\s|$|#))[^\\s]',
3333
blessed_solution:
3434
'Pin to full commit SHA with a # vX.Y.Z comment: uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4',
3535
rationale:
36-
'Tag-based action pins are mutable — a supply chain attacker can move the tag to a malicious commit. SHA pins are immutable.',
36+
'Any non-SHA ref (@vN, @main, @master, semver tags) is mutable — only a 40-char hex SHA is immutable.',
3737
created_at: '2026-05-20T00:00:00.000Z',
3838
} as const;
3939

packages/policies/src/patch.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ export interface PatchResult {
1111
replacements: PatchReplacement[];
1212
}
1313

14-
// Matches a `uses:` line with a floating tag — excludes local and Stackbilt-dev refs
15-
const FLOATING_USES_RE = /^(\s*-?\s*uses:\s+)((?!Stackbilt-dev\/)(?!\.\/)[^\s@]+)@(v[\d][\d.]*)(.*)$/;
14+
// Matches any non-SHA ref (@vN, @main, @master, semver) — excludes local (./) and Stackbilt-dev refs.
15+
// A 40-char hex SHA is the only exempt form.
16+
const FLOATING_USES_RE = /^(\s*-?\s*uses:\s+)((?!Stackbilt-dev\/)(?!\.\/)[^\s@]+)@(?![0-9a-f]{40}(?:\s|$|#))([^\s#]+)(.*)$/;
1617

1718
export async function patchFloatingActionPins(content: string): Promise<PatchResult> {
1819
const replacements: PatchReplacement[] = [];
@@ -46,14 +47,15 @@ export async function patchFloatingActionPins(content: string): Promise<PatchRes
4647
function resolveActionSha(action: string, tag: string): Promise<string | null> {
4748
return new Promise((resolve) => {
4849
const url = `https://github.com/${action}`;
49-
const cmd = `git ls-remote "${url}" "refs/tags/${tag}" "refs/tags/${tag}^{}"`;
50+
// Try annotated tag deref, plain tag, then branch head (covers @main, @master, etc.)
51+
const cmd = `git ls-remote "${url}" "refs/tags/${tag}" "refs/tags/${tag}^{}" "refs/heads/${tag}"`;
5052
exec(cmd, { timeout: 20000 }, (err, stdout) => {
5153
if (err || !stdout.trim()) { resolve(null); return; }
5254
const lines = stdout.trim().split('\n');
53-
// Prefer dereferenced annotated tag (^{}) — gives the commit SHA, not tag object SHA
5455
const deref = lines.find((l) => l.includes(`refs/tags/${tag}^{}`));
5556
const plain = lines.find((l) => l.includes(`refs/tags/${tag}`) && !l.includes('^{}'));
56-
const winner = deref ?? plain;
57+
const branch = lines.find((l) => l.includes(`refs/heads/${tag}`));
58+
const winner = deref ?? plain ?? branch;
5759
if (!winner) { resolve(null); return; }
5860
const sha = winner.split('\t')[0].trim();
5961
resolve(sha.length === 40 ? sha : null);

0 commit comments

Comments
 (0)