Skip to content

Commit 351622b

Browse files
committed
fix(audits): resolve import specifiers to module files only; tighten review-flagged guidance
1 parent b4ac006 commit 351622b

6 files changed

Lines changed: 31 additions & 15 deletions

File tree

‎.agents/skills/add-connector/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,7 @@ If `ExternalDocument.sourceUrl` is set, the sync engine stores it on the documen
541541

542542
If `listDocuments` can ever return **less than the full source set** on a non-incremental sync — a `maxItems`/`maxDocuments`-style cap, or a transient per-item error that drops a still-existing document from the listing — it MUST set `syncContext.listingCapped = true` when that happens.
543543

544-
The engine reconciles deletions only when the listing is marked safe (`checkpoint.unsafe` in `lib/knowledge/connectors/listing-checkpoint.ts`): `syncContext.listingCapped`, `ExternalDocumentList.reconciliationSafe: false` (required for offset/unstable pagination), or a non-null `listingFailures` marks it unsafe. Anything absent from a safe listing is tombstoned on that sync and hard-deleted when the next sync still does not see it — so a truncated listing without this flag eventually removes every real document beyond the cap.
544+
The engine reconciles deletions only when the listing is marked safe (`checkpoint.unsafe` in `lib/knowledge/connectors/listing-checkpoint.ts`): `syncContext.listingCapped`, `syncContext.listingTruncated`, `syncContext.reconciliationUnsafe`, `ExternalDocumentList.reconciliationSafe: false` (required for offset/unstable pagination), or a non-null `listingFailures` marks it unsafe. Anything absent from a safe listing is tombstoned on that sync and hard-deleted when the next sync still does not see it — so a truncated listing without this flag eventually removes every real document beyond the cap.
545545

546546
```typescript
547547
if (hitLimit && syncContext) {

‎.agents/skills/add-permission-group-item/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ Allowlist when the safe posture is "only what the admin named" and the member se
4141
| `'executor'` | Read per block/tool/model at run time by `assertPermissionsAllowed` in `ee/access-control/utils/permission-check.ts`. Governs what a *run* may do, which no operation gate can express (one API call executes fifty blocks). Only `allowedIntegrations`, `allowedModelProviders`, `deniedModels`, `deniedTools` live here. The matching primitives these four keys are compared with live in `lib/permission-groups/` — `block-access.ts` (exemptions, superseded-version resolution), `operation-access.ts` (`createToolAccessGate`), `model-access.ts` (`createModelAccessGate`), `integration-allowlist.ts` — shared so the run-time gate and the editor/Copilot projections cannot drift. `allowedIntegrations` alone is also asserted outside a run, by `assertSelectorIntegrationAllowed` (`lib/selectors/server/integration-access.ts`) ahead of the provider call in `selectors.execute`, against the selector's own `resourceServiceId` / `integrationBlockTypes` rather than the credentials it accepts — reaching a provider API is a use of the integration, so a key here can still need a non-run enforcement site |
4242
| `'ui-only'` | Hides a surface without withholding it. **Almost never right** — nothing ships as `ui-only`. Justify in the `enforcement` comment why a determined caller reaching the data is acceptable, and expect review to question it |
4343

44-
**Is it per-operation at all?** `personal_api_key.use` and `oauth_apps.use` are principal-wide (`PRINCIPAL_WIDE_CAPABILITIES` in `lib/core/application/operation.ts`): they withhold a principal kind across every operation, checked in the funnel's `personal_api_key` / `oauth_access_token` branches (`workspace-authorization.ts`) and in `app/api/v1/middleware.ts`. Declaring one on an operation throws at definition time, so its absence from every `capability:` field is correct.
44+
**Is it per-operation at all?** `personal_api_key.use` and `oauth_apps.use` are principal-wide (`PRINCIPAL_WIDE_CAPABILITIES` in `lib/core/application/operation.ts`): they withhold a principal kind across every operation, checked in the funnel's `personal_api_key` / `oauth_access_token` branches (`workspace-authorization.ts`); `app/api/v1/middleware.ts` repeats only the `personal_api_key.use` check, since v1 accepts API keys, not OAuth tokens. Declaring one on an operation throws at definition time, so its absence from every `capability:` field is correct.
4545

4646
**Is the decision knowable from the config alone?** A rule needing a request value (an auth mode, a connector id) is *parameterized* and cannot be declared on an operation — see Step 3.
4747

‎.agents/skills/babysit/SKILL.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,9 @@ conditions freshly after every push.
8888
across all pages has `isResolved: true`, and every check has finished and passed, stop —
8989
report the outcome (see "Reporting" below) and skip the rest of this list.
9090

91-
2. **If the PR has a merge conflict**, merge `origin/staging`, resolve the conflicts, then
92-
follow steps 6–8 on the merge result: the full sync check plus `/ship` steps 4–6, push, and
93-
re-trigger review.
91+
2. **If the PR has a merge conflict**, resolve it with step 6 (its rebase-based sync flow and
92+
the `/ship` gates; a merge commit would be discarded by that rebase), then steps 7–8: push
93+
with `--force-with-lease` and re-trigger review.
9494

9595
3. **If no review has run yet** (fresh PR, no bot comments): both run automatically on PR open —
9696
confirm via `gh pr checks <n>` (look for `Greptile Review` and `cubic · AI code reviewer`) and

‎.claude/rules/sim-architecture.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ Repository layout, package boundaries, the application operation boundary, namin
1010

1111
## The `'use client'` server boundary
1212

13-
Server-evaluated code (RSC pages/layouts, `prefetch.ts`, route handlers, block definitions, triggers) can render an export of a `'use client'` module but never call it; the call throws at runtime and `next build` does not catch it. Keep anything a server module calls (key factories, fetchers, mappers, constants) in a non-`'use client'` module. Detail and the escape hatch: `.claude/rules/sim-queries.md`. `bun run check:client-boundary` enforces it for `prefetch*.ts`, API route handlers, triggers, and blocks; nothing checks pages and layouts.
13+
Server-evaluated code (RSC pages/layouts, `prefetch.ts`, route handlers, block definitions, triggers) can render an export of a `'use client'` module but never call it; the call throws at runtime and `next build` does not catch it. Keep anything a server module calls (key factories, fetchers, mappers, constants) in a non-`'use client'` module. Detail and the escape hatch: `.claude/rules/sim-queries.md`. `bun run check:client-boundary` checks the direct imports of `prefetch*.ts`, API route handlers, triggers, and blocks; it does not follow transitive imports, and nothing checks pages and layouts.
1414

1515
## The app/worker runtime boundary
1616

‎.cursor/rules/sim-architecture.mdc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ Repository layout, package boundaries, the application operation boundary, namin
1111

1212
## The `'use client'` server boundary
1313

14-
Server-evaluated code (RSC pages/layouts, `prefetch.ts`, route handlers, block definitions, triggers) can render an export of a `'use client'` module but never call it; the call throws at runtime and `next build` does not catch it. Keep anything a server module calls (key factories, fetchers, mappers, constants) in a non-`'use client'` module. Detail and the escape hatch: `.claude/rules/sim-queries.md`. `bun run check:client-boundary` enforces it for `prefetch*.ts`, API route handlers, triggers, and blocks; nothing checks pages and layouts.
14+
Server-evaluated code (RSC pages/layouts, `prefetch.ts`, route handlers, block definitions, triggers) can render an export of a `'use client'` module but never call it; the call throws at runtime and `next build` does not catch it. Keep anything a server module calls (key factories, fetchers, mappers, constants) in a non-`'use client'` module. Detail and the escape hatch: `.claude/rules/sim-queries.md`. `bun run check:client-boundary` checks the direct imports of `prefetch*.ts`, API route handlers, triggers, and blocks; it does not follow transitive imports, and nothing checks pages and layouts.
1515

1616
## The app/worker runtime boundary
1717

‎scripts/check-guidance-refs.ts‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
* Run: `bun run check:guidance-refs`
2121
*/
2222
import { execFileSync } from 'node:child_process'
23-
import { existsSync, readFileSync, realpathSync } from 'node:fs'
23+
import { existsSync, readFileSync, realpathSync, statSync } from 'node:fs'
2424
import path from 'node:path'
2525
import { parseRule } from './sync-skills'
2626

@@ -124,6 +124,26 @@ function resolvesFrom(base: string, ref: string): boolean {
124124
return SOURCE_EXTENSIONS.some((ext) => existsSync(path.join(base, `${ref}${ext}`)))
125125
}
126126

127+
const MODULE_CANDIDATES = [
128+
'',
129+
'.ts',
130+
'.tsx',
131+
'.js',
132+
'.mjs',
133+
'.json',
134+
'/index.ts',
135+
'/index.tsx',
136+
'/index.js',
137+
]
138+
139+
/** An import specifier resolves only to a module file, never to a bare directory. */
140+
function moduleResolves(base: string, ref: string): boolean {
141+
return MODULE_CANDIDATES.some((suffix) => {
142+
const candidate = path.join(base, `${ref}${suffix}`)
143+
return existsSync(candidate) && statSync(candidate).isFile()
144+
})
145+
}
146+
127147
const PATH_PREFIXES = ['apps/', 'packages/', 'scripts/', '.claude/', '.agents/', '.github/']
128148

129149
/**
@@ -151,16 +171,12 @@ function importResolves(spec: string, packages: Map<string, WorkspacePackage>):
151171
const clean = spec.replace(/\/$/, '')
152172
if (clean === '@' || clean === '@sim') return true
153173
if (clean.startsWith('@/')) {
154-
const rel = clean.slice(2)
155-
return (
156-
resolvesFrom(APP_ROOT, rel) ||
157-
['/index.ts', '/index.tsx'].some((index) => existsSync(path.join(APP_ROOT, `${rel}${index}`)))
158-
)
174+
return moduleResolves(APP_ROOT, clean.slice(2))
159175
}
160176
const [scope, name, ...rest] = clean.split('/')
161177
const pkg = packages.get(`${scope}/${name}`)
162178
if (!pkg) return false
163-
if (!pkg.exports) return rest.length === 0 || resolvesFrom(pkg.dir, rest.join('/'))
179+
if (!pkg.exports) return rest.length === 0 || moduleResolves(pkg.dir, rest.join('/'))
164180
const subpath = rest.length === 0 ? '.' : `./${rest.join('/')}`
165181
return Object.entries(pkg.exports).some(([key, value]) => {
166182
const [head, tail] = key.split('*')
@@ -169,7 +185,7 @@ function importResolves(spec: string, packages: Map<string, WorkspacePackage>):
169185
// A wildcard export only proves the pattern; the file it maps to must also exist.
170186
const target = exportTarget(value)
171187
const matched = subpath.slice(head.length, subpath.length - tail.length)
172-
return target !== null && resolvesFrom(pkg.dir, target.replace('*', matched))
188+
return target !== null && moduleResolves(pkg.dir, target.replace('*', matched))
173189
})
174190
}
175191

0 commit comments

Comments
 (0)