Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
86 changes: 86 additions & 0 deletions .opencode/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion apps/app/src/app/lib/openwork-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ export type OpenworkHubRepo = {
owner?: string;
repo?: string;
ref?: string;
token?: string;
};

export type OpenworkWorkspaceFileContent = {
Expand Down Expand Up @@ -1420,9 +1421,11 @@ export function createOpenworkServerClient(options: { baseUrl: string; token?: s
const owner = options?.repo?.owner?.trim();
const repo = options?.repo?.repo?.trim();
const ref = options?.repo?.ref?.trim();
const repoToken = options?.repo?.token?.trim();
if (owner) params.set("owner", owner);
if (repo) params.set("repo", repo);
if (ref) params.set("ref", ref);
if (repoToken) params.set("token", repoToken);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Putting the PAT in the query string means it lands in server/proxy access logs and history. The install path already sends the repo (and token) in a POST body — better to do the same here, or send the token as a request header, rather than via ?token=.

const query = params.size ? `?${params.toString()}` : "";
return requestJson<{ items: OpenworkHubSkillItem[] }>(baseUrl, `/hub/skills${query}`, {
token,
Expand All @@ -1432,7 +1435,7 @@ export function createOpenworkServerClient(options: { baseUrl: string; token?: s
installHubSkill: (
workspaceId: string,
name: string,
options?: { overwrite?: boolean; repo?: { owner?: string; repo?: string; ref?: string } },
options?: { overwrite?: boolean; repo?: { owner?: string; repo?: string; ref?: string; token?: string } },
) =>
requestJson<{ ok: boolean; name: string; path: string; action: "added" | "updated"; written: number; skipped: number }>(
baseUrl,
Expand Down
1 change: 1 addition & 0 deletions apps/app/src/app/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ export type HubSkillRepo = {
owner: string;
repo: string;
ref: string;
accessToken?: string;
};

export type HubSkillCard = {
Expand Down
22 changes: 20 additions & 2 deletions apps/app/src/react-app/domains/settings/pages/skills-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ type SkillsViewLocalState = {
customRepoOwner: string;
customRepoName: string;
customRepoRef: string;
customRepoToken: string;
customRepoError: string | null;
shareTarget: SkillCard | null;
cloudSessionNonce: number;
Expand Down Expand Up @@ -168,6 +169,7 @@ const initialSkillsViewLocalState: SkillsViewLocalState = {
customRepoOwner: "",
customRepoName: "",
customRepoRef: "main",
customRepoToken: "",
customRepoError: null,
shareTarget: null,
cloudSessionNonce: 0,
Expand Down Expand Up @@ -242,6 +244,7 @@ export function SkillsView(props: SkillsViewProps) {
customRepoOwner,
customRepoName,
customRepoRef,
customRepoToken,
customRepoError,
shareTarget,
cloudSessionNonce,
Expand Down Expand Up @@ -270,6 +273,7 @@ export function SkillsView(props: SkillsViewProps) {
const setCustomRepoOwner = (value: SetStateAction<string>) => setLocal("customRepoOwner", value);
const setCustomRepoName = (value: SetStateAction<string>) => setLocal("customRepoName", value);
const setCustomRepoRef = (value: SetStateAction<string>) => setLocal("customRepoRef", value);
const setCustomRepoToken = (value: SetStateAction<string>) => setLocal("customRepoToken", value);
const setCustomRepoError = (value: SetStateAction<string | null>) => setLocal("customRepoError", value);
const setShareTeamBusy = (value: SetStateAction<boolean>) => setLocal("shareTeamBusy", value);
const setShareTeamError = (value: SetStateAction<string | null>) => setLocal("shareTeamError", value);
Expand Down Expand Up @@ -644,6 +648,7 @@ export function SkillsView(props: SkillsViewProps) {
setCustomRepoOwner(hubRepo?.owner ?? "");
setCustomRepoName(hubRepo?.repo ?? "");
setCustomRepoRef(hubRepo?.ref || "main");
setCustomRepoToken(hubRepo?.accessToken ?? "");
setCustomRepoError(null);
}, [hubRepo, props.busy]);

Expand All @@ -656,15 +661,16 @@ export function SkillsView(props: SkillsViewProps) {
const owner = customRepoOwner.trim();
const repo = customRepoName.trim();
const ref = customRepoRef.trim() || "main";
const accessToken = customRepoToken.trim();
if (!owner || !repo) {
setCustomRepoError(t("skills.owner_repo_required"));
return;
}
void Promise.resolve(extensions.addHubRepo({ owner, repo, ref })).then(() => {
void Promise.resolve(extensions.addHubRepo({ owner, repo, ref, accessToken })).then(() => {
void extensions.refreshHubSkills({ force: true });
});
closeCustomRepoModal();
}, [closeCustomRepoModal, customRepoName, customRepoOwner, customRepoRef, extensions]);
}, [closeCustomRepoModal, customRepoName, customRepoOwner, customRepoRef, customRepoToken, extensions]);

const isOpenworkInjectedSkill = (skill: SkillCard) => {
const normalizedName = skill.name.trim().toLowerCase();
Expand Down Expand Up @@ -1307,6 +1313,18 @@ export function SkillsView(props: SkillsViewProps) {
/>
</label>

<label className="space-y-1">
<div className="text-xs font-semibold uppercase tracking-widest text-dls-secondary">{t("skills.token_label", "Personal Access Token (Optional)")}</div>
<input
type="password"
value={customRepoToken}
onChange={(event) => setCustomRepoToken(event.currentTarget.value)}
placeholder="ghp_..."
className="w-full rounded-lg border border-dls-border bg-dls-hover px-3 py-2 text-xs font-mono text-dls-text focus:outline-none"
spellCheck={false}
/>
</label>

{customRepoError ? <div className="rounded-xl border border-red-7/20 bg-red-1/40 px-4 py-3 text-xs text-red-12">{customRepoError}</div> : null}
</div>
<DialogFooter>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1292,6 +1292,7 @@ export function createExtensionsStore(options: {
owner: repo.owner,
repo: repo.repo,
ref: repo.ref,
token: repo.accessToken,
},
});
if (refreshHubSkillsAborted) return;
Expand All @@ -1314,9 +1315,11 @@ export function createExtensionsStore(options: {
return;
}

const headers: Record<string, string> = { Accept: "application/vnd.github+json" };
if (repo.accessToken) headers["Authorization"] = `Bearer ${repo.accessToken}`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

repo.accessToken is always undefined here, so no Authorization header is ever sent. The token from the modal is passed to addHubRepo, but normalizeHubRepo (around line 559) returns only { owner, repo, ref } and drops accessToken before it's stored in hubRepo. Every read of repo.accessToken downstream (this fetch, the install repoOverride) therefore sees undefined.

Also the openwork-server path above (canUseOpenworkServerlistHubSkills({ repo: { owner, repo, ref } })) doesn't forward token at all. So as written the PAT is silently discarded and private repos still 404 — the public-repo case works because it needs no token.

Fix: preserve accessToken through normalizeHubRepo, and add token to the server-path listHubSkills call.

const listingRes = await fetch(
`https://api.github.com/repos/${encodeURIComponent(repo.owner)}/${encodeURIComponent(repo.repo)}/contents/skills?ref=${encodeURIComponent(repo.ref)}`,
{ headers: { Accept: "application/vnd.github+json" } },
{ headers },
);
if (!listingRes.ok) {
throw new Error(`Failed to fetch hub catalog (${listingRes.status})`);
Expand Down Expand Up @@ -1667,7 +1670,7 @@ export function createExtensionsStore(options: {
setStateField("skillsStatus", null);

try {
const repoOverride: OpenworkHubRepo = { owner: repo.owner, repo: repo.repo, ref: repo.ref };
const repoOverride: OpenworkHubRepo = { owner: repo.owner, repo: repo.repo, ref: repo.ref, token: repo.accessToken };
if (!openworkClient || !openworkWorkspaceId) return { ok: false, message: "Hub install requires OpenWork server." };
const result = await openworkClient.installHubSkill(openworkWorkspaceId, trimmed, { repo: repoOverride });
await Promise.all([refreshSkills({ force: true }), refreshHubSkills({ force: true })]);
Expand Down
3 changes: 3 additions & 0 deletions apps/server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1880,10 +1880,12 @@ function createRoutes(
const owner = ctx.url.searchParams.get("owner")?.trim();
const repo = ctx.url.searchParams.get("repo")?.trim();
const ref = ctx.url.searchParams.get("ref")?.trim();
const token = ctx.url.searchParams.get("token")?.trim();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Sensitive GitHub PAT is transmitted via URL query parameter on GET /hub/skills, risking exposure in access logs, browser history, and intermediary caches.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/server/src/server.ts, line 1883:

<comment>Sensitive GitHub PAT is transmitted via URL query parameter on GET /hub/skills, risking exposure in access logs, browser history, and intermediary caches.</comment>

<file context>
@@ -1880,10 +1880,12 @@ function createRoutes(
     const owner = ctx.url.searchParams.get("owner")?.trim();
     const repo = ctx.url.searchParams.get("repo")?.trim();
     const ref = ctx.url.searchParams.get("ref")?.trim();
+    const token = ctx.url.searchParams.get("token")?.trim();
     const items = await listHubSkills({
       owner: owner || "different-ai",
</file context>

const items = await listHubSkills({
owner: owner || "different-ai",
repo: repo || "openwork-hub",
ref: ref || "main",
token,
});
return jsonResponse({ items });
});
Expand Down Expand Up @@ -1911,6 +1913,7 @@ function createRoutes(
owner: typeof repoPayload.owner === "string" ? repoPayload.owner : undefined,
repo: typeof repoPayload.repo === "string" ? repoPayload.repo : undefined,
ref: typeof repoPayload.ref === "string" ? repoPayload.ref : undefined,
token: typeof repoPayload.token === "string" ? repoPayload.token : undefined,
}
: undefined;

Expand Down
46 changes: 25 additions & 21 deletions apps/server/src/skill-hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { exists } from "./utils.js";
import { validateSkillName } from "./validators.js";
import { projectSkillsDir } from "./workspace-files.js";

type HubRepo = { owner: string; repo: string; ref: string };
type HubRepo = { owner: string; repo: string; ref: string; token?: string };

const DEFAULT_HUB_REPO: HubRepo = {
owner: "different-ai",
Expand All @@ -31,27 +31,29 @@ function hubRawBase(repo: HubRepo) {
return `https://raw.githubusercontent.com/${encodeURIComponent(repo.owner)}/${encodeURIComponent(repo.repo)}/${encodeURIComponent(repo.ref)}`;
}

async function fetchJson(url: string): Promise<any> {
const res = await fetch(url, {
headers: {
Accept: "application/vnd.github+json",
"User-Agent": "openwork-server",
},
});
async function fetchJson(url: string, token?: string): Promise<any> {
const headers: Record<string, string> = {
Accept: "application/vnd.github+json",
"User-Agent": "openwork-server",
};
if (token) headers["Authorization"] = `Bearer ${token}`;

const res = await fetch(url, { headers });
if (!res.ok) {
const text = await res.text().catch(() => "");
throw new ApiError(502, "hub_fetch_failed", `Failed to fetch hub data (${res.status}): ${text || url}`);
}
return res.json();
}

async function fetchText(url: string): Promise<string> {
const res = await fetch(url, {
headers: {
Accept: "text/plain",
"User-Agent": "openwork-server",
},
});
async function fetchText(url: string, token?: string): Promise<string> {
const headers: Record<string, string> = {
Accept: "text/plain",
"User-Agent": "openwork-server",
};
if (token) headers["Authorization"] = `Bearer ${token}`;

const res = await fetch(url, { headers });
if (!res.ok) {
const text = await res.text().catch(() => "");
throw new ApiError(502, "hub_fetch_failed", `Failed to fetch hub data (${res.status}): ${text || url}`);
Expand Down Expand Up @@ -108,7 +110,7 @@ export async function listHubSkills(repo: HubRepo = DEFAULT_HUB_REPO): Promise<H
return cachedCatalog.items;
}

const listing = await fetchJson(`${hubApiBase(repo)}/contents/skills?ref=${encodeURIComponent(repo.ref)}`);
const listing = await fetchJson(`${hubApiBase(repo)}/contents/skills?ref=${encodeURIComponent(repo.ref)}`, repo.token);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Catalog cache key omits authentication context, so private repo listings fetched with a PAT can be served to later unauthenticated requests for the same owner/repo/ref.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/server/src/skill-hub.ts, line 113:

<comment>Catalog cache key omits authentication context, so private repo listings fetched with a PAT can be served to later unauthenticated requests for the same owner/repo/ref.</comment>

<file context>
@@ -108,7 +110,7 @@ export async function listHubSkills(repo: HubRepo = DEFAULT_HUB_REPO): Promise<H
   }
 
-  const listing = await fetchJson(`${hubApiBase(repo)}/contents/skills?ref=${encodeURIComponent(repo.ref)}`);
+  const listing = await fetchJson(`${hubApiBase(repo)}/contents/skills?ref=${encodeURIComponent(repo.ref)}`, repo.token);
   const dirs = Array.isArray(listing)
     ? listing
</file context>

const dirs = Array.isArray(listing)
? listing
.filter((entry) => entry && typeof entry === "object" && entry.type === "dir" && typeof entry.name === "string")
Expand All @@ -120,7 +122,7 @@ export async function listHubSkills(repo: HubRepo = DEFAULT_HUB_REPO): Promise<H
try {
const skillName = dirName.trim();
validateSkillName(skillName);
const skillMd = await fetchText(`${rawBase}/skills/${encodeURIComponent(skillName)}/SKILL.md`);
const skillMd = await fetchText(`${rawBase}/skills/${encodeURIComponent(skillName)}/SKILL.md`, repo.token);
const { data, body } = parseFrontmatter(skillMd);
const name = typeof data.name === "string" ? data.name : skillName;
const descriptionRaw = typeof data.description === "string" ? data.description : "";
Expand Down Expand Up @@ -191,6 +193,7 @@ export async function installHubSkill(
owner: input.repo?.owner?.trim() || DEFAULT_HUB_REPO.owner,
repo: input.repo?.repo?.trim() || DEFAULT_HUB_REPO.repo,
ref: input.repo?.ref?.trim() || DEFAULT_HUB_REPO.ref,
token: input.repo?.token?.trim(),
};

const prefix = `skills/${name}/`;
Expand All @@ -200,7 +203,7 @@ export async function installHubSkill(

await mkdir(baseDir, { recursive: true });

const tree = await fetchJson(`${hubApiBase(repo)}/git/trees/${encodeURIComponent(repo.ref)}?recursive=1`);
const tree = await fetchJson(`${hubApiBase(repo)}/git/trees/${encodeURIComponent(repo.ref)}?recursive=1`, repo.token);
const entries: HubTreeEntry[] = Array.isArray(tree?.tree) ? tree.tree : [];
const files = entries
.filter((entry) => entry && entry.type === "blob" && typeof entry.path === "string" && entry.path.startsWith(prefix))
Expand Down Expand Up @@ -230,9 +233,10 @@ export async function installHubSkill(
}

await mkdir(dirname(destPath), { recursive: true });
const res = await fetch(`${rawBase}/${file.path}`, {
headers: { "User-Agent": "openwork-server" },
});
const headers: Record<string, string> = { "User-Agent": "openwork-server" };
if (repo.token) headers["Authorization"] = `Bearer ${repo.token}`;

const res = await fetch(`${rawBase}/${file.path}`, { headers });
if (!res.ok) {
const text = await res.text().catch(() => "");
throw new ApiError(502, "hub_fetch_failed", `Failed to fetch hub file (${res.status}): ${text || file.path}`);
Expand Down