Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions app/components/Package/Dependencies.vue
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ function getDeprecatedDepInfo(depName: string) {
return vulnTree.value.deprecatedPackages.find(p => p.name === depName && p.depth === 'direct')
}

// Check if a dependency uses git: or https: URL
function getUrlDepInfo(depName: string) {
if (!vulnTree.value) return null
return vulnTree.value.urlDependencies.find(p => p.name === depName)
}

// Expanded state for each section
const depsExpanded = shallowRef(false)
const peerDepsExpanded = shallowRef(false)
Expand Down Expand Up @@ -73,6 +79,8 @@ const sortedOptionalDependencies = computed(() => {

// Get version tooltip
function getDepVersionTooltip(dep: string, version: string) {
const urlDep = getUrlDepInfo(dep)
if (urlDep) return urlDep.url
const outdated = outdatedDeps.value[dep]
if (outdated) return getOutdatedTooltip(outdated, t)
if (getVulnerableDepInfo(dep) || getDeprecatedDepInfo(dep)) return version
Expand All @@ -82,6 +90,7 @@ function getDepVersionTooltip(dep: string, version: string) {

// Get version class
function getDepVersionClass(dep: string) {
if (getUrlDepInfo(dep)) return 'text-orange-700 dark:text-orange-500'
const outdated = outdatedDeps.value[dep]
if (outdated) return getVersionClass(outdated)
if (getVulnerableDepInfo(dep) || getDeprecatedDepInfo(dep)) return getVersionClass(undefined)
Expand Down Expand Up @@ -164,6 +173,19 @@ const numberFormatter = useNumberFormatter()
>
<span class="sr-only">{{ $t('package.deprecated.label') }}</span>
</LinkBase>
<TooltipApp
v-if="getUrlDepInfo(dep)"
class="shrink-0 text-orange-700 dark:text-orange-500"
:text="getUrlDepInfo(dep)!.url"
>
<button
type="button"
class="p-2 -m-2"
:aria-label="`git/https dependency: ${getUrlDepInfo(dep)!.url}`"
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.

This hardcoded English string needs to be internationalized. Check out guidance in CONTRIBUTING.md if you aren't familiar!

>
<span class="i-lucide:triangle-alert w-3 h-3" aria-hidden="true" />
</button>
</TooltipApp>
<LinkBase
:to="packageRoute(dep, version)"
class="block truncate"
Expand Down
53 changes: 52 additions & 1 deletion server/utils/dependency-analysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {
PackageVulnerabilityInfo,
VulnerabilityTreeResult,
DeprecatedPackageInfo,
UrlDependencyInfo,
OsvAffected,
OsvRange,
} from '#shared/types/dependency-analysis'
Expand Down Expand Up @@ -255,6 +256,52 @@ function getSeverityLevel(vuln: OsvVulnerability): OsvSeverityLevel {
return 'unknown'
}

/**
* Check if a dependency URL is a git: or https: URL that should be flagged.
*/
function isUrlDependency(url: string): boolean {
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.

I think this function is a good candidate for great good unit test coverage. Let's do some table tests covering all the supported formats (see my comment below).

return url.startsWith('git:') || url.startsWith('https:') || url.startsWith('git+https:')
}

/**
* Scan a package's dependencies for git: and https: URLs.
* Returns a map of package names to their URL dependencies.
*/
async function scanUrlDependencies(
name: string,
version: string,
depth: DependencyDepth,
path: string[],
): Promise<UrlDependencyInfo[]> {
try {
const packument = await fetchNpmPackage(name)
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.

This will be very slow, since it's re-fetching the packument for every package in the tree. This data was already constructed to begin with by fetching all these packuments and the path that "leads" to a given package is returned, so it seems it should be possible to derive what we need directly, no? 🤔

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.

If it helps simplify things, I think it would even be fine to just attach a new field for this in resolveDependencyTree directly.

const versionData = packument.versions[version]
if (!versionData) return []

const urlDeps: UrlDependencyInfo[] = []
const allDeps = {
...versionData.dependencies,
...versionData.optionalDependencies,
...versionData.devDependencies,
}

for (const [depName, depUrl] of Object.entries(allDeps || {})) {
if (isUrlDependency(depUrl)) {
urlDeps.push({
name: depName,
url: depUrl,
depth,
path: [...path, `${depName}@${depUrl}`],
Comment thread
coderabbitai[bot] marked this conversation as resolved.
})
}
}

return urlDeps
} catch {
return []
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}
}

/**
* Analyze entire dependency tree for vulnerabilities and deprecated packages.
* Uses OSV batch API for efficient vulnerability discovery, then fetches
Expand Down Expand Up @@ -289,6 +336,9 @@ export const analyzeDependencyTree = defineCachedFunction(
return depthOrder[a.depth] - depthOrder[b.depth]
})

// Scan for git: and https: URL dependencies in the root package
const urlDependencies = await scanUrlDependencies(name, version, 'root', [])
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

// Step 1: Use batch API to find which packages have vulnerabilities
// This is much faster than individual queries - one request for all packages
const { vulnerableIndices, failed: batchFailed } = await queryOsvBatch(packages)
Expand Down Expand Up @@ -347,6 +397,7 @@ export const analyzeDependencyTree = defineCachedFunction(
version,
vulnerablePackages,
deprecatedPackages,
urlDependencies,
totalPackages: packages.length,
failedQueries,
totalCounts,
Expand All @@ -356,6 +407,6 @@ export const analyzeDependencyTree = defineCachedFunction(
maxAge: 60 * 60,
swr: true,
name: 'dependency-analysis',
getKey: (name: string, version: string) => `v2:${name}@${version}`,
getKey: (name: string, version: string) => `v3:${name}@${version}`,
},
)
2 changes: 2 additions & 0 deletions server/utils/dependency-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ export interface ResolvedPackage {
path?: string[]
/** Deprecation message if the version is deprecated */
deprecated?: string
/** Original URL if this was a git: or https: dependency */
url?: string
}

/**
Expand Down
15 changes: 15 additions & 0 deletions shared/types/dependency-analysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,19 @@ export interface DeprecatedPackageInfo {
message: string
}

/**
* URL-based dependency info (git:, https:)
*/
export interface UrlDependencyInfo {
name: string
/** The git: or https: URL */
url: string
/** Depth in dependency tree: root (0), direct (1), transitive (2+) */
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.

🤔 how could root ever happen? this is only a possibility in the dependencies array of another package, isn't it?

depth: DependencyDepth
/** Dependency path from root package */
path: string[]
}

/**
* Result of dependency tree analysis
*/
Expand All @@ -192,6 +205,8 @@ export interface VulnerabilityTreeResult {
vulnerablePackages: PackageVulnerabilityInfo[]
/** All deprecated packages in the tree */
deprecatedPackages: DeprecatedPackageInfo[]
/** All dependencies using git: or https: URLs */
urlDependencies: UrlDependencyInfo[]
Comment on lines +208 to +209
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.

The two fields above use the term "package", so it would be nice to have consistency here to avoid giving a false sense of difference:

Suggested change
/** All dependencies using git: or https: URLs */
urlDependencies: UrlDependencyInfo[]
/** All packages using unsafe `git:`, `https:` (etc.) URLs in the tree */
packagesWithUrlVersion: UrlDependencyInfo[]

/** Total packages analyzed */
totalPackages: number
/** Number of packages that could not be checked (OSV query failed) */
Expand Down
Loading