Skip to content

Commit c2dae4b

Browse files
authored
feat(artifact-delivery): fall back to the requester's GitHub token (#495)
1 parent a495b6f commit c2dae4b

14 files changed

Lines changed: 385 additions & 4 deletions

‎layers/artifact-delivery/server/api/v1/resolutions/index.post.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ export default withArtifactProblems(defineApiHandler({
4949
repositoryId: privateAccess.repositoryId,
5050
}
5151
: { visibility: 'public' },
52+
requesterAccountId: user?.id ?? null,
5253
})
5354
if (result._tag === 'idempotency-conflict') {
5455
throw createError({

‎layers/artifact-delivery/server/utils/build.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
import { checkArtifactSource, checksBlockArtifact, checksPermitSigning } from './checks'
3030
import { canonicalJson, digestHex } from './encoding'
3131
import { isRetryableProblem, storedFilesPassLoadRules } from './github-source'
32+
import { withRequesterFallback } from './requester-github'
3233
import {
3334
ARTIFACT_POLICY_VERSION,
3435
BYTE_COMPATIBLE_POLICY_VERSIONS,
@@ -46,6 +47,12 @@ export interface ArtifactBuildDependencies {
4647
db: D1Database
4748
github: PublicGithubSourceClient
4849
privateGithub?: (row: ResolutionRow) => Promise<PublicGithubSourceClient | SourceRejection>
50+
/**
51+
* The GitHub client of the signed-in account that asked for a public
52+
* Resolution, or null. A build uses it only for a read the shared
53+
* credential's quota refused, and only for that account's own Resolution.
54+
*/
55+
requesterGithub?: (row: ResolutionRow) => Promise<PublicGithubSourceClient | null>
4956
bucket: R2Bucket
5057
privateArtifacts?: {
5158
put: (input: {
@@ -824,8 +831,12 @@ async function githubForResolution(
824831
dependencies: ArtifactBuildDependencies,
825832
row: ResolutionRow,
826833
): Promise<PublicGithubSourceClient | SourceRejection> {
827-
if (row.visibility === 'public')
828-
return dependencies.github
834+
if (row.visibility === 'public') {
835+
const requesterGithub = dependencies.requesterGithub
836+
return requesterGithub
837+
? withRequesterFallback(dependencies.github, () => requesterGithub(row))
838+
: dependencies.github
839+
}
829840
if (!dependencies.privateGithub) {
830841
return {
831842
_tag: 'rejected',

‎layers/artifact-delivery/server/utils/queue.ts‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import { createGithubSourceClient, createPublicGithubSourceClient } from './gith
2626
import { createD1PrivateArtifactKeyProvider, privateArtifactWrappingKeysFromEnv } from './private-crypto'
2727
import { privateArtifactAccessEnabled } from './private-feature'
2828
import { putPrivateArtifact } from './private-storage'
29+
import { createRequesterGithubSource, forgetResolutionRequester } from './requester-github'
2930
import { getResolution } from './state'
3031
import { parseTrustedRoot } from './trusted-root'
3132

@@ -93,6 +94,10 @@ export async function consumeArtifactBuildBatch(
9394
continue
9495
}
9596
if (outcome._tag === 'ok') {
97+
// A settled build never reads GitHub again, so it no longer needs to
98+
// know who asked. A superseded one leaves that to the build that won.
99+
if (outcome.value._tag !== 'superseded')
100+
await forgetResolutionRequester(dependencies.db, parsed.data.resolutionId)
96101
message.ack()
97102
continue
98103
}
@@ -112,6 +117,7 @@ export async function consumeArtifactBuildBatch(
112117
const row = await getResolution(dependencies.db, parsed.data.resolutionId)
113118
if (row)
114119
await failResolution(dependencies, row, 'SERVICE_UNAVAILABLE', true)
120+
await forgetResolutionRequester(dependencies.db, parsed.data.resolutionId)
115121
message.ack()
116122
}
117123
}
@@ -209,9 +215,23 @@ export function createArtifactBuildDependencies(env: Cloudflare.Env): ArtifactBu
209215
const privateDependencies = privateArtifactAccessEnabled(env)
210216
? createPrivateBuildDependencies(env, runtimeFetch)
211217
: {}
218+
const githubObjects = createKvGithubObjectCache(env.KV_CACHE)
212219
return {
213220
db: env.DB,
214-
github: createArtifactGithubSource(env, defaultGithubSourceRuntime(), createKvGithubObjectCache(env.KV_CACHE)),
221+
github: createArtifactGithubSource(env, defaultGithubSourceRuntime(), githubObjects),
222+
requesterGithub: createRequesterGithubSource({
223+
db: env.DB,
224+
tokenKey: () => env.NUXT_TOKEN_KEY,
225+
fetch: runtimeFetch,
226+
now: () => Math.floor(Date.now() / 1000),
227+
cache: githubObjects,
228+
onReadFailure: reportGithubReadFailure,
229+
onUnusableToken: reason => emitOperationalEvent(createWideEvent({
230+
operation: 'artifact-github-credential',
231+
outcome: 'requester-token-unusable',
232+
reason,
233+
})),
234+
}),
215235
bucket: env.PUBLIC_ARTIFACTS,
216236
signer: createArtifactSigner(env.ARTIFACT_SIGNER),
217237
trustedRoot: parseTrustedRoot(env.ARTIFACT_TRUSTED_ROOT_JSON, Math.floor(Date.now() / 1000)),

‎layers/artifact-delivery/server/utils/request-resolution.ts‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { SourceRequest } from '../schemas/contracts'
33
import type { FetchAdmittedSkillIdentity } from './admitted-identity'
44
import type { CreateResolutionResult } from './state'
55
import { admittedSourceRequest } from './admitted-identity'
6+
import { recordResolutionRequester } from './requester-github'
67
import { createResolution, resolutionRequestIdentity } from './state'
78

89
export interface ResolutionRequestDependencies {
@@ -48,14 +49,30 @@ export function fetchAdmittedSkillIdentity(context?: H3EventContext): FetchAdmit
4849
*/
4950
export async function requestResolution(
5051
dependencies: ResolutionRequestDependencies,
51-
input: { source: SourceRequest, idempotencyKey: string, access: ResolutionAccess },
52+
input: {
53+
source: SourceRequest
54+
idempotencyKey: string
55+
access: ResolutionAccess
56+
/**
57+
* The signed-in account that sent a public request, or null. Its build may
58+
* read GitHub with that account's own token once the shared quota is spent.
59+
*/
60+
requesterAccountId?: number | null
61+
},
5262
): Promise<CreateResolutionResult> {
5363
const accountId = input.access.visibility === 'private' ? input.access.accountId : undefined
5464
const identity = await resolutionRequestIdentity(input.source, input.idempotencyKey, accountId)
5565
const source = input.access.visibility === 'private'
5666
? input.source
5767
: await admittedSourceRequest(input.source, dependencies.lookupAdmitted)
5868
const result = await createResolution(dependencies.db, source, identity, dependencies.now(), input.access)
69+
if (result._tag === 'created' && input.access.visibility === 'public' && input.requesterAccountId) {
70+
await recordResolutionRequester(dependencies.db, {
71+
resolutionId: result.row.id,
72+
accountId: input.requesterAccountId,
73+
now: dependencies.now(),
74+
})
75+
}
5976
if (result._tag !== 'idempotency-conflict' && result.row.state === 'requested')
6077
await dependencies.enqueue(result.row.id)
6178
return result
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
import type { GithubObjectCache, GithubReadFailure, PublicGithubSourceClient } from './github-source'
2+
import type { ResolutionRow } from './state'
3+
import { decryptToken } from '#layers/identity/server/utils/crypto'
4+
import { createPublicGithubSourceClient } from './github-source'
5+
6+
/**
7+
* Record the signed-in account that asked for a public Resolution.
8+
*
9+
* Only that Resolution's build reads it, and only after the shared GitHub
10+
* credentials are rate limited. The row goes when the build settles.
11+
*/
12+
export async function recordResolutionRequester(
13+
db: D1Database,
14+
input: { resolutionId: string, accountId: number, now: number },
15+
): Promise<void> {
16+
await db.prepare(
17+
`INSERT OR IGNORE INTO artifact_resolution_requesters (resolution_id, account_id, created_at)
18+
VALUES (?1, ?2, ?3)`,
19+
).bind(input.resolutionId, input.accountId, input.now).run()
20+
}
21+
22+
/** Forget who asked for a Resolution. Its build has settled. */
23+
export async function forgetResolutionRequester(db: D1Database, resolutionId: string): Promise<void> {
24+
await db.prepare(`DELETE FROM artifact_resolution_requesters WHERE resolution_id = ?1`).bind(resolutionId).run()
25+
}
26+
27+
export interface RequesterGithubDependencies {
28+
db: D1Database
29+
/**
30+
* `NUXT_TOKEN_KEY`, which encrypts the stored GitHub OAuth tokens. It is read
31+
* only when a requester's token is needed.
32+
*/
33+
tokenKey: () => string
34+
fetch: typeof globalThis.fetch
35+
/** Unix seconds. */
36+
now: () => number
37+
/** Commits and trees by SHA, shared with the build's own client. */
38+
cache?: GithubObjectCache
39+
onReadFailure?: (failure: GithubReadFailure) => void
40+
/** Called when a stored token cannot be used. It never receives the token. */
41+
onUnusableToken?: (reason: string) => void
42+
}
43+
44+
/**
45+
* The GitHub client of the account that asked for one public Resolution, or
46+
* null when nobody signed in asked for it, or the account holds no usable
47+
* GitHub token.
48+
*
49+
* The token is the account's GitHub sign-in token. Its scopes, `read:user` and
50+
* `user:email`, read public Repositories at that account's own quota, and no
51+
* private one. The public client also refuses any Repository GitHub reports
52+
* as private, so a build read with it can only produce public content.
53+
*/
54+
export function createRequesterGithubSource(
55+
dependencies: RequesterGithubDependencies,
56+
): (row: ResolutionRow) => Promise<PublicGithubSourceClient | null> {
57+
return async (row) => {
58+
if (row.visibility !== 'public')
59+
return null
60+
const stored = await dependencies.db.prepare(
61+
`SELECT u.github_token_encrypted, u.github_token_expires_at
62+
FROM artifact_resolution_requesters r
63+
JOIN users u ON u.id = r.account_id
64+
WHERE r.resolution_id = ?1
65+
LIMIT 1`,
66+
).bind(row.id).first<{ github_token_encrypted: string | null, github_token_expires_at: number | null }>()
67+
if (!stored?.github_token_encrypted)
68+
return null
69+
if (stored.github_token_expires_at !== null && stored.github_token_expires_at <= dependencies.now() + 60)
70+
return null
71+
const token = await decryptToken(stored.github_token_encrypted, dependencies.tokenKey()).then(
72+
value => ({ _tag: 'token' as const, value }),
73+
(error: unknown) => ({ _tag: 'unreadable' as const, reason: error instanceof Error ? error.message : String(error) }),
74+
)
75+
if (token._tag === 'unreadable') {
76+
dependencies.onUnusableToken?.(`The stored GitHub token did not decrypt: ${token.reason}`.slice(0, 200))
77+
return null
78+
}
79+
return createPublicGithubSourceClient({
80+
fetch: dependencies.fetch,
81+
token: token.value,
82+
cache: dependencies.cache,
83+
onReadFailure: dependencies.onReadFailure,
84+
})
85+
}
86+
}
87+
88+
/**
89+
* A client that reads with the shared credential, and repeats a read the
90+
* shared quota refused with the requester's own client.
91+
*
92+
* Every other answer, success or rejection, stays the shared one, so a
93+
* requester's token is spent only when nothing else could read.
94+
*/
95+
export function withRequesterFallback(
96+
shared: PublicGithubSourceClient,
97+
requester: () => Promise<PublicGithubSourceClient | null>,
98+
): PublicGithubSourceClient {
99+
let own: Promise<PublicGithubSourceClient | null> | null = null
100+
const ownClient = () => {
101+
own ??= requester()
102+
return own
103+
}
104+
return {
105+
async resolve(request) {
106+
const first = await shared.resolve(request)
107+
if (first._tag !== 'rejected' || first.code !== 'RATE_LIMITED')
108+
return first
109+
const client = await ownClient()
110+
return client ? await client.resolve(request) : first
111+
},
112+
async load(source) {
113+
const first = await shared.load(source)
114+
if (first._tag !== 'rejected' || first.code !== 'RATE_LIMITED')
115+
return first
116+
const client = await ownClient()
117+
return client ? await client.load(source) : first
118+
},
119+
}
120+
}

‎layers/identity/server/utils/account-deletion.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ const ACCOUNT_ROW_DELETES = [
2727
`DELETE FROM digest_runs WHERE user_id = ?1`,
2828
`DELETE FROM weekly_runs WHERE user_id = ?1`,
2929
`DELETE FROM email_preference_events WHERE user_id = ?1`,
30+
`DELETE FROM artifact_resolution_requesters WHERE account_id = ?1`,
3031
`DELETE FROM artifact_download_grants WHERE account_id = ?1`,
3132
`DELETE FROM private_artifact_attestations WHERE account_id = ?1`,
3233
`DELETE FROM private_artifacts WHERE account_id = ?1`,

‎layers/identity/server/utils/data-retention.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ export const RETENTION_SECONDS = {
1414
cliSignIn: DAY_SECONDS,
1515
/** Delivery history and preference changes support three months of reporting. */
1616
emailHistory: 90 * DAY_SECONDS,
17+
/**
18+
* The account that asked for a public Artifact build. The build deletes it
19+
* when it settles; this removes one a dead build left behind.
20+
*/
21+
runRequester: DAY_SECONDS,
1722
} as const
1823

1924
export interface RetentionPurgeResult {
@@ -23,6 +28,7 @@ export interface RetentionPurgeResult {
2328
digestRuns: number
2429
weeklyRuns: number
2530
emailPreferenceEvents: number
31+
runRequesters: number
2632
}
2733

2834
/**
@@ -40,6 +46,7 @@ export async function purgeRetainedPersonalData(
4046
const endedTokenCutoff = now - RETENTION_SECONDS.endedCliToken
4147
const signInCutoff = now - RETENTION_SECONDS.cliSignIn
4248
const historyCutoff = now - RETENTION_SECONDS.emailHistory
49+
const requesterCutoff = now - RETENTION_SECONDS.runRequester
4350

4451
const results = await db.batch([
4552
db.prepare(
@@ -62,6 +69,7 @@ export async function purgeRetainedPersonalData(
6269
).bind(historyCutoff),
6370
db.prepare(`DELETE FROM weekly_runs WHERE window_end <= ?1`).bind(historyCutoff),
6471
db.prepare(`DELETE FROM email_preference_events WHERE occurred_at <= ?1`).bind(historyCutoff),
72+
db.prepare(`DELETE FROM artifact_resolution_requesters WHERE created_at <= ?1`).bind(requesterCutoff),
6573
])
6674

6775
const changes = results.map(result => result.meta.changes)
@@ -72,5 +80,6 @@ export async function purgeRetainedPersonalData(
7280
digestRuns: changes[3] ?? 0,
7381
weeklyRuns: changes[4] ?? 0,
7482
emailPreferenceEvents: changes[5] ?? 0,
83+
runRequesters: changes[6] ?? 0,
7584
}
7685
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
-- The signed-in account that asked for a public Resolution, kept only while
2+
-- its build runs. When the shared GitHub credentials are rate limited, the
3+
-- build reads GitHub with this account's own token, for this build only.
4+
-- The build deletes the row when it settles; the daily personal data purge
5+
-- removes any row a dead build left behind.
6+
CREATE TABLE artifact_resolution_requesters (
7+
resolution_id TEXT PRIMARY KEY REFERENCES artifact_resolutions(id) ON DELETE CASCADE,
8+
account_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE,
9+
created_at INTEGER NOT NULL
10+
);
11+
12+
CREATE INDEX idx_artifact_resolution_requesters_created_at
13+
ON artifact_resolution_requesters(created_at);

‎test/unit/account-deletion.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,14 @@ function seedAccount(raw: DatabaseSync, account: Account, githubToken: string |
190190
id,
191191
installationId,
192192
)
193+
run(
194+
`INSERT INTO artifact_resolutions (
195+
id, request_fingerprint, state, requested_owner, requested_repository, selector_type, selector_value, visibility
196+
) VALUES (?, ?, 'resolving', 'acme', 'skills', 'named-skill', 'tdd', 'public')`,
197+
`public-run-${login}`,
198+
`public-fingerprint-${login}`,
199+
)
200+
run(`INSERT INTO artifact_resolution_requesters (resolution_id, account_id, created_at) VALUES (?, ?, ?)`, `public-run-${login}`, id, NOW)
193201
run(`INSERT INTO artifact_check_results (resolution_id, name, version, outcome, required) VALUES (?, 'skill-structure', '1', 'pass', 1)`, resolutionId)
194202
run(
195203
`INSERT INTO artifacts (id, content_sha256, content_bytes, format, r2_key) VALUES (?, ?, 128, 'skilld-tar-v1', ?)`,

‎test/unit/artifact-build-reuse.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ const MIGRATIONS = [
4444
'migrations/0112_private_artifact_keys.sql',
4545
'migrations/0122_artifact_resolution_retry_after.sql',
4646
'migrations/0131_artifact_build_reuse.sql',
47+
'migrations/0144_artifact_resolution_requesters.sql',
4748
]
4849
const COMMIT = '0123456789abcdef0123456789abcdef01234567'
4950
const NEXT_COMMIT = 'fedcba9876543210fedcba9876543210fedcba98'

0 commit comments

Comments
 (0)